[i2c] [patch 2.6.23.1] add support for the PCF8575 chip

Bart Van Assche bart.vanassche at gmail.com
Wed Oct 24 13:09:41 CEST 2007


Hello Jean,

Thanks for the extensive review. By this time I have posted a third
version of the patch. I have tried to address all of your comments,
except as noted below.

On 10/23/07, Jean Delvare <khali at linux-fr.org> wrote:
>
> > +     return sprintf(buf, "%u\n", val);
>
> %hu

Because I changed the datatype of val from unsigned short to u16, I
left the format specifier as %u. C compilers have to promote values
passed on the stack to at least the size of an int. So for
printf()/sprintf()/snprintf() %u and %hu are equivalent.

> The value of data->write is uninitialized when you load the driver,
> you're returning a random value here, that's not correct. Please return
> -EAGAIN in this case, as the pcf8574 driver does.

The pcf8575 driver now returns -EAGAIN upon attempts to read
data->write before it is initialized. But are you sure that the
pcf8574 driver does this already ?

> You driver needs a module parameter to work. This should be documented.
> See Documentation/i2c/chips/pcf8574, maybe you can do something similar
> for your driver.

Are you sure the file Documentation/i2c/chips/pcf8574 exists ? I have
added a note at the top of the pcf8575.c source file.

-- 
Regards,

Bart Van Assche.



More information about the i2c mailing list