Hi Jonathan, On 26.10.2024 18:57, Jonathan Cameron wrote: > On Fri, 25 Oct 2024 11:49:40 +0200 > Angelo Dureghello <adureghello@xxxxxxxxxxxx> wrote: > > > From: Angelo Dureghello <adureghello@xxxxxxxxxxxx> > > > > Add High Speed ad3552r platform driver. > > > > The ad3552r DAC is controlled by a custom (fpga-based) DAC IP > > through the current AXI backend, or similar alternative IIO backend. > > > > Compared to the existing driver (ad3552r.c), that is a simple SPI > > driver, this driver is coupled with a DAC IIO backend that finally > > controls the ad3552r by a fpga-based "QSPI+DDR" interface, to reach > > maximum transfer rate of 33MUPS using dma stream capabilities. > > > > All commands involving QSPI bus read/write are delegated to the backend > > through the provided APIs for bus read/write. > > > > Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx> > > --- > Hi Angelo, > > I'd missed a build issue in previous reviews. :( > > > drivers/iio/dac/Kconfig | 14 ++ > > drivers/iio/dac/Makefile | 1 + > > drivers/iio/dac/ad3552r-hs.c | 530 +++++++++++++++++++++++++++++++++++++++++++ > > drivers/iio/dac/ad3552r-hs.h | 19 ++ > > drivers/iio/dac/ad3552r.h | 4 + > > 5 files changed, 568 insertions(+) > > > > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > > index 26f9de55b79f..f76eaba140d8 100644 > > --- a/drivers/iio/dac/Kconfig > > +++ b/drivers/iio/dac/Kconfig > > @@ -6,6 +6,20 @@ > > > > menu "Digital to analog converters" > > > > +config AD3552R_HS > > + tristate "Analog Devices AD3552R DAC High Speed driver" > > + select ADI_AXI_DAC > > + help > > + Say yes here to build support for Analog Devices AD3552R > > + Digital to Analog Converter High Speed driver. > > + > > + The driver requires the assistance of an IP core to operate, > > + since data is streamed into target device via DMA, sent over a > > + QSPI + DDR (Double Data Rate) bus. > > Tabs and space mix that needs fixing. > > > + > > + To compile this driver as a module, choose M here: the > > + module will be called ad3552r-hs. > > + > > config AD3552R > > tristate "Analog Devices AD3552R DAC driver" > > depends on SPI_MASTER > > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile > > index c92de0366238..d92e08ca93ca 100644 > > --- a/drivers/iio/dac/Makefile > > +++ b/drivers/iio/dac/Makefile > > @@ -4,6 +4,7 @@ > > # > > > > # When adding new entries keep the list in alphabetical order > > +obj-$(CONFIG_AD3552R_HS) += ad3552r-hs.o ad3552r-common.o > > obj-$(CONFIG_AD3552R) += ad3552r.o ad3552r-common.o > > This causes all sorts of issues. The same code should not be linked into two > separate drivers. Try building one as a module and one built in. > right now, seems i cannot catch any issue, nor building or in runtime: ad3552r [M] ad3552r-hs [*] (ad3552r-common stays built in), ad3552r visible in lsmod, ad3552r-hs works ad3552r [*] ad3552r-hs [M] (ad3552r-common stays built in), ad3552r-hs visible in lsmod, ad3552r-hs works ad3552r [M] ad3552r-hs [M] (ad3552r-common.ko), ad3552r, ad3552r-hs and ad3552r-common are visible in lsmod, ad3552r-hs works, probe and removal, and also link/unlink tested). Please let me know, i can proceed modifying as you require, if it's the case. > The trick is a hidden symbol in Kconfig and an extra line in here > obj-$(CONFIG_AD3352R_LIB) += ad3552-common.o > > and > //note no text as we don't want this to be user selectable > > config AD3352R_LIB > tristate > > config AD3552R_HS > tristate "Analog Devices AD3552R DAC High Speed driver" > select ADI_AXI_DAC > select AD3352R_LIB > help > Say yes here to build support for Analog Devices AD3552R > Digital to Analog Converter High Speed driver. > > The driver requires the assistance of an IP core to operate, > since data is streamed into target device via DMA, sent over a > QSPI + DDR (Double Data Rate) bus. > > To compile this driver as a module, choose M here: the > module will be called ad3552r-hs. > > > config AD3552R > tristate "Analog Devices AD3552R DAC driver" > depends on SPI_MASTER > select AD3352R_LIB > help > ... > > The pressure/mpl115 is done like this. > > > > obj-$(CONFIG_AD5360) += ad5360.o > > obj-$(CONFIG_AD5380) += ad5380.o > > Anyhow, to me the code looks ready to go subject to this. > > If nothing else comes up I'm almost confident enough of the fix to just > do it (and the few trivial things in previous review), but probably quicker > and less prone to error if you have time to spin a v9, perhaps after letting others > have a day or two to review v8 next week. > > rc5 is tomorrow, so we have a little time left this cycle. > > Jonathan Regards, angelo
