Re: [PATCH v2 6/9] iio: dac: ad3552r-hs: use instruction mode for configuration

[David Lechner <dlechner@baylibre.com>]

On 1/8/25 11:29 AM, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Use "instruction" mode over initial configuration and all other
> non-streaming operations.
> 
> DAC boots in streaming mode as default, and the driver is not
> changing this mode.
> 
> Instruction r/w is still working becouse instruction is processed

s/becouse/because/

> from the DAC after chip select is deasserted, this works until
> loop mode is 0 or greater than the instruction size.
> 
> All initial operations should be more safely done in instruction
> mode, a mode provided for this.

I'm not sure it is really "safer". The way I read the datasheet, this just
enables bulk reads of multiple registers. So unless we need to do bulk reads
it seems like this is just adding extra complexity to the driver without a
tangible benefit.

> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  drivers/iio/dac/ad3552r-hs.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 27949f207d42..991b11702273 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -132,6 +132,13 @@ static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
>  		return -EINVAL;
>  	}
>  
> +	/* Primary region access, set streaming mode (now in SPI + SDR). */
> +	ret = ad3552r_qspi_update_reg_bits(st,
> +					   AD3552R_REG_ADDR_INTERFACE_CONFIG_B,
> +					   AD3552R_MASK_SINGLE_INST, 0, 1);

Missing undoing this operation in the error path if a later operation in this
function fails?

> +	if (ret)
> +		return ret;
> +
>  	ret = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
>  				      loop_len, 1);
>  	if (ret)