[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220327164734.4b678562@jic23-huawei>
Date: Sun, 27 Mar 2022 16:47:34 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Stephen Boyd <swboyd@...omium.org>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Lars-Peter Clausen <lars@...afoo.de>,
linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
Gwendal Grignou <gwendal@...omium.org>
Subject: Re: [PATCH] iio:proximity:sx9324: Fix hardware gain read/write
On Tue, 22 Mar 2022 17:57:26 -0400
Stephen Boyd <swboyd@...omium.org> wrote:
> Quoting Jonathan Cameron (2022-03-22 13:38:44)
> > On Mon, 21 Mar 2022 19:36:33 +0100
> > Stephen Boyd <swboyd@...omium.org> wrote:
> > > Quoting Jonathan Cameron (2022-03-19 08:26:41)
> > > > On Fri, 18 Mar 2022 13:48:08 -0700
> > > > Stephen Boyd <swboyd@...omium.org> wrote:
> > > > >
> > > > > diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
> > > > > index 0d9bbbb50cb4..a3c8e02f5a56 100644
> > > > > --- a/drivers/iio/proximity/sx9324.c
> > > > > +++ b/drivers/iio/proximity/sx9324.c
> > > > > @@ -379,7 +379,10 @@ static int sx9324_read_gain(struct sx_common_data *data,
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > - *val = 1 << FIELD_GET(SX9324_REG_PROX_CTRL0_GAIN_MASK, regval);
> > > > > + regval = FIELD_GET(SX9324_REG_PROX_CTRL0_GAIN_MASK, regval);
> > > > > + if (regval)
> > > >
> > > > If 0 is reserved then I'd return and error code here to indicate
> > > > we don't know what the gain is rather than carrying on regardless.
> > > > Or is this going to cause problems as it will be an ABI change (error
> > > > return possible when it wasn't really before)?
> > > >
> > >
> > > That sounds OK to me. The driver is only being introduced now so we can
> > > still fix it to reject a gain of 0. Unless 0 should mean "off", i.e.
> > > hardware gain of 1?
> > No. I don't think we want to add that sort of fiddly definition.
> > So error is the way to go - I'd forgotten we only just introduced this
> > so no ABI breakage risk.
> >
>
> Ok got it. Does the write_gain function also need to reject values
> greater than 8 and less than or equal to 0?
Ah. Yes, it should indeed as the value is coming from userspace
so we shouldn't trust it to be sensible.
Jonathan
Powered by blists - more mailing lists