Re: [PATCH v5 2/2] iio: adc: Add the NXP SAR ADC support for the s32g2/3 platforms

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Nov 18, 2025 at 02:57:41PM +0100, Daniel Lezcano wrote:
> On 10/31/25 13:45, Andy Shevchenko wrote:
> > On Fri, Oct 31, 2025 at 12:32:03PM +0100, Daniel Lezcano wrote:
> > > On 10/30/25 10:28, Andy Shevchenko wrote:
> > > > On Thu, Oct 30, 2025 at 09:27:21AM +0100, Daniel Lezcano wrote:
> > > > > On 10/18/25 22:12, Andy Shevchenko wrote:
> > > > > > On Fri, Oct 17, 2025 at 06:42:38PM +0200, Daniel Lezcano wrote:

[ ... ]

> > > > > > > +	dmaengine_tx_status(info->dma_chan, info->cookie, &state);
> > > > > > 
> > > > > > No return value check?
> > > > > 
> > > > > The return value is not necessary here because the caller of the callback
> > > > > will check with dma_submit_error() in case of error which covers the
> > > > > DMA_ERROR case and the other cases are not useful because the residue is
> > > > > taken into account right after.
> > > > 
> > > > In some cases it might return DMA_PAUSE (and actually this is the correct way
> > > > to get residue, one needs to pause the channel to read it, otherwise it will
> > > > give outdated / incorrect information).
> > > 
> > > But if the residue is checked in the callback routine without checking
> > > DMA_PAUSED, the result is the same no ?
> > 
> > DMA in some corner cases might have already be charged for the next transfer.
> > Do you have a synchronisation between DMA start and residue check?
> > 
> > I.o.w. this may work for your case, but in general it's not guaranteed. The proper
> > read of residue is to: pause DMA --> read residue --> resume DMA.
> 
> I discussed with Vinod about this change and he suggested to use the
> callback_result() to get the residue as a parameter so the
> dmaengine_txstatus() call won't be needed anymore.
> 
> Unfortunately, it does not work. I had a look in the DMA driver and the
> internals but my knowledge is limited in this area so I was unable to find
> out what is going on. Moreover there are no so many driver using this API I
> can use as an example. The best I was able to do was propagating the residue
> to the result in the vchan_complete() but it does not work.
> 
> Then I stepped back by not using the callback_result() and used
> dmaengine_pause(), read the residue, dmaengine_resume() but there are no
> result after these calls. I don't know why.
> 
> The issue you are mentioning above should be handled in other drivers doing
> the same kind of acquisition but the routine is similar to the one proposed
> here (eg. stm32).
> 
> The NXP SAR acquisition routine is running since several years in production
> AFAICT.
> 
> I investigated the different solutions without success, while I can run the
> acquisition routine without problem here with my hardware. A signal
> generator captured by the ADC, plotted and compared with the oscilloscope
> display.
> 
> The circ buffer is working well here and no bug was spotted with the current
> routine. I think I did my best to make the driver better from its initial
> submission. The best is the enemy of the good, and I would like to make some
> progress here in the driver acceptance. Changing the entire driver for the
> sake of replacing the circ_buffer by the kfifo and change the code for a
> scenario which is not happening is not really worth. Especially that the DMA
> engine is being modified to take into the cyclic DMA in its API, thus the
> circ_buffer and the routine will go away once the driver is changed to take
> into account this new API.
> 
> IOW, can we keep this routine as it is for now as it works fine and go
> forward for a v6 ?

Thanks for trying and getting to this. Please, make a summary of the research
and pack it into a paragraph in the commit message, so we will have this in
the history. Maybe even a short comment in the code.

-- 
With Best Regards,
Andy Shevchenko