Hi Nuno, On 28.10.2024 14:34, Nuno Sá wrote: > On Fri, 2024-10-25 at 11:49 +0200, Angelo Dureghello 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> > > --- > > LGTM, just one question inline. If not an issue: > > Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx> > > > 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. > > + > > + 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 > > obj-$(CONFIG_AD5360) += ad5360.o > > obj-$(CONFIG_AD5380) += ad5380.o > > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c > > new file mode 100644 > > index 000000000000..97dfc598aec6 > > --- /dev/null > > +++ b/drivers/iio/dac/ad3552r-hs.c > > @@ -0,0 +1,530 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Analog Devices AD3552R > > + * Digital to Analog converter driver, High Speed version > > + * > > + * Copyright 2024 Analog Devices Inc. > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/delay.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/iio/backend.h> > > +#include <linux/iio/buffer.h> > > +#include <linux/mod_devicetable.h> > > +#include <linux/platform_device.h> > > +#include <linux/property.h> > > +#include <linux/units.h> > > + > > +#include "ad3552r.h" > > +#include "ad3552r-hs.h" > > + > > +struct ad3552r_hs_state { > > + const struct ad3552r_model_data *model_data; > > + struct gpio_desc *reset_gpio; > > + struct device *dev; > > + struct iio_backend *back; > > + bool single_channel; > > + struct ad3552r_ch_data ch_data[AD3552R_MAX_CH]; > > + struct ad3552r_hs_platform_data *data; > > +}; > > + > > +static int ad3552r_qspi_update_reg_bits(struct ad3552r_hs_state *st, > > + u32 reg, u32 mask, u32 val, > > + size_t xfer_size) > > +{ > > + u32 rval; > > + int ret; > > + > > + ret = st->data->bus_reg_read(st->back, reg, &rval, xfer_size); > > + if (ret) > > + return ret; > > + > > + rval = (rval & ~mask) | val; > > + > > + return st->data->bus_reg_write(st->back, reg, rval, xfer_size); > > +} > > + > > +static int ad3552r_hs_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct ad3552r_hs_state *st = iio_priv(indio_dev); > > + int ret; > > + int ch = chan->channel; > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_SAMP_FREQ: > > + /* > > + * Using 4 lanes (QSPI), then using 2 as DDR mode is > > + * considered always on (considering buffering mode always). > > + */ > > + *val = DIV_ROUND_CLOSEST(st->data->bus_sample_data_clock_hz * > > + 4 * 2, chan->scan_type.realbits); > > + > > + return IIO_VAL_INT; > > + > > + case IIO_CHAN_INFO_RAW: > > + ret = st->data->bus_reg_read(st->back, > > + AD3552R_REG_ADDR_CH_DAC_16B(chan->channel), > > + val, 2); > > + if (ret) > > + return ret; > > Is there any potential issue by doing a raw access when streaming? > seems not creating any issue, even if, on write while buffering i get an error, that should be ok: root@analog:~# echo 45222 > /sys/bus/iio/devices/iio\:device0/out_voltage0_raw -bash: echo: write error: Device or resource busy while on raw read i always get a value of "1", not clear why. Once stopped the buffering, raw read/write works back properly. > - Nuno Sá > > regards, angelo