On Fri, 2025-01-10 at 11:24 +0100, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
>
> The ad354xr requires DSPI mode (2 data lanes) to work in buffering
> mode, so backend needs to allow a mode selection between:
> SPI (entire ad35xxr family),
> DSPI (ad354xr),
> QSPI (ad355xr).
>
I guess this could be misleading people to think this is being handled by the
backend framework when it's not. I would rephrase things a bit in here.
> About removal of AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER, according to
> the HDL history the flag has never been used. So looks like the driver
> was including it by mistake or in anticipation for something that was
> never implemented on HDL side.
>
> Current HDL updated documentation confirm it is actually not in use
> anymore and replaced by the IO_MODE bits.
>
> Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
> ---
With the improved change and the inline note:
Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> drivers/iio/dac/ad3552r-hs.h | 8 ++++++++
> drivers/iio/dac/adi-axi-dac.c | 22 +++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad3552r-hs.h b/drivers/iio/dac/ad3552r-hs.h
> index 724261d38dea..4a9e35234124 100644
> --- a/drivers/iio/dac/ad3552r-hs.h
> +++ b/drivers/iio/dac/ad3552r-hs.h
> @@ -8,11 +8,19 @@
>
> struct iio_backend;
>
> +enum ad3552r_io_mode {
> + AD3552R_IO_MODE_SPI,
> + AD3552R_IO_MODE_DSPI,
> + AD3552R_IO_MODE_QSPI,
> +};
> +
> struct ad3552r_hs_platform_data {
> int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val,
> size_t data_size);
> int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val,
> size_t data_size);
> + int (*bus_set_io_mode)(struct iio_backend *back,
> + enum ad3552r_io_mode mode);
> u32 bus_sample_data_clock_hz;
> };
>
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index ac871deb8063..bcaf365feef4 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -64,7 +64,7 @@
> #define AXI_DAC_UI_STATUS_IF_BUSY BIT(4)
> #define AXI_DAC_CUSTOM_CTRL_REG 0x008C
> #define AXI_DAC_CUSTOM_CTRL_ADDRESS GENMASK(31, 24)
> -#define AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER BIT(2)
> +#define AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE GENMASK(3, 2)
> #define AXI_DAC_CUSTOM_CTRL_STREAM BIT(1)
> #define AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA BIT(0)
>
> @@ -722,6 +722,25 @@ static int axi_dac_bus_reg_read(struct iio_backend *back,
> u32 reg, u32 *val,
> return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
> }
>
> +static int axi_dac_bus_set_io_mode(struct iio_backend *back,
> + enum ad3552r_io_mode mode)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> + int ival, ret;
> +
No harm in doing some validation on 'mode'.
- Nuno Sá
> + guard(mutex)(&st->lock);
> +
> + ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> + AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE,
> + FIELD_PREP(AXI_DAC_CUSTOM_CTRL_MULTI_IO_MODE, mode));
> + if (ret)
> + return ret;
> +
> + return regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG,
> ival,
> + FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0, 10,
> + 100 * KILO);
> +}
> +
> static void axi_dac_child_remove(void *data)
> {
> platform_device_unregister(data);
> @@ -733,6 +752,7 @@ static int axi_dac_create_platform_device(struct
> axi_dac_state *st,
> struct ad3552r_hs_platform_data pdata = {
> .bus_reg_read = axi_dac_bus_reg_read,
> .bus_reg_write = axi_dac_bus_reg_write,
> + .bus_set_io_mode = axi_dac_bus_set_io_mode,
> .bus_sample_data_clock_hz = st->dac_clk_rate,
> };
> struct platform_device_info pi = {
>
