On Fri, Dec 1, 2023 at 3:08 AM Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > > On Thu, 2023-11-30 at 17:30 -0600, David Lechner wrote: > > On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay > > <devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote: > > > > > > From: Nuno Sa <nuno.sa@xxxxxxxxxx> > > > > > > Convert the driver to use the new IIO backend framework. The device > > > functionality is expected to be the same (meaning no added or removed > > > features). > > > > Missing a devicetree bindings patch before this one? > > > > > > > > Also note this patch effectively breaks ABI and that's needed so we can > > > properly support this device and add needed features making use of the > > > new IIO framework. > > > > Can you be more specific about what is actually breaking? > > > > > > > > Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx> > > > --- > > > drivers/iio/adc/Kconfig | 2 +- > > > drivers/iio/adc/ad9467.c | 256 +++++++++++++++++++++++++++++------------------ > > > 2 files changed, 157 insertions(+), 101 deletions(-) > > > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > > index 1e2b7a2c67c6..af56df63beff 100644 > > > --- a/drivers/iio/adc/Kconfig > > > +++ b/drivers/iio/adc/Kconfig > > > @@ -275,7 +275,7 @@ config AD799X > > > config AD9467 > > > tristate "Analog Devices AD9467 High Speed ADC driver" > > > depends on SPI > > > - depends on ADI_AXI_ADC > > > + select IIO_BACKEND > > > help > > > Say yes here to build support for Analog Devices: > > > * AD9467 16-Bit, 200 MSPS/250 MSPS Analog-to-Digital Converter > > > diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c > > > index 5db5690ccee8..8b0402e73ace 100644 > > > --- a/drivers/iio/adc/ad9467.c > > > +++ b/drivers/iio/adc/ad9467.c > > > > <snip> > > > > > +static int ad9467_buffer_get(struct iio_dev *indio_dev) > > > > perhaps a more descriptive name: ad9467_buffer_setup_optional? > > > > Hmm, no strong feeling. So yeah, can do as you suggest. Even though, now that I'm > thinking, I'm not so sure if this is just some legacy thing we had in ADI tree. I > wonder if it actually makes sense for a device like with no buffering support?! > > > > +{ > > > + struct device *dev = indio_dev->dev.parent; > > > + const char *dma_name; > > > + > > > + if (!device_property_present(dev, "dmas")) > > > + return 0; > > > + > > > + if (device_property_read_string(dev, "dma-names", &dma_name)) > > > + dma_name = "rx"; > > > + > > > + return devm_iio_dmaengine_buffer_setup(dev, indio_dev, dma_name); > > > > The device tree bindings for "adi,ad9467" don't include dma properties > > (nor should they). Perhaps the DMA lookup should be a callback to the > > backend? Or something similar to the SPI Engine offload that we are > > working on? > > > > Oh yes, I need to update the bindings. In the link I sent you we can see my thoughts > on this. In theory, hardwarewise, it would actually make sense for the DMA to be on > the backend device because that's where the connection is in HW. However, since we > want to have the IIO interface in the frontend, it would be hard to do that without > hacking devm_iio_dmaengine_buffer_setup(). I mean, lifetime wise it would be far from > wise to have the DMA buffer associated to a completely different device than the IIO > parent device. I mean, one way could just be export iio_dmaengine_buffer_free() and > iio_dmaengine_buffer_alloc() so we can actually control the lifetime of the buffer > from the frontend device. If Jonathan is fine with this, I'm on board for it.... > > - Nuno Sá > > > I was planning on exporting iio_dmaengine_buffer_alloc() [1] for SPI Engine offload support, so I hope that is the right way to go. ;-) [1]: https://github.com/analogdevicesinc/linux/pull/2341/commits/71048ff83a63e9d0a5ddb9ffa331871edd6bd2a5