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

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

On Wed, Apr 29, 2026 at 11:15:44AM +0530, Piyush Patle 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.

...

>  /*
> - * HX711: analog to digital converter for weight sensor module
> + * HX711 and compatible ADCs driver for weight sensor modules

As noticed in previous round, shouldn't we fix weight --> weigh?

>   *
>   * Copyright (c) 2016 Andreas Klinger <ak@it-klinger.de>
>   */

...

>  #include <linux/err.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>

> +#include <linux/types.h>

No, you clearly see the order. Please, preserve it.

>  #include <linux/mod_devicetable.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>

...

> + * @fixed_gain: true if the variant has a fixed ADC gain

> +	bool				fixed_gain;

In accordance with the kernel-doc, name the variable "has_fixed_gain".

...

>  	/*
>  	 * triggered buffer
> -	 * 2x32-bit channel + 64-bit naturally aligned timestamp
> +	 * up to 3x32-bit channels + pad + 64-bit naturally aligned timestamp
>  	 */
>  	struct {
> -		u32 channel[2];
> +		u32 channel[3];
> +		u32 pad;
>  		aligned_s64 timestamp;
>  	} buffer;

I think I still doesn't follow if this kind of changes affects ABI or not.
Jonathan, is it usually okay to inject more entries in this structure?

...

> +static int hx711_set_hx710b_channel(struct hx711_data *hx711_data,
> +				    const struct iio_chan_spec *chan)
> +{
> +	int ret;
> +
> +	if (hx711_data->channel_set == (unsigned int)chan->channel)

Why do we need explicit casting?

> +		return 0;
> +
> +	ret = hx711_read(hx711_data, chan->address);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = hx711_wait_for_ready(hx711_data);
> +	if (ret)
> +		return ret;
> +
> +	hx711_data->channel_set = chan->channel;
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko