Hi Rob, On 06/09/2017 01:13 PM, Cédric Le Goater wrote: > Hello, > >>>> Thanks for the patch. >>>> >>>> Generally I'd split it into two patches: >>>> 1/2: addition of LED class specific DT support >>>> 2/2: addition of GPIO support (for this one please cc also >>>> GPIO subsystem maintainer) >>> >>> OK. I will in next version. >>> >>>> Please also see my comments below. >>>> >>>> On 05/09/2017 08:36 AM, Cédric Le Goater wrote: >>>>> 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 support for device tree and Open Firmware to be >>>>> able do define different operation modes for each pin. See bindings >>>>> documentation for more details. The pca955x driver is then extended >>>>> with a gpio_chip when pins are operating in GPIO mode. >>>>> >>>>> Signed-off-by: Cédric Le Goater <clg@xxxxxxxx> >>>>> --- >>>>> .../devicetree/bindings/leds/leds-pca955x.txt | 103 ++++++++ >>>>> drivers/leds/Kconfig | 11 + >>>>> drivers/leds/leds-pca955x.c | 290 ++++++++++++++++++--- >>>>> 3 files changed, 374 insertions(+), 30 deletions(-) >>>>> create mode 100644 Documentation/devicetree/bindings/leds/leds-pca955x.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-pca955x.txt b/Documentation/devicetree/bindings/leds/leds-pca955x.txt >>>>> new file mode 100644 >>>>> index 000000000000..98d1053dd1b0 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/leds/leds-pca955x.txt >>>>> @@ -0,0 +1,103 @@ >>>>> +LEDs connected to pca9550, pca9551, pca9552, pca9553, >>>>> + >>>>> +Required properties: >>>>> +- compatible : should be one of : >>>>> + "nxp,pca9550" >>>>> + "nxp,pca9551" >>>>> + "nxp,pca9552" >>>>> + "nxp,pca9553" >>>>> +- #address-cells: must be 1 >>>>> +- #size-cells: must be 0 >>>>> +- reg: I2C slave address. depends on the model. >>>>> + >>>>> +Optional properties: >>>>> +- gpio-controller: allows pins to be used as GPIOs. >>>>> +- #gpio-cells: if present, must not be 0. >>>>> +- gpio-base : base number of the pins used as GPIOs. If there are more >>>>> + than one, they should be contiguous. See 'type' property >>>>> + below. >>>>> + >>>>> +LED sub-node properties: >>>>> +- reg : number of LED line. >>>>> + from 0 to 1 in pca9550 >>>>> + from 0 to 7 in pca9551 >>>>> + from 0 to 15 in pca9552 >>>>> + from 0 to 3 in pca9553 >>>>> +- compatible: either "none", "led" (default) or "gpio". >>>>> +- label : (optional) >>>>> + see Documentation/devicetree/bindings/leds/common.txt >>>>> +- linux,default-trigger : (optional) >>>>> + see Documentation/devicetree/bindings/leds/common.txt >>>>> + >>>>> +Examples: >>>>> + >>>>> +pca9552: pca9552@60 { >>>>> + compatible = "nxp,pca9552"; >>>>> + #address-cells = <1>; >>>>> + #size-cells = <0>; >>>>> + reg = <0x60>; >>>>> + >>>>> + gpio-controller; >>>>> + #gpio-cells = <2>; >>>>> + gpio-base = <12>; >>>>> + >>>>> + gpio@12 { >>>>> + label = "GPIO12"; >>>>> + reg = <12>; >>>>> + compatible = "gpio"; >>>>> + }; >>>>> + gpio@13 { >>>>> + label = "GPIO13"; >>>>> + reg = <13>; >>>>> + compatible = "gpio"; >>>>> + }; >>>>> + gpio@14 { >>>>> + label = "GPIO14"; >>>>> + reg = <14>; >>>>> + compatible = "gpio"; >>>>> + }; >>>>> + gpio@15 { >>>>> + label = "GPIO15"; >>>>> + reg = <15>; >>>>> + compatible = "gpio"; >>>>> + }; >>>> >>>> I think that for GPIO pins we should have GPIO specific bindings, >>>> but DT maintainer would have to give his opinion here. >>> >>> I have got my inspiration from the tca6507 bindings. >> >> Right, but AFAIK there was no DT maintainer position at that time >> and those bindings don't have relevant ack. I'd rather wait for >> DT maintainer opinion so as to avoid spawning wrong practices. >> >>>> Also I'm not sure if using compatible property this way is correct. >>> >>> yes. > > So, is the usage of the 'compatible' property correct ? > > If not, we could use a 'type' property with an include file defining > the possible values. This is a common pattern. Could you please confirm if using compatible property this way is acceptable? It follows Documentation/devicetree/bindings/leds/tca6507.txt. -- Best regards, Jacek Anaszewski -- 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