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