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

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 10 Sep 2025 12:57:19 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Tue, 2025-09-09 at 18:22 +0200, Daniel Lezcano wrote:
> > On 09/09/2025 11:29, Nuno Sá wrote:  
> > > Hi *Daniel* (sorry for that :)),
> > > 
> > > On Mon, 2025-09-08 at 08:58 -0500, David Lechner wrote:  
> > > > On 9/8/25 7:16 AM, Daniel Lezcano wrote:  
> > > > > On 05/09/2025 23:54, David Lechner wrote:  
> > > > > > On 9/5/25 3:58 PM, Daniel Lezcano wrote:  
> > > > > > > On 05/09/2025 17:25, David Lechner wrote:  
> > > > > > > > On 9/5/25 4:44 AM, Daniel Lezcano wrote:  
> > > > > > > > > On 04/09/2025 19:49, David Lechner wrote:  
> > > > > > > > > > On 9/4/25 12:40 PM, Daniel Lezcano wrote:  
> > > > > > > 
> > > > > > > [ ... ]  
> > 
> > [ ... ]
> >   
> > > > Unfortunately, not really. Until the last few years, there wasn't really
> > > > any users of these APIs. I added
> > > > devm_iio_dmaengine_buffer_setup_with_handle()
> > > > for the SPI offloading work I did recently. The only reason it had to be
> > > > added is because we needed to get the DMA handle from a different
> > > > devicetree
> > > > node from the ADC's node. Since this device has dmas and dma-names in
> > > > the devicetree, then if devm_iio_dmaengine_buffer_setup[_ext]() doesn't
> > > > work
> > > > with that, then it might have other problems (assumptions made for a
> > > > specific
> > > > use case) than just not calling dma_slave_config().
> > > > 
> > > > I think maybe Nuno and certainly I are guilty of trying to offer you
> > > > advice
> > > > without looking deeply enough into what you already submitted. :-/
> > > >   
> > > 
> > > Yes, I pretty much just asked for (at least) some discussion about this and
> > > see
> > > if we could use what we already have in IIO.
> > >   
> > > > I see now that what you are doing with the DMA looks more like other SoC
> > > > ADCs
> > > > (AT91/STM32/AM335x) which is quite different from how the
> > > > iio_dmaengine_buffer
> > > > stuff works, e.g. cyclic vs. not. So unless you are interested in evolving  
> > > 
> > > Supporting cyclic should be fairly easy (Paul left it almost prepared for
> > > it)
> > > and do I have some patches already. I'm just waiting on having something
> > > accepted on the ADI DMA IP driver (dmaengine) before sending the IIO patches
> > > I
> > > already have.
> > >   
> > > > the iio_dmaengine_buffer code to be more general to handle this case as
> > > > well,
> > > > it might not be the right tool for the job currently.  
> > > 
> > > I think for the STM (IIRC) case they "open" coded it because the IIO DMA
> > > support
> > > we had at the time (if any) was more "rudimentary" - (no real zero copy
> > > interface with userspace for example). But yeah, it seems there are some
> > > gaps
> > > for your usecase so as David said, you would need to be interested in
> > > evolving
> > > the IIO DMA API to accommodate your needs. Again, if nicely integrated you
> > > would
> > > gain some nice "improvements" in performance (not having to use the fileio
> > > interface for reading the buffers) for "free".
> > > 
> > > As for dma_slave_config(), you're right and we have no usecase needing that
> > > setup and TBH, I did not looked or have any idea (atm) on how to do it. That
> > > said, I'll be here to help and contribute if you decide to try and follow
> > > the
> > > IIO DMA buffer API.  
> > 
> > I would be glad to help improving the IIO DMA but I don't have enough 
> > bandwidth ATM. I was hoping to get this driver merged for v6.18 which is 
> > the next LTS AFAICT. Is it something possible by taking into account all 
> > the comments without improving the DMA code ?
> >   
> 
> No personal problem with that. But ultimately that is up to Jonathan :)

I'm fine with a device doing DMA that isn't using the DMA buffer interface.
That is, basically what you have here. The only risk is walking ourselves
into a corner of having to maintain compatibility if later on the zero
copy performance is needed.

We can solve that, but today I don't recall us having any devices
offering both interfaces.

> 
> > Do you have a pointer to Paul's series and the patches you mentioned 
> > above ? I can give a try when having some spare time
> > 
> >   
> 
> Here is the DMABUF work from Paul [1]. But I do not think this is of much use
> for you (as an in kernel consumer) but it is still interesting :).
> 
> My own patches are in here [2] but you have out of tree "noise" in there [2]. At
> ADI tree we had some legacy "high speed" implementation that we're still
> supporting (eventually it will go away and we will sync completely with the
> upstream solution). I was hopping for this [3] to land first before sending the
> IIO bits but I'm having some issues with lack of bandwidth (I guess) from the
> DMA maintainer. They are not really dependent on each other so I might send the
> IIO stuff soon enough.
> 
> One last comment about dma_slave_config(). That should be easy to go around with
> devm_iio_dmaengine_buffer_setup_with_handle(). I mean, we canl do all the DMA
> chan request in the consumer driver (which would allow to easily call
> dma_slave_config() when needed) and pass the DMA handle to IIO with the API
> David introduced. In the future, given enough users, we could introduce an
> helper in the IIO code and it would be easy to "convert" the driver. Now, if you
> do need to do something special in the DMA termination callback, we would
> definitely need to add the mechanism for that. In the ADI tree we do have a way
> for a custom DMA submit operation [4] but we never had a proper upstreamable
> user for it :(
> 
> Anyways, just some thoughts if you or someone else ever have the time for this
> :)
> 
> 
> [1]: https://lore.kernel.org/linux-iio/20240620122726.41232-1-paul@crapouillou.net/
> [2]: https://github.com/analogdevicesinc/linux/commit/a1f64e5e7927d1d96da08245cce35ba9e08a5f52
> [3]: https://lore.kernel.org/dmaengine/20250811-dev-axi-dmac-fixes-v1-0-8d9895f6003f@analog.com/
> [4]: https://github.com/analogdevicesinc/linux/blob/main/drivers/iio/buffer/industrialio-buffer-dma.c#L1214
> 
> - Nuno Sá