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

[Jonathan Cameron <jic23@kernel.org>]

On Thu,  4 Jun 2026 00:18:59 +0530
Piyush Patle <piyushpatle228@gmail.com> 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@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Hi,

One small thing inline which is is kind of a side effect of cleaning up
the regulator description in earlier patches

Thanks,

Jonathan

> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index fde95758b9a0..447699e1d60b 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c


>  static int hx711_probe(struct platform_device *pdev)
>  {
>  	const struct hx711_chip_info *chip_info;
> @@ -543,32 +709,43 @@ static int hx711_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(hx711_data->gpiod_dout),
>  				     "failed to get dout-gpiod\n");
>  
> -	ret = devm_regulator_get_enable_read_voltage(dev, "avdd");
> +	if (chip_info->has_fixed_gain)
> +		ret = devm_regulator_get_enable_read_voltage(dev, "vref");
> +	else
> +		ret = devm_regulator_get_enable_read_voltage(dev, "avdd");
>  	if (ret < 0)
>  		return ret;

https://sashiko.dev/#/patchset/20260603184859.89693-1-piyushpatle228%40gmail.com

Sashiko calls this out (confusingly on patch 4 review). The binding
says we need avdd and dvdd as well. So we should be turning those on,
even if we don't want to read their voltages - the dvdd one is a bit
more arguable, but it is very odd to ignore avdd when the driver
is touching it anyway.

I think you need something like:


	if (chip_info->has_fixed_gain) {
		ret = devm_regulator_get_enable_read_voltage(dev, "vref");
	 	if (ret < 0)
	  		return ret;
		ret = devm_regulator_get_enable(dev, "avdd");
		if (ret < 0)
			return ret;
	} else {
		ret = devm_regulator_get_enable_read_voltage(dev, "avdd");
		if (ret < 0)
			return ret;
	}
	ret = devm_regulator_get_enable(dev, "dvdd");
	if (ret < 0)
		return ret;

For any that aren't provided in DT you'll get stub regulators as long as we
don't need to read the voltage.

Other than that the only thing I'd look into is the suggestion read_raw()
might clash with buffered reads and corrupt the capture.  I haven't checked that.

Thanks,

Jonathan