On Thu, 03 Oct 2024 19:29:07 +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
A few trivial things inline.
Jonathan
> 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..1e141d573d76
> --- /dev/null
> +++ b/drivers/iio/dac/ad3552r-hs.c
> +static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
> +{
> + u8 gs_p, gs_n;
> + s16 goffs;
> + u16 id, rfb;
> + u16 gain = 0, offset = 0;
> + u32 val, range;
> + int err;
...
> + err = ad3552r_get_custom_gain(st->dev, child, &gs_p, &gs_n, &rfb,
> + &goffs);
> + if (err)
> + return err;
> +
> + gain = ad3552r_calc_custom_gain(gs_p, gs_n, goffs);
> + offset = abs((s32)goffs);
Why the cast? abs is special cased for short which should work with s16 I think.
> +
> + return ad3552r_hs_setup_custom_gain(st, gain, offset);
> +}
> +static int ad3552r_hs_probe(struct platform_device *pdev)
> +{
> + struct ad3552r_hs_state *st;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->dev = &pdev->dev;
> +
> + st->data = pdev->dev.platform_data;
> + if (!st->data) {
> + dev_err(&pdev->dev, "no platform data!\n");
> + return -ENODEV;
Trivial preference for return dev_err_probe() mostly because
we can then drop the brackets.
> + }
> diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
> index 088eb8ecfac6..d9da4794fcb3 100644
> --- a/drivers/iio/dac/ad3552r.h
> +++ b/drivers/iio/dac/ad3552r.h
> #define AD3552R_GAIN_SCALE 1000
> #define AD3552R_LDAC_PULSE_US 100
>
> +#define AD3552R_CH0_ACTIVE BIT(0)
> +#define AD3552R_CH1_ACTIVE BIT(1)
> +#define AD3552R_CH0_CH1_ACTIVE (AD3552R_CH0_ACTIVE | \
> + AD3552R_CH1_ACTIVE)
Add a space before the above. I think it's aligned with the (
but should be just after that.
> +
> #define AD3552R_MAX_RANGES 5
> #define AD3542R_MAX_RANGES 6
