Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
In-Reply-To: <20220322203844.0000466f@Huawei.com>
References: <20220318204808.3404542-1-swboyd@chromium.org> <20220319152641.49d8b3e1@jic23-huawei>
 <CAE-0n52jDZz0qKhfg8OWVDmDg5+xXo-qSL3jNka82QHwA2-xsw@mail.gmail.com> <20220322203844.0000466f@Huawei.com>
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.10
Date:   Tue, 22 Mar 2022 17:57:26 -0400
Message-ID: <CAE-0n51=ewQnTctWsmvawb_p3CrE0JgJFwLy+FR6rUPkWmTgHw@mail.gmail.com>
Subject: Re: [PATCH] iio:proximity:sx9324: Fix hardware gain read/write
To:     Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc:     Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
        Gwendal Grignou <gwendal@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Quoting Jonathan Cameron (2022-03-22 13:38:44)
> On Mon, 21 Mar 2022 19:36:33 +0100
> Stephen Boyd <swboyd@chromium.org> wrote:
> > Quoting Jonathan Cameron (2022-03-19 08:26:41)
> > > On Fri, 18 Mar 2022 13:48:08 -0700
> > > Stephen Boyd <swboyd@chromium.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?