Re: [PATCH v8 v8 11/11] iio: adc: hx711: add support for HX710B

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect scale returned for the HX710B supply monitor channel (channel 2).
--

commit 7c73b66947438720fd9e5f95112049924938a086
Author: Piyush Patle <piyushpatle228@gmail.com>

iio: adc: hx711: add support for HX710B
    
Add support for the AVIA HX710B ADC, which shares the HX711 GPIO interface but
uses trailing PD_SCK pulses to select the active mode. Model the HX710B with
variant-specific channel tables and IIO info, and track the active channel
across conversions.

> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index 8ec4dee1f451e..49c14489d5ec4 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c
> @@ -463,6 +502,10 @@ static const struct iio_info hx711_iio_info = {
>  	.attrs			= &hx711_attribute_group,
>  };
>  
> +static const struct iio_info hx710b_iio_info = {
> +	.read_raw		= hx711_read_raw,
> +};
> +
[ ... ]
> @@ -495,6 +538,68 @@ static const struct iio_chan_spec hx711_chan_spec[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(2),
>  };
>  
> +/*
> + * HX710B channels (Table 3 in datasheet).
> + * 25 pulses (1 trailing): differential input, 10 SPS -> channel 0
> + * 26 pulses (2 trailing): DVDD-AVDD supply monitor, 40 SPS -> channel 2
> + * 27 pulses (3 trailing): differential input, 40 SPS -> channel 3
> + * .address stores the trailing pulse count for hx711_set_hx710b_channel().
> + * Channel 2 is used for the supply monitor to avoid aliasing the
> + * channel2 terminal of the first differential pair.
> + */
> +static const struct iio_chan_spec hx710b_chan_spec[] = {
[ ... ]
> +	{
> +		.type = IIO_VOLTAGE,
> +		.channel = 2,
> +		.indexed = 1,
> +		.address = 2,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 1,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 24,
> +			.storagebits = 32,
> +			.endianness = IIO_CPU,
> +		},
> +	},

Could this regression result in an incorrect scale being returned for the
HX710B supply monitor channel?

The driver defines BIT(IIO_CHAN_INFO_SCALE) in info_mask_separate for the
HX710B channels, advertising a separate scale per channel.

However, looking at hx711_read_raw(), when mask == IIO_CHAN_INFO_SCALE the
chan parameter is ignored and the scale is unconditionally returned using
hx711_data->gain_set:

    case IIO_CHAN_INFO_SCALE:
        *val = 0;
        mutex_lock(&hx711_data->lock);

        *val2 = hx711_get_gain_to_scale(hx711_data->gain_scale,
                        hx711_data->gain_set);

For the HX710B, hx711_data->gain_set is fixed at 128.

Because channel 2 is a DVDD-AVDD supply monitor, it typically bypasses the
PGA and has an effective gain of 1 rather than 128.

Returning the scale for a gain of 128 causes userspace to calculate a voltage
that is 128 times too small.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511174342.123820-1-piyushpatle228@gmail.com?part=11