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

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

 




Hello,

On 08/11/2017 03:47 PM, Pavel Machek wrote:
> 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?

ah. this is a left over. I can fix in a resend.

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

Thanks,

C.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux