Re: [PATCH v2 2/7] iio:accel:stk8312: check for invalid value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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
>> --
>> 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
>>
>
--
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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux