linux-kernel - Re: [PATCH] iio: addac: ad74413r: update channel function set

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230913200944.086ec3c3@jic23-huawei>
Date:   Wed, 13 Sep 2023 20:09:44 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Antoniu Miclaus <antoniu.miclaus@...log.com>
Cc:     <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iio: addac: ad74413r: update channel function set

On Mon, 11 Sep 2023 17:29:50 +0300
Antoniu Miclaus <antoniu.miclaus@...log.com> wrote:

> According to the datasheet switching to a new channel function implies
> multiple steps.
> 
> All functions must be selected for a minimum of 130 μs before changing
> to another function.
> The DAC_CODEx registers are not reset by changing channel functions.
> Prior to changing channel functions, it is recommended to set the DAC
> code to 0x0000 via the DAC_CODEx registers. Set the channel function
> to high impedance via the CH_FUNC_SETUPx registers before transitioning
> to the new channel function. After the new channel function is configured,
> it is recommended to wait 150 μs before updating the DAC code.
> 
> Even though the channel switch is done at only at probe, where a device
> reset is performed, that does not guarantee that the steps prior
> changing to new channel function should be neglected.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>

Given description and lack of fixes tag, I'm not treating this as a fix.
Let me know if I should and provide a fixes tag in reply to this therad.

Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,

Jonathan
> ---
>  drivers/iio/addac/ad74413r.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> index 6b0e8218f150..7b9ebb481815 100644
> --- a/drivers/iio/addac/ad74413r.c
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -441,12 +441,30 @@ static int ad74413r_set_channel_function(struct ad74413r_state *st,
>  {
>  	int ret;
>  
> +	ret = regmap_update_bits(st->regmap,
> +				 AD74413R_REG_CH_FUNC_SETUP_X(channel),
> +				 AD74413R_CH_FUNC_SETUP_MASK,
> +				 CH_FUNC_HIGH_IMPEDANCE);
> +	if (ret)
> +		return ret;
> +
> +	/* Set DAC code to 0 prior to changing channel function */
> +	ret = ad74413r_set_channel_dac_code(st, channel, 0);
> +	if (ret)
> +		return ret;
> +
> +	/* Delay required before transition to new desired mode */
> +	usleep_range(130, 150);
> +
>  	ret = regmap_update_bits(st->regmap,
>  				  AD74413R_REG_CH_FUNC_SETUP_X(channel),
>  				  AD74413R_CH_FUNC_SETUP_MASK, func);
>  	if (ret)
>  		return ret;
>  
> +	/* Delay required before updating the new DAC code */
> +	usleep_range(150, 170);
> +
>  	if (func == CH_FUNC_CURRENT_INPUT_LOOP_POWER)
>  		ret = regmap_set_bits(st->regmap,
>  				      AD74413R_REG_ADC_CONFIG_X(channel),