Re: [PATCH] iio: adc: ti-ads8688: use read_avail for available attributes
From: Jonathan Cameron
Date: Wed Mar 25 2026 - 15:46:21 EST
On Tue, 24 Mar 2026 13:42:40 +0200
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> On Mon, Mar 23, 2026 at 09:56:33PM +0000, Gabriel Rondon wrote:
> > Convert the in_voltage_scale_available and in_voltage_offset_available
> > attributes from legacy IIO_DEVICE_ATTR with custom show functions to the
> > IIO framework's read_avail callback. This uses the framework's built-in
> > support for _available attributes, removing the need for manual sysfs
> > formatting.
> >
> > Precompute the available scale values at probe time since they depend on
> > the reference voltage which does not change after initialization.
>
> ...
>
> > +static const int ads8688_offset_avail[] = {
> > + -(1 << (ADS8688_REALBITS - 1)),
>
> This is fragile code (however it probably won't be exposed IRL)
> if ADS8688_REALBITS == 32 this is UB in accordance with C standard.
>
> Also the trick with -BIT(x) is harder to read than simple GENMASK().
> If ADS8688_REALBITS == 1, this becomes -1 (all ones), is it correct?
>
> > + 0,
Replying here because the follow up lost too much context.
This is a classic offset for bipolar ADC, which is - half the full
range binary value. So we won't hit the edge cases, but maybe
expressing it different will be clearer.
-(GENMASK(ADS8688_REALBITS - 1, 0) >> 1) does feel a bit overkill
but -GENMASK(ADS8688_REALBITS - 2, 0) feels under explained (much
like the current).
We 'could' use -(U16MAX >> 1) but that feels very specific to
it being 16 bits.
Maybe just leave this as it already is in the table the value
is copied from.