[i2c] [patch 188.8.131.52] add support for the PCF8575 chip
Bart Van Assche
bart.vanassche at gmail.com
Wed Oct 24 13:09:41 CEST 2007
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);
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.
Bart Van Assche.
More information about the i2c