Re: [PATCH v3 3/4] leds: pca955x: add GPIO support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi!

> The PCA955x family of chips are I2C LED blinkers whose pins not used
> to control LEDs can be used as general purpose I/Os (GPIOs).
> 
> The following adds such a support by defining different operation
> modes for the pins (See bindings documentation for more details). The
> pca955x driver is then extended with a gpio_chip when some of pins are
> operating as GPIOs. The default operating mode is to behave as a LED.
> 
> The GPIO support is conditioned by CONFIG_LEDS_PCA955X_GPIO.
> 
> Signed-off-by: Cédric Le Goater <clg@xxxxxxxx>

For the series,

Acked-by: Pavel Machek <pavel@xxxxxx>

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?

> +#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?
									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


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux