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

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

 



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

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

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