Re: [PATCHv1] Add support for Intersil ISL12057 I2C RTC chip

[arno-LkuqDEemtHBg9hUCZPvPmw@public.gmane.org (Arnaud Ebalard)]

Hi Mark,

Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:

>> +static int isl12057_i2c_read_regs(struct i2c_client *client, u8 reg, u8 buf[],
>> +                                 unsigned len)
>> +{
>> +       struct i2c_msg msgs[2] = {
>> +               {
>> +                       .addr = client->addr,
>> +                       .flags = 0,
>> +                       .len = sizeof(u8),
>> +                       .buf = buf
>> +               },
>> +               {
>> +                       .addr = client->addr,
>> +                       .flags = I2C_M_RD,
>> +                       .len = len,
>> +                       .buf = buf
>> +               }
>> +       };
>> +       int ret;
>> +
>> +       BUG_ON(reg > ISL12057_REG_SR);
>> +       BUG_ON(reg + len > ISL12057_REG_SR + 1);
>> +
>> +       ret = i2c_transfer(client->adapter, msgs, 2);
>> +       if (ret != 2)
>> +               return ret;
>
> This might return 1.
>
> Given that in isl12057_rtc_read_time you check if the return value is
> negative (also indirectly via isl12057_i2c_validate_client), I think it
> would make sense to always either return a negative error code or 0.

Good catch. Will fix that and spend some more time on return paths, and
then send a v2.

Cheers,

a+
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html