Re: [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier

Gwendal Grignou
2021-12-17T20:24:23Z

On Thu, Dec 16, 2021 at 7:54 AM Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:
>
> On Sun, 12 Dec 2021 18:40:54 -0800
> Gwendal Grignou <gwendal@chromium.org> wrote:
>
> > Add modifier IIO_MOD_REFERENCE for reporting sx9360 reference
> > proximity measurement.
> > All modifier must be defined for libiio to recognize
> > |in_proximity_reference| as a channel.
> >
> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Hmm. So the question is whether this is a valid modifier.
>
> I'm not totally convinced, because I can see we might well
> get stacking cases say
>
> iio_concentration_o2_reference
>
> However we do have precedence with 'ambient' which applies
> to temperature sensors.
>
> The alternative here would be to have it as a normal indexed
> channel but with a label saying it is the reference.
>
> Would that work for this case? If I were doing the ambient
> case again I'd use label for that as well, but label is a more
> recent addition to the ABI.
That would work, I am submitting a v4 without IIO_MOD_REFERENCE.

Gwendal.
>
> > ---
> > New in v3.
> >
> > drivers/iio/industrialio-core.c | 1 +
> > include/uapi/linux/iio/types.h | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 20d5178ca0739a..2b272f54de8ae9 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -134,6 +134,7 @@ static const char * const iio_modifier_names[] = {
> > [IIO_MOD_ETHANOL] = "ethanol",
> > [IIO_MOD_H2] = "h2",
> > [IIO_MOD_O2] = "o2",
> > + [IIO_MOD_REFERENCE] = "reference",
> > };
> >
> > /* relies on pairs of these shared then separate */
> > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> > index 48c13147c0a870..aa83a9b578502a 100644
> > --- a/include/uapi/linux/iio/types.h
> > +++ b/include/uapi/linux/iio/types.h
> > @@ -95,6 +95,7 @@ enum iio_modifier {
> > IIO_MOD_ETHANOL,
> > IIO_MOD_H2,
> > IIO_MOD_O2,
> > + IIO_MOD_REFERENCE,
> > };
> >
> > enum iio_event_type {
>