| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a643e2cc-b540-42df-b63b-07cc339bb33e@baylibre.com>
Date: Fri, 10 Jan 2025 10:12:12 -0600
From: David Lechner <dlechner@...libre.com>
To: Angelo Dureghello <adureghello@...libre.com>,
Lars-Peter Clausen <lars@...afoo.de>,
Michael Hennerich <Michael.Hennerich@...log.com>,
Jonathan Cameron <jic23@...nel.org>, Nuno Sa <nuno.sa@...log.com>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 8/9] iio: dac: ad3552r-hs: add ad3541/2r support
On 1/10/25 4:24 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@...libre.com>
>
> A new FPGA HDL has been developed from ADI to support ad354xr
> devices.
>
> Add support for ad3541r and ad3542r with following additions:
>
> - use common device_info structures for hs and non hs drivers,
> - DMA buffering, use DSPI mode for ad354xr and QSPI for ad355xr,
> - change sample rate to respect number of lanes.
>
> Signed-off-by: Angelo Dureghello <adureghello@...libre.com>
> ---
> drivers/iio/dac/ad3552r-common.c | 4 +
> drivers/iio/dac/ad3552r-hs.c | 225 ++++++++++++++++++++++++++++++++-------
> drivers/iio/dac/ad3552r.h | 3 +
> 3 files changed, 195 insertions(+), 37 deletions(-)
>
...
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index bfb6228c9b9b..4600a9e84dfc 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
...
> @@ -104,6 +136,42 @@ static int ad3552r_hs_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int ad3552r_hs_set_bus_io_mode_hs(struct ad3552r_hs_state *st)
> +{
> + int bus_mode;
> +
> + if (st->model_data->num_spi_data_lanes == 4)
> + bus_mode = AD3552R_IO_MODE_QSPI;
> + else
> + bus_mode = AD3552R_IO_MODE_DSPI;
> +
> + return st->data->bus_set_io_mode(st->back, bus_mode);
> +}
> +
> +static int ad3552r_hs_set_target_io_mode_hs(struct ad3552r_hs_state *st)
> +{
> + int mode_target;
u32 would be more natural for register value. Or make an enum for this.
> +
> + /*
> + * Best access for secondary reg area, QSPI where possible,
> + * else as DSPI.
> + */
> + if (st->model_data->num_spi_data_lanes == 4)
> + mode_target = AD3552R_QUAD_SPI;
> + else
> + mode_target = AD3552R_DUAL_SPI;
> +
> + /*
> + * Better to not use update here, since generally it is already
> + * set as DDR mode, and it's not possible to read in DDR mode.
> + */
> + return st->data->bus_reg_write(st->back,
> + AD3552R_REG_ADDR_TRANSFER_REGISTER,
> + FIELD_PREP(AD3552R_MASK_MULTI_IO_MODE,
> + mode_target) |
> + AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> +}
> +
> static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct ad3552r_hs_state *st = iio_priv(indio_dev);
> @@ -132,6 +200,11 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
> return -EINVAL;
> }
>
> + /*
> + * With ad3541/2r support, QSPI pin is held low at reset from HDL,
> + * streaming start sequence must respect strictly the order below.
> + */
> +
> /* Primary region access, set streaming mode (now in SPI + SDR). */
> ret = ad3552r_qspi_update_reg_bits(st,
> AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> @@ -139,48 +212,106 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
> if (ret)
> return ret;
>
> - ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
> + /*
> + * Set target loop len, 0x2c 0r 0x2a, descending loop, and keeping loop
Not sure what 0x2c and 0x2a mean, so comment could be improved. And 0r looks
like a typo.
> + * len value so it's not cleared hereafter when enabling streaming mode
> + * (cleared by CS_ up).
> + */
> + ret = ad3552r_qspi_update_reg_bits(st,
> + AD3552R_REG_ADDR_TRANSFER_REGISTER,
> + AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE,
> + AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> + if (ret)
> + goto exit_err_streaming;
> +
> + ret = st->data->bus_reg_write(st->back,
> + AD3552R_REG_ADDR_STREAM_MODE,
> loop_len, 1);
> if (ret)
> - return ret;
> + goto exit_err_streaming;
>
> - /* Inform DAC chip to switch into DDR mode */
> - ret = ad3552r_qspi_update_reg_bits(st,
> - AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> - AD3552R_MASK_SPI_CONFIG_DDR,
> - AD3552R_MASK_SPI_CONFIG_DDR, 1);
> + /* Setting DDR now, caching current config_d. */
> + ret = st->data->bus_reg_read(st->back,
> + AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> + &st->config_d, 1);
> if (ret)
> - return ret;
> + goto exit_err_streaming;
> +
> + st->config_d |= AD3552R_MASK_SPI_CONFIG_DDR;
> + ret = st->data->bus_reg_write(st->back,
> + AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> + st->config_d, 1);
> + if (ret)
> + goto exit_err_streaming;
>
> - /* Inform DAC IP to go for DDR mode from now on */
> ret = iio_backend_ddr_enable(st->back);
> - if (ret) {
> - dev_err(st->dev, "could not set DDR mode, not streaming");
> - goto exit_err;
> - }
> + if (ret)
> + goto exit_err_ddr_mode_target;
> +
> + /*
> + * From here onward mode is DDR, so reading any register is not possible
> + * anymore, including calling "ad3552r_qspi_update_reg_bits" function.
> + */
> +
> + /* Set target to best high speed mode (D or QSPI). */
> + ret = ad3552r_hs_set_target_io_mode_hs(st);
> + if (ret)
> + goto exit_err_ddr_mode;
> +
> + /* Set bus to best high speed mode (D or QSPI). */
> + ret = ad3552r_hs_set_bus_io_mode_hs(st);
> + if (ret)
> + goto exit_err_bus_mode_target;
>
> + /*
> + * Backend setup must be done now only, or related register values will
> + * be disrupted by previous bus accesses.
> + */
> ret = iio_backend_data_transfer_addr(st->back, val);
> if (ret)
> - goto exit_err;
> + goto exit_err_bus_mode_target;
>
> ret = iio_backend_data_format_set(st->back, 0, &fmt);
> if (ret)
> - goto exit_err;
> + goto exit_err_bus_mode_target;
>
> ret = iio_backend_data_stream_enable(st->back);
> if (ret)
> - goto exit_err;
> + goto exit_err_bus_mode_target;
>
> return 0;
>
> -exit_err:
> - ad3552r_qspi_update_reg_bits(st,
> - AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> - AD3552R_MASK_SPI_CONFIG_DDR,
> - 0, 1);
> +exit_err_bus_mode_target:
> + /* Back to simple SPI, not using update to avoid read. */
> + st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_TRANSFER_REGISTER,
> + FIELD_PREP(AD3552R_MASK_MULTI_IO_MODE,
> + AD3552R_SPI) |
> + AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
> +
> + /*
> + * Back bus to simple SPI, this must be executed together with above
> + * target mode unwind, and can be done only after it.
> + */
> + st->data->bus_set_io_mode(st->back, AD3552R_IO_MODE_SPI);
>
> +exit_err_ddr_mode:
> iio_backend_ddr_disable(st->back);
>
> +exit_err_ddr_mode_target:
> + /*
> + * Back to SDR. In DDR we cannot read, whatever the mode is, so not
> + * using update.
> + */
> + st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
> + FIELD_PREP(AD3552R_MASK_SDO_DRIVE_STRENGTH, 1),
> + 1);
Should be using st->config_d & ~AD3552R_MASK_SPI_CONFIG_DDR here instead of
hard-coding FIELD_PREP(AD3552R_MASK_SDO_DRIVE_STRENGTH, 1).
> +
> +exit_err_streaming:
> + /* Back to single instruction mode, disabling loop. */
> + st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> + AD3552R_MASK_SINGLE_INST |
> + AD3552R_MASK_SHORT_INSTRUCTION, 1);
> +
> return ret;
> }
>
Powered by blists - more mailing lists