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

Andy Shevchenko
2026-04-29T19:06:40Z

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