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

[Piyush Patle]

On Tue, May 12, 2026 at 6:08 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Mon, 11 May 2026 23:13:36 +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 Piyush,
>
> A few things I'd missed that Sashiko mentioned.
> Note that most of what it calls out in v7 of this patch is wrong because
> of the whole active_scan_mask / timestamp bit being set or not thing.
>
> The channel representation is indeed odd and I think that bit needs
> a rethink unfortunately.  I'd failed to notice it wasn't just two different
> channels but instead is one physical set of inputs measured at different
> sampling rates.
>
>
> > @@ -403,16 +442,16 @@ static irqreturn_t hx711_trigger(int irq, void *p)
> >
> >       mutex_lock(&hx711_data->lock);
> >
> > -     memset(&hx711_data->buffer, 0, sizeof(hx711_data->buffer));
> > +     memset(hx711_data->buffer, 0, sizeof(hx711_data->buffer));
> >
> >       iio_for_each_active_channel(indio_dev, i) {
> > -             hx711_data->buffer.channel[j] =
> > +             hx711_data->buffer[j] =
> >                       hx711_reset_read(hx711_data, &indio_dev->channels[i]);
>
> Sashiko pointed out (v7 review) that this can return an error. We should
> really be checking for negative values and if they occur don't push data
> to the buffer.  Given this is unlikely to happen except when the device
> is very broken, a rate limited dev_err is usual way to report this then
> carry on without calling iio_push_to_buffers_with_timestamp().
Agreed. I missed that.
In v9 I will make hx711_trigger() check hx711_reset_read() and skip
iio_push_to_buffers_with_timestamp() if any channel read fails.
>
> >               j++;
> >       }
> >
> > -     iio_push_to_buffers_with_timestamp(indio_dev, &hx711_data->buffer,
> > -                                                     pf->timestamp);
> > +     iio_push_to_buffers_with_timestamp(indio_dev, hx711_data->buffer,
> > +                                        pf->timestamp);
> >
> >       mutex_unlock(&hx711_data->lock);
> >
> > @@ -463,6 +502,10 @@ static const struct iio_info hx711_iio_info = {
> >       .attrs                  = &hx711_attribute_group,
> >  };
>
> > +/*
> > + * HX710B channels (Table 3 in datasheet).
> > + * 25 pulses (1 trailing): differential input, 10 SPS -> channel 0
> > + * 26 pulses (2 trailing): DVDD-AVDD supply monitor, 40 SPS -> channel 2
> > + * 27 pulses (3 trailing): differential input, 40 SPS -> channel 3
>
> I'd missed this previously but sashiko raised a question on it.
> Why are we representing the same physical input channel as two different IIO channels
> based only on the sampling rate?  That doesn't seem to make a lot of sense.
> Should be one channel with a sampling_frequency control.
>

Yes, you are right. I should not expose the 10 SPS and 40 SPS
differential modes as separate IIO channels.
In v9 I will model HX710B as two physical channels:
- voltage0-voltage1: differential input, with selectable sampling
frequency of 10 or 40 SPS
- voltage2: DVDD-AVDD supply monitor, fixed at 40 SPS
The differential channel will get IIO_CHAN_INFO_SAMP_FREQ. The driver
will map 10 SPS to one trailing pulse and 40 SPS to three trailing
pulses at read time. The supply monitor will always use two trailing
pulses.

I will also update channel_set immediately after hx711_read() succeeds,
because that is when the trailing pulses have been sent and the hardware
state has actually changed.

Will send v9 with these fixes.
>
> > + * .address stores the trailing pulse count for hx711_set_hx710b_channel().
> > + * Channel 2 is used for the supply monitor to avoid aliasing the
> > + * channel2 terminal of the first differential pair.
> > + */
> > +static const struct iio_chan_spec hx710b_chan_spec[] = {
> > +     {
> > +             .type = IIO_VOLTAGE,
> > +             .differential = 1,
> > +             .channel = 0,
> > +             .channel2 = 1,
> > +             .indexed = 1,
> > +             .address = 1,
> > +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +                                   BIT(IIO_CHAN_INFO_SCALE),
> > +             .scan_index = 0,
> > +             .scan_type = {
> > +                     .sign = 'u',
> > +                     .realbits = 24,
> > +                     .storagebits = 32,
> > +                     .endianness = IIO_CPU,
> > +             },
> > +     },
> > +     {
> > +             .type = IIO_VOLTAGE,
> > +             .channel = 2,
> > +             .indexed = 1,
> > +             .address = 2,
> > +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +                                   BIT(IIO_CHAN_INFO_SCALE),
> > +             .scan_index = 1,
> > +             .scan_type = {
> > +                     .sign = 'u',
> > +                     .realbits = 24,
> > +                     .storagebits = 32,
> > +                     .endianness = IIO_CPU,
> > +             },
> > +     },
> > +     {
> > +             .type = IIO_VOLTAGE,
> > +             .differential = 1,
> > +             .channel = 3,
> > +             .channel2 = 4,
> > +             .indexed = 1,
> > +             .address = 3,
> > +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +                                   BIT(IIO_CHAN_INFO_SCALE),
> > +             .scan_index = 2,
> > +             .scan_type = {
> > +                     .sign = 'u',
> > +                     .realbits = 24,
> > +                     .storagebits = 32,
> > +                     .endianness = IIO_CPU,
> > +             },
> > +     },
> > +     IIO_CHAN_SOFT_TIMESTAMP(3),
> > +};
>