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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The triggered buffer loop unconditionally performs physical hardware reads for all active channels, including the IIO soft timestamp channel. For the HX711, `hx711_reset_read()` returns `-EINVAL` for the timestamp channel; this patch introduces a new `if (ret < 0)` check that catches this error, prints a failure, and aborts the trigger entirely, completely breaking buffer capture when the timestamp is enabled. For the newly added HX710B, the driver incorrectly interprets the timestamp channel as having `differential = 0`, performing a ~100ms blocking dummy hardware read and then writing the result to `hx711_data->buffer.channel[j++]` at index 2, which violates the `u32 channel[2]` array bounds and risks UBSAN kernel panics.

Pre-existing issues:
- [High] The driver allows `IIO_CHAN_INFO_RAW` sysfs reads to proceed concurrently with triggered buffer captures due to a missing `iio_device_claim_direct()` call, which causes hardware state machine corruption and data loss.
--

commit 4a57ad17002849c3ffed8137401cf0ed7fd81778
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,
track the active channel across conversions, and use the fixed gain
value when computing scale.

> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index 5c326f01c5543..e90780dfd34e5 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c

[ ... ]

> @@ -322,6 +379,9 @@ static int hx711_read_raw(struct iio_dev *indio_dev,
>  		if (*val < 0)
>  			return *val;
>  		return IIO_VAL_INT;

[Severity: High]
This is a pre-existing issue, but does hx711_read_raw() allow
IIO_CHAN_INFO_RAW sysfs reads to proceed concurrently with triggered
buffer captures?

Since there is no iio_device_claim_direct() call to serialize against
the active IIO buffer, a concurrent sysfs read might acquire the lock
between trigger samples.

Because the hardware relies on the trailing pulses of the previous read
to set the channel and gain for the next conversion, could an interleaved
sysfs read clock out data intended for the buffer and reset the hardware,
corrupting the data stream for userspace?

> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = hx711_data->samp_freq;
> +		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 0;
>  		mutex_lock(&hx711_data->lock);

[ ... ]

> @@ -399,20 +459,26 @@ static irqreturn_t hx711_trigger(int irq, void *p)
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct hx711_data *hx711_data = iio_priv(indio_dev);
>  	int i, j = 0;
> +	int ret;
>  
>  	mutex_lock(&hx711_data->lock);
>  
>  	memset(&hx711_data->buffer, 0, sizeof(hx711_data->buffer));
>  
>  	iio_for_each_active_channel(indio_dev, i) {
> -		hx711_data->buffer.channel[j] =
> -			hx711_reset_read(hx711_data, &indio_dev->channels[i]);
> -		j++;
> +		ret = hx711_reset_read(hx711_data, &indio_dev->channels[i]);
> +		if (ret < 0) {
> +			dev_err_ratelimited(hx711_data->dev,
> +					    "read failed: %d\n", ret);
> +			goto out_unlock;
> +		}
> +		hx711_data->buffer.channel[j++] = ret;

[Severity: High]
Does this new error check accidentally cause a regression by aborting the
trigger entirely when the IIO soft timestamp channel is enabled?

iio_for_each_active_channel() iterates over the active_scan_mask, which
includes the timestamp channel.

For the HX711, it appears hx711_reset_read() will return -EINVAL for the
timestamp channel. Would this new check catch that and completely abort
the capture without pushing data?

Furthermore, for the newly added HX710B, hx710b_trailing_pulses() seems
to evaluate !chan->differential as true for the timestamp channel. Would
this result in a hardware read that writes to hx711_data->buffer.channel[2],
overflowing the bounds of the u32 channel[2] array?

>  	}
>  
>  	iio_push_to_buffers_with_timestamp(indio_dev, &hx711_data->buffer,
>  							pf->timestamp);
>  
> +out_unlock:
>  	mutex_unlock(&hx711_data->lock);

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