Re: [PATCH v5 2/2] iio: adc: Add the NXP SAR ADC support for the s32g2/3 platforms
From: Daniel Lezcano
Date: Tue Nov 18 2025 - 09:01:15 EST
Hi Andy,
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:
[ ... ]
+ dma_samples = (u32 *)dma_buf->buf;
Is it aligned properly for this type of casting?
TBH, I don't know the answer :/
How can I check that ?
Is buf defined as a pointer to u32 / int or bigger? or is it just byte buffer?
If the latter, how does the address of it being formed? Does it come from a heap
(memory allocator)? If yes, we are fine, as this is usually the case for all
(k)malloc'ed memory.
buf is a byte buffer allocated with dmam_alloc_coherent(..., GFP_KERNEL)
We are fine :-)
...
+ 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 ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog