Hi Jean Delvare again
Thank you for checking patch.
> > + reg->val = i2c_smbus_read_byte_data(priv->client, reg->reg);
> > +
> > + if (reg->val < 0)
>
> Nitpicking: traditional coding style says no blank line between
> function call and its error checking.
Thank you.
I'll fix it.
> Also, I just noticed that reg->val is an __u64 so it can't be < 0.
> You'll have to use a local int to store the result of
> i2c_smbus_read_byte_data(). I'm surprised the compiler didn't warn
> you... Maybe you didn't test with CONFIG_VIDEO_ADV_DEBUG=y?
>
> > + return -EIO;
>
> You might as well return the error value you got from
> i2c_smbus_read_byte_data(), it may be more detailed.
OK. I'll fix it.
> > + if (i2c_smbus_write_byte_data(priv->client, reg->reg, reg->val) < 0)
> > + return -EIO;
>
> Here again it would be preferable to return the error value you got
> from i2c_smbus_write_byte_data() instead of hard-coding one.
OK. I'll fix it.
> > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> > + dev_warn(&adapter->dev,
> > + "I2C-Adapter doesn't support I2C_FUNC_SMBUS_BYTE\n");
>
> The warning message still doesn't match the failed check
> (I2C_FUNC_SMBUS_BYTE_DATA != I2C_FUNC_SMBUS_BYTE). Also, I believe you
> should use dev_err(), not dev_warn(), as this is a fatal error.
Is that so?
mt9m001.c used dev_warn...
I'm sorry about the content of the message.
I thought what you say is "adapter".
OK. I'll fix it.
/Morimoto
_______________________________________________
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
