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