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

[Jonathan Cameron]

On Mon, 25 May 2026 16:21:27 +0530
Piyush Patle <piyushpatle228@xxxxxxxxx> wrote:

> 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.
> 
> Also update the adjacent Kconfig text, file header, and module
> description so the driver text matches the newly supported variant.
> 
> Signed-off-by: Piyush Patle <piyushpatle228@xxxxxxxxx>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
Hi

A few small things in here seeing as you are going to be doing a v11 for
the patch description updates on the dt patches.

> ---
>  drivers/iio/adc/Kconfig |   8 +-
>  drivers/iio/adc/hx711.c | 230 ++++++++++++++++++++++++++++++++++++----
>  2 files changed, 212 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index a9dedbb8eb46..38ba8cae13f1 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -791,13 +791,15 @@ config HI8435
>  	  called hi8435.
>  
>  config HX711
> -	tristate "AVIA HX711 ADC for weight cells"
> +	tristate "AVIA HX711 and compatible ADCs"
As below - and similar (not and compatible because they aren't quite).
>  	depends on GPIOLIB
>  	select IIO_BUFFER
>  	select IIO_TRIGGERED_BUFFER
>  	help
> -	  If you say yes here you get support for AVIA HX711 ADC which is used
> -	  for weigh cells
> +	  If you say Y here you get support for the following AVIA ADCs:
> +	    - HX710B
> +	    - HX711
> +	  which are used for bridge sensors such as weigh cells.
>  
>  	  This driver uses two GPIOs, one acts as the clock and controls the
>  	  channel selection and gain, the other one is used for the measurement
> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index fde95758b9a0..32a35f644834 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - * HX711: analog to digital converter for weight sensor module
> + * HX711 and compatible ADCs driver for weigh sensor modules

Never use 'and compatible' when the driver needs to handle things differently
for each part.  'and similar' is the most common weasel words for this.

> +
> +/*
> + * Switch the HX710B to the requested channel for the next conversion.
> + * channel_set tracks the trailing pulse count that was last sent to the
> + * chip; it is updated immediately after hx711_read() commits the new
> + * mode in hardware, before waiting for the chip to be ready again.
> + */
> +static int hx711_set_hx710b_channel(struct hx711_data *hx711_data,
> +				    const struct iio_chan_spec *chan)
> +{
> +	int trailing_pulses = hx710b_trailing_pulses(hx711_data, chan);
> +	int ret;
> +
> +	if (hx711_data->channel_set == trailing_pulses)
> +		return 0;
> +
> +	ret = hx711_read(hx711_data, trailing_pulses);
> +	if (ret < 0)
> +		return ret;
> +
> +	hx711_data->channel_set = trailing_pulses;
> +
> +	ret = hx711_wait_for_ready(hx711_data);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

	return hx711_wait_for_read(hx711_data);
doesn't obviously hurt readability here and saves us a few lines of code.

> +}
> +
>  static int hx711_reset_read(struct hx711_data *hx711_data,
>  			    const struct iio_chan_spec *chan)
>  {
> +	const struct hx711_chip_info *info = hx711_data->chip_info;
>  	int trailing_pulses;
>  	int ret;
>  
> @@ -298,9 +352,16 @@ static int hx711_reset_read(struct hx711_data *hx711_data,
>  		return -EIO;
>  	}
>  
> -	ret = hx711_set_hx711_channel(hx711_data, chan, &trailing_pulses);
> -	if (ret < 0)
> -		return ret;
> +	if (info->has_fixed_gain) {
> +		ret = hx711_set_hx710b_channel(hx711_data, chan);
> +		if (ret < 0)
> +			return ret;
> +		trailing_pulses = hx710b_trailing_pulses(hx711_data, chan);
> +	} else {
> +		ret = hx711_set_hx711_channel(hx711_data, chan, &trailing_pulses);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	return hx711_read(hx711_data, trailing_pulses);
>  }
> @@ -322,6 +383,9 @@ static int hx711_read_raw(struct iio_dev *indio_dev,
>  		if (*val < 0)
>  			return *val;
>  		return IIO_VAL_INT;
> +	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 +463,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;
>  	}
>  
>  	iio_push_to_buffers_with_timestamp(indio_dev, &hx711_data->buffer,
> -							pf->timestamp);
> +					   pf->timestamp);

This an unrelated change so should not be in a patch doing anything
significant like this one is.

>  
> +out_unlock:
>  	mutex_unlock(&hx711_data->lock);
>  
>  	iio_trigger_notify_done(indio_dev->trig);
> @@ -462,6 +532,54 @@ static const struct iio_info hx711_iio_info = {
>  	.attrs			= &hx711_attribute_group,
>  };