Re: [PATCH v2 2/2] iio: adc: hx711: add support for HX710B

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sun, Apr 19, 2026 at 11:16:40PM +0530, Piyush Patle wrote:
> The HX711 uses trailing SCK pulses after each 24-bit conversion to
> select the channel and gain for the next measurement: 1 pulse gives
> channel A at gain 128, 2 pulses give channel B at gain 32, and 3 pulses
> give channel A at gain 64.
> 
> The HX710B works differently: gain is fixed at 128 and the trailing
> pulses select only the channel. One trailing pulse selects the
> differential input (channel 0, 10 SPS) and two trailing pulses select
> the DVDD-AVDD supply monitor (channel 1, 40 SPS).
> 
> Refactor the driver around a per-chip hx711_chip_info structure so both
> variants can share the same core. Each chip provides its own
> iio_chan_spec array and iio_info pointer. The HX710B stores per-channel
> trailing pulse counts in chan->address (1 for channel 0, 2 for
> channel 1) instead of a separate array. A bool fixed_gain flag and
> fixed_gain_val field in hx711_chip_info distinguish the fixed-gain path
> from the HX711's user-selectable gain path without conflating unrelated
> properties. The HX710B differential input channel is described with
> .differential=1 and .channel2=1 as required by the IIO ABI.
> 
> Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
> ---
> Changes in v2:
> - Fix pulse count bug: hx710b values were {25, 26} (total SCK cycles);
>   corrected to {1, 2} (trailing-only, hx711_read() clocks 24 data bits)

Which questions how it was tested...

> - Add .differential = 1 and .channel2 = 1 to HX710B channel 0
> - Move trailing pulse counts from a separate array to chan->address
>   (1 for ch0, 2 for ch1); remove chan_pulse_count / num_chan_pulses
> - Replace chan_pulse_count != NULL tests with dedicated bool fixed_gain
>   flag; add fixed_gain_val field to hx711_chip_info
> - Add const struct iio_info *iio_info to hx711_chip_info; probe sets
>   indio_dev->info = chip_info->iio_info directly
> - Remove NULL check after device_get_match_data()
> - Remove reset_channel from hx711_chip_info (always 0; use literal)
> - Change hx711_reset_read() and hx710b_set_channel() to take
>   const struct iio_chan_spec * instead of int chan
> - Revert hx711_data struct member alignment noise
> - Restore trailing blank line at end of file (unrelated removal reverted)
> - Sort of_device_id entries alphabetically (hx710b before hx711)
> - Expand commit message to explain HX711 vs HX710B trailing-pulse model
> - Restore file header to mention weight sensor modules

...

>  config HX711
> -	tristate "AVIA HX711 ADC for weight cells"
> +	tristate "AVIA HX711 and HX710B ADC"

This won't scale. Better to put something like "and compatible".
Also use plural "ADCs".

>  	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 yes here you get support for AVIA HX711 and HX710B ADCs
> +	  which are used for bridge sensors such as weigh cells.

Usually for the less churn in the future we list them one per line. This
will give easier understanding of what is supported (keep them also sorted).

	  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
>  	  data
>  
> +	  The HX710B is a variant with fixed gain and a different channel
> +	  selection scheme.
> +
>  	  Currently the raw value is read from the chip and delivered.
>  	  To get an actual weight one needs to subtract the
>  	  zero offset and multiply by a scale factor.

...

> - * HX711: analog to digital converter for weight sensor module
> + * HX711/HX710B: ADC driver for weight sensor modules

Use same as in Kconfig:

 * HX711 and compatible ADCs driver for weight sensor modules

 ...

> +/**
> + * struct hx711_chip_info - per-variant static configuration
> + * @name: IIO device name
> + * @channels: channel specification
> + * @num_channels: number of channels
> + * @iio_info: IIO info ops for this variant
> + * @fixed_gain: true if the variant has a fixed ADC gain (e.g. HX710B)
> + * @fixed_gain_val: the fixed gain value used to compute scale (when fixed_gain)
> + */
> +struct hx711_chip_info {
> +	const char			*name;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;

Why signed?

> +	const struct iio_info		*iio_info;
> +	bool				fixed_gain;
> +	int				fixed_gain_val;
> +};

...

>  struct hx711_data {
>  	struct device		*dev;
>  	struct gpio_desc	*gpiod_pd_sck;
>  	struct gpio_desc	*gpiod_dout;
>  	int			gain_set;	/* gain set on device */
>  	int			gain_chan_a;	/* gain for channel A */

> +	int			channel_set;	/* HX710B current channel */
> +	int			scale;		/* HX710B fixed scale */

Check if those need to be signed.

> +	const struct hx711_chip_info *chip_info;
>  	struct mutex		lock;

>  }

...

> -	for (i = 0; i < hx711_get_gain_to_pulse(hx711_data->gain_set); i++)
> +	for (i = 0; i < trailing_pulses; i++)

If 'i' is used only once here, it can be converted to

	for (unsigned int i = 0; i < trailing_pulses; i++)

>  		hx711_cycle(hx711_data);
>  
>  	return value;

...

> +	.num_channels	= ARRAY_SIZE(hx711_chan_spec),

You also probably want to revisit inclusion block. At least follow IWYU in the
code you added here, exempli gratia include array_size.h and types.h if not yet
included.

...

>  	hx711_data->clock_frequency = 400000;

Sounds like I2C or SD standard speed :-)

Not sure, but can be also converted to use 400 * HZ_PER_KHZ (from units.h)
in a separate change.

...

>  	hx711_data->data_ready_delay_ns =
>  				1000000000 / hx711_data->clock_frequency;

Side note, in a separate change you can use constants from time.h, id est
NSEC_PER_SEC.

...

>  static const struct of_device_id of_hx711_match[] = {
> -	{ .compatible = "avia,hx711", },
> +	{ .compatible = "avia,hx710b", .data = &hx710b_chip },

Please, split this to a separate change. So, first introduce the chip_info,
then add your device support using given infrastructure.

That said, you also can convert the driver to use guard()() from cleanup.h
in a separate change.


> +	{ .compatible = "avia,hx711",  .data = &hx711_chip  },
>  	{ }
>  };

...

> -MODULE_DESCRIPTION("HX711 bitbanging driver - ADC for weight cells");

Ah, this removes crucial information. So, this driver probably just needs to
use i2c_gpio?

> +MODULE_DESCRIPTION("HX711/HX710B GPIO ADC driver");

Use what is written in Kconfig.

-- 
With Best Regards,
Andy Shevchenko