Hi!
> > with minor comments below.
> >
> >> @@ -125,6 +130,7 @@ struct pca955x_led {
> >> struct led_classdev led_cdev;
> >> int led_num; /* 0 .. 15 potentially */
> >> char name[32];
> >> + u32 type;
> >
> > u32 is highly non-standard here. enum pca955x_led_type?
> >
> >
> >> +/*
> >> + * Read the INPUT register, which contains the state of LEDs.
> >> + */
> >> +static u8 pca955x_read_input(struct i2c_client *client, int n)
> >> +{
> >> + return (u8)i2c_smbus_read_byte_data(client, n);
> >> +}
> >
> > Is the cast needed? Should you attempt to handle errors?
>
> ah. this is a left over. I can fix in a resend.
No need to resend just for this.
Should you WARN_ON() if the read_byte_data returns < 0 or something?
https://linuxtv.org/downloads/v4l-dvb-internals/device-drivers/API-i2c-smbus-read-byte-data.html
> >> +#ifndef _DT_BINDINGS_LEDS_PCA955X_H
> >> +#define _DT_BINDINGS_LEDS_PCA955X_H
> >> +
> >> +#define PCA955X_TYPE_NONE 0
> >> +#define PCA955X_TYPE_LED 1
> >> +#define PCA955X_TYPE_GPIO 2
> >
> > And make these into the new enum pca955x_led_type?
>
> These are used in the device tree bindings, so we should keep the u32
> I think.
Aha, ok, makes sense.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: Digital signature
