> -----Original Message----- > From: Jonathan Cameron [mailto:jic23@xxxxxxxxxx] > Sent: Saturday, August 8, 2015 2:18 PM > To: Hartmut Knaack; Crt Mori > Cc: linux-iio@xxxxxxxxxxxxxxx; Lars-Peter Clausen; Peter Meerwald; Breana, > Tiberiu A > Subject: Re: [PATCH v2 2/7] iio:accel:stk8312: check for invalid value > > On 06/08/15 23:23, Hartmut Knaack wrote: > > Crt Mori schrieb am 01.08.2015 um 09:35: > >> On 1 August 2015 at 00:54, Hartmut Knaack <knaack.h@xxxxxx> wrote: > >>> Crt Mori schrieb am 30.07.2015 um 09:09: > >>>> On 29 July 2015 at 23:39, Hartmut Knaack <knaack.h@xxxxxx> wrote: > >>>>> Revision 1.2 of the datasheet recommends on page 22 to only write > >>>>> non-zero values read from OTP register 0x70 into AFECTRL register. > >>>>> > >>>>> Signed-off-by: Hartmut Knaack <knaack.h@xxxxxx> > >>>>> Reviewed-by: Tiberiu Breana <tiberiu.a.breana@xxxxxxxxx> > >>>>> --- > >>>>> drivers/iio/accel/stk8312.c | 2 ++ > >>>>> 1 file changed, 2 insertions(+) > >>>>> > >>>>> diff --git a/drivers/iio/accel/stk8312.c > >>>>> b/drivers/iio/accel/stk8312.c index c2bd1444d6da..6592be8e6377 > >>>>> 100644 > >>>>> --- a/drivers/iio/accel/stk8312.c > >>>>> +++ b/drivers/iio/accel/stk8312.c > >>>>> @@ -150,6 +150,8 @@ static int stk8312_otp_init(struct stk8312_data > *data) > >>>>> goto exit_err; > >>>>> > >>>>> ret = i2c_smbus_read_byte_data(client, > >>>>> STK8312_REG_OTPDATA); > >>>>> + if (ret == 0) > >>>>> + ret = -EINVAL; > >>>> This seems fishy. We have a macro value written to client which > >>>> cannot really give us EINVAL, except if we are checking the client, > >>>> but then this would only fail if we had some other i2c device on > >>>> the line with stk8312 address, which would ack other i2c commands > above this. > >>> > >>> I'm not sure to really understand your point, or if you actually > >>> understood this patch. I'm not totally happy with EINVAL as error > >>> code, but it just seemed to fit better than any other possible error > >>> code. Of course I would be happy to get recommendations for better > fitting codes. > >>> A short explanation why this is done: in the sample code provided in > >>> the data sheet, it is mentioned that if the value read from that > >>> register 0x70 is zero, then it should not be written into the > >>> AFECTRL register. So, that's what I have addressed here. > >> EINVAL is more like bad user input, so if this fails because of the > >> last command then it is correct, otherwise I would put more like > >> -EBUSY or -EACCES > >> > > > > Tiberiu, I'd like to get your opinion on these alternative error > > codes. Do you have any information why the read value could be zero > > and why that AFECTRL register should not be set to zero? > > Thanks, > > > > Hartmut I don't know why the read value could be 0. The only information I have is the datasheet, same as you. I think EINVAL is fine; EBUSY would also make sense. It's up to you. Tiberiu > I already applied version 1 of this patch, so send an follow up if the decision is > that there is a better error code. > > J > > > >> Thank you for the explanation. > >> > >>> > >>>>> if (ret < 0) > >>>>> goto exit_err; > >>>>> > >>>>> -- > >>>>> 2.4.6 > >>>>> > >>>>> -- > >>>>> To unsubscribe from this list: send the line "unsubscribe > >>>>> linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx > >>>>> More majordomo info at http://vger.kernel.org/majordomo- > info.html ��.n��������+%������w��{.n�����{��(��)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
