Message ID | 20180617114929.719-1-daniel@zonque.org |
---|---|
State | Superseded, archived |
Headers | show |
Series | [1/5] dt-bindings: leds: Add bindings for lltc,lt3593 | expand |
Hi Daniel, Thank you for the patch. On 06/17/2018 01:49 PM, Daniel Mack wrote: > Signed-off-by: Daniel Mack <daniel@zonque.org> > --- > .../devicetree/bindings/leds/leds-lt3593.txt | 25 +++++++++++++++++++ > 1 file changed, 25 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lt3593.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lt3593.txt b/Documentation/devicetree/bindings/leds/leds-lt3593.txt > new file mode 100644 > index 000000000000..0a7035acda8f > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lt3593.txt > @@ -0,0 +1,25 @@ > +Bindings for Linear Technologies LT3593 LED controller > + > +Required properties: > +- compatible : should be "lltc,lt3593". > +- label: A label for the LED. > + > +Optional properties: > +- linux,default-trigger : (optional) > + see Documentation/devicetree/bindings/leds/common.txt > +- default-state: (optional) The initial state of the LED. > + see Documentation/devicetree/bindings/leds/common.txt > + > +The controller only suports one LED, hence the properties are all set > +in the root node of the device. If multiple chips of this kind are > +found in a design, each one needs to be handled by its own device node. Please do not make an excuse for a single LED. Let's keep the bindings consistent across all LED class drivers, i.e. each LED should have a dedicated child node. Even there is a single output available. Please compare how recently added LED bindings look like. This also influences the way of LED class device name construction. Please also refer to the recently added drivers for that, e.g.: drivers/leds/leds-cr0014114.c. > +Example: > + > +led-controller { > + compatible = "lltc,lt3593"; > + label = "led-stripe"; Label should stick to the "color:function" pattern. > + gpios = <&gpio 0 GPIO_ACTIVE_HIGH>; > + linux,default-trigger = "heartbeat"; > +}; > + >
Hi Jacek, Thanks for the prompt reply! On Sunday, June 17, 2018 09:39 PM, Jacek Anaszewski wrote: > On 06/17/2018 01:49 PM, Daniel Mack wrote: >> Signed-off-by: Daniel Mack <daniel@zonque.org> >> --- >> .../devicetree/bindings/leds/leds-lt3593.txt | 25 +++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-lt3593.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-lt3593.txt b/Documentation/devicetree/bindings/leds/leds-lt3593.txt >> new file mode 100644 >> index 000000000000..0a7035acda8f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-lt3593.txt >> @@ -0,0 +1,25 @@ >> +Bindings for Linear Technologies LT3593 LED controller >> + >> +Required properties: >> +- compatible : should be "lltc,lt3593". >> +- label: A label for the LED. >> + >> +Optional properties: >> +- linux,default-trigger : (optional) >> + see Documentation/devicetree/bindings/leds/common.txt >> +- default-state: (optional) The initial state of the LED. >> + see Documentation/devicetree/bindings/leds/common.txt >> + >> +The controller only suports one LED, hence the properties are all set >> +in the root node of the device. If multiple chips of this kind are >> +found in a design, each one needs to be handled by its own device node. > > Please do not make an excuse for a single LED. Let's keep the bindings > consistent across all LED class drivers, i.e. each LED should have > a dedicated child node. Even there is a single output available. Okay, I can do that. But you do agree with the decision to only support one single LED in the driver and the binding? The hardware chip can only drive one single LED, but of course, the driver could support multiple chips with a single device node and different sub-nodes. In fact the driver did support this via pdata before this series. I believe, however, that this is against the principles of how device-trees are supposed to describe the hardware, so I changed it. And as I wrote in the commit log, the only mainline user only uses one LED anyway, so the change won't cause any regression. Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday, June 17, 2018 09:52 PM, Daniel Mack wrote: > On Sunday, June 17, 2018 09:39 PM, Jacek Anaszewski wrote: >> Please do not make an excuse for a single LED. Let's keep the bindings >> consistent across all LED class drivers, i.e. each LED should have >> a dedicated child node. Even there is a single output available. > > Okay, I can do that. But you do agree with the decision to only support > one single LED in the driver and the binding? The hardware chip can only > drive one single LED, but of course, the driver could support multiple > chips with a single device node and different sub-nodes. In fact the > driver did support this via pdata before this series. > > I believe, however, that this is against the principles of how > device-trees are supposed to describe the hardware, so I changed it. And > as I wrote in the commit log, the only mainline user only uses one LED > anyway, so the change won't cause any regression. And btw, that one user of the driver is soon to be replaced by a device-tree based version, and once that's finished, I'll send another patch to kill pdata handling from this driver entirely. So the question is only how the DT bindings should look like, the pdata bits are just kept around for legacy compat and don't matter. Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/leds/leds-lt3593.txt b/Documentation/devicetree/bindings/leds/leds-lt3593.txt new file mode 100644 index 000000000000..0a7035acda8f --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-lt3593.txt @@ -0,0 +1,25 @@ +Bindings for Linear Technologies LT3593 LED controller + +Required properties: +- compatible : should be "lltc,lt3593". +- label: A label for the LED. + +Optional properties: +- linux,default-trigger : (optional) + see Documentation/devicetree/bindings/leds/common.txt +- default-state: (optional) The initial state of the LED. + see Documentation/devicetree/bindings/leds/common.txt + +The controller only suports one LED, hence the properties are all set +in the root node of the device. If multiple chips of this kind are +found in a design, each one needs to be handled by its own device node. + +Example: + +led-controller { + compatible = "lltc,lt3593"; + label = "led-stripe"; + gpios = <&gpio 0 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "heartbeat"; +}; +
Signed-off-by: Daniel Mack <daniel@zonque.org> --- .../devicetree/bindings/leds/leds-lt3593.txt | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-lt3593.txt