Re: [PATCH v5 09/10] iio: dac: add mcf54415 DAC

On Mon, 15 Jun 2026 20:20:21 +0900
Angelo Dureghello <[email protected]> wrote:

> Hi Jonathan,
> 
> On Thu, Jun 11, 2026 at 11:48:00AM +0100, Jonathan Cameron wrote:
> > On Wed, 10 Jun 2026 22:35:14 +0200
> > Angelo Dureghello <[email protected]> wrote:
> >  
> > > From: Angelo Dureghello <[email protected]>
> > >
> > > Add basic version of mcf54415 DAC driver. DAC is embedded in the SoC and
> > > DAC configuration registers are mapped in the internal IO address space.
> > >
> > > The DAC accepts a 12-bit digital signal and creates a monotonic 12-bit
> > > analog output varying from DAC_VREFL to DAC_VREFH. The DAC module
> > > consists of a conversion unit, an output amplifier, and the associated
> > > digital control blocks. Default register values for DAC_VREFL and DAC_VREFH
> > > are respectively 0 and 0xfff, left untouched in this initial version.
> > >
> > > This initial version of the driver is minimalistic, "output raw" only, to
> > > be extended in the future. DMA and external sync are disabled, default mode
> > > is high speed, default format is right-justified 12-bit on 16-bit word.
> > >
> > > Signed-off-by: Angelo Dureghello <[email protected]>  
> > I'm lazy so didn't check earlier versions but assume the two bits
> > of feedback from Sashiko are false positives:
> > https://sashiko.dev/#/patchset/20260610-wip-stmark2-dac-v5-0-b76b83366d5c%40baylibre.com
> >
> > The one about clock underflow if resume fails, and then devm cleanup happens later
> > is a bit nasty.
> >
> > I did a bit of digging and maybe it is better to just leave the clock on?
> > The status dev.power.is_suspended is set to false whether or not resume succeeded
> > and I believe a following suspend will not take into account that resume failed.
> >
> > I'm not set up to poke the combinations but it might be worth trying that.
> > +CC common clk people who may immediately know what the right answer is.
> >  
> 
> was about testing this, there are no bus faults on read/write of registers
> with clock disabled, nor warnings in dmesg on double disable.

Great. That makes life easier.  I'm curious to why we don't get a double disable
though... Guess I'm missing something in the flows.
> 
> Anyway, i see now from arch Kconfig that is not possible to have CONFIG_PM
> for this specifig CPU (with MMU, PM is force-disabled), so would remove pm_ops.
> Ok ?

I suspect that's something that might get 'fixed' at somepoint, so I'd keep
the PM stuff in this driver as long as you can check it basically works.

Jonathan

> 
> Will fix all other things you pointyed out in v6.
> 
> > Otherwise just a few minor style comments inline.
> >
> > Thanks,
> >
> > Jonathan
> >  
> 
> thanks,
> regards,
> angelo
>