linux-kernel - Re: [PATCH] iio: addac: ad74413r: fix Current Input, Loop Powered Mode

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230304171542.642d88e3@jic23-huawei>
Date:   Sat, 4 Mar 2023 17:15:42 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Cosmin Tanislav <cosmin.tanislav@...log.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: addac: ad74413r: fix Current Input, Loop Powered
 Mode

On Wed,  1 Mar 2023 12:55:11 +0100
Rasmus Villemoes <linux@...musvillemoes.dk> wrote:

> Currently, the driver handles CH_FUNC_CURRENT_INPUT_LOOP_POWER and
> CH_FUNC_CURRENT_INPUT_EXT_POWER completely identically. But that's not
> correct. In order for CH_FUNC_CURRENT_INPUT_LOOP_POWER to work, two
> changes must be made:
> 
> (1) expose access to the DAC_CODE_x register so that the intended
> output current can be set, i.e. expose the channel as both current
> output and current input, and
> 
> (2) per the data sheet
> 
>   When selecting the current input loop powered function, tie the
>   VIOUTN_x pin to ground via the on-chip 200 kΩ resistor by enabling
>   the CH_200K_TO_GND bit in the ADC_CONFIGx registers.
> 
> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>
> ---
> There's of course also CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART which is
> likely to require a similar fix, but as I don't have a ad74413r I
> can't test that. 

Looks correct to me based on the "current input, loop powered" section
of the datasheet, but want to leave a little more time for someone
familiar with the part to comment.

Given me a poke if I seem to have lost this in 2 weeks.  (That happens
less now I track with patchwork, but you never know ;)

Thanks,

Jonathan

> 
>  drivers/iio/addac/ad74413r.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> index f32c8c2fb26d..f5d072092709 100644
> --- a/drivers/iio/addac/ad74413r.c
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -99,6 +99,7 @@ struct ad74413r_state {
>  #define AD74413R_REG_ADC_CONFIG_X(x)		(0x05 + (x))
>  #define AD74413R_ADC_CONFIG_RANGE_MASK		GENMASK(7, 5)
>  #define AD74413R_ADC_CONFIG_REJECTION_MASK	GENMASK(4, 3)
> +#define AD74413R_ADC_CONFIG_CH_200K_TO_GND	BIT(2)
>  #define AD74413R_ADC_RANGE_10V			0b000
>  #define AD74413R_ADC_RANGE_2P5V_EXT_POW		0b001
>  #define AD74413R_ADC_RANGE_2P5V_INT_POW		0b010
> @@ -424,9 +425,20 @@ static int ad74413r_set_channel_dac_code(struct ad74413r_state *st,
>  static int ad74413r_set_channel_function(struct ad74413r_state *st,
>  					 unsigned int channel, u8 func)
>  {
> -	return regmap_update_bits(st->regmap,
> +	int ret;
> +
> +	ret = regmap_update_bits(st->regmap,
>  				  AD74413R_REG_CH_FUNC_SETUP_X(channel),
>  				  AD74413R_CH_FUNC_SETUP_MASK, func);
> +	if (ret)
> +		return ret;
> +
> +	if (func == CH_FUNC_CURRENT_INPUT_LOOP_POWER)
> +		ret = regmap_set_bits(st->regmap,
> +				      AD74413R_REG_ADC_CONFIG_X(channel),
> +				      AD74413R_ADC_CONFIG_CH_200K_TO_GND);
> +
> +	return ret;
>  }
>  
>  static int ad74413r_set_adc_conv_seq(struct ad74413r_state *st,
> @@ -1112,6 +1124,11 @@ static struct iio_chan_spec ad74413r_current_input_channels[] = {
>  	AD74413R_ADC_CURRENT_CHANNEL,
>  };
>  
> +static struct iio_chan_spec ad74413r_current_input_loop_channels[] = {
> +	AD74413R_DAC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE)),
> +	AD74413R_ADC_CURRENT_CHANNEL,
> +};
> +
>  static struct iio_chan_spec ad74413r_resistance_input_channels[] = {
>  	AD74413R_ADC_CHANNEL(IIO_RESISTANCE, BIT(IIO_CHAN_INFO_PROCESSED)),
>  };
> @@ -1135,7 +1152,7 @@ static const struct ad74413r_channels ad74413r_channels_map[] = {
>  	[CH_FUNC_CURRENT_OUTPUT] = AD74413R_CHANNELS(current_output),
>  	[CH_FUNC_VOLTAGE_INPUT] = AD74413R_CHANNELS(voltage_input),
>  	[CH_FUNC_CURRENT_INPUT_EXT_POWER] = AD74413R_CHANNELS(current_input),
> -	[CH_FUNC_CURRENT_INPUT_LOOP_POWER] = AD74413R_CHANNELS(current_input),
> +	[CH_FUNC_CURRENT_INPUT_LOOP_POWER] = AD74413R_CHANNELS(current_input_loop),
>  	[CH_FUNC_RESISTANCE_INPUT] = AD74413R_CHANNELS(resistance_input),
>  	[CH_FUNC_DIGITAL_INPUT_LOGIC] = AD74413R_CHANNELS(digital_input),
>  	[CH_FUNC_DIGITAL_INPUT_LOOP_POWER] = AD74413R_CHANNELS(digital_input),