* [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding [not found] <20200712210601.5239-1-marek.behun@nic.cz> @ 2020-07-12 21:06 ` Marek Behún 2020-07-12 21:27 ` Jacek Anaszewski 0 siblings, 1 reply; 6+ messages in thread From: Marek Behún @ 2020-07-12 21:06 UTC (permalink / raw) To: linux-leds Cc: Pavel Machek, jacek.anaszewski, Dan Murphy, Marek Behún, Rob Herring, devicetree Add device-tree bindings documentation for Turris Omnia RGB LEDs. Signed-off-by: Marek Behún <marek.behun@nic.cz> Cc: Rob Herring <robh+dt@kernel.org> Cc: devicetree@vger.kernel.org --- .../leds/cznic,turris-omnia-leds.yaml | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml new file mode 100644 index 000000000000..9817ea3ac69b --- /dev/null +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml @@ -0,0 +1,82 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/cznic,turris-omnia-leds.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: CZ.NIC's Turris Omnia LEDs driver + +maintainers: + - Marek Behún <marek.behun@nic.cz> + +description: + This module adds support for the RGB LEDs found on the fron panel of the + Turris Omnia router. There are 12 RGB LEDs, they are controlled by device's + microcontroller with which the system communicates via I2C. Each LED is + described as a subnode of this I2C device. + +properties: + compatible: + const: cznic,turris-omnia-leds + + reg: + description: I2C slave address of the microcontroller. + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^multi-led[0-9a-f]$": + type: object + allOf: + - $ref: leds-class-multicolor.yaml# + description: + This node represents one of the RGB LED devices on Turris Omnia. + + properties: + reg: + minimum: 0 + maximum: 11 + description: + This property identifies one of the LEDs on the front panel of the + Turris Omnia router. + + required: + - reg + +examples: + - | + + #include <dt-bindings/leds/common.h> + + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@2b { + compatible = "cznic,turris-omnia-leds"; + reg = <0x2b>; + #address-cells = <1>; + #size-cells = <0>; + + multi-led@0 { + reg = <0>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_POWER; + linux,default-trigger = "heartbeat"; + }; + + multi-led@a { + reg = <0xa>; + color = <LED_COLOR_ID_MULTI>; + function = LED_FUNCTION_INDICATOR; + function-enumerator = <1>; + }; + }; + }; + +... -- 2.26.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding 2020-07-12 21:06 ` [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún @ 2020-07-12 21:27 ` Jacek Anaszewski 2020-07-12 21:40 ` Marek Behun 0 siblings, 1 reply; 6+ messages in thread From: Jacek Anaszewski @ 2020-07-12 21:27 UTC (permalink / raw) To: Marek Behún, linux-leds Cc: Pavel Machek, Dan Murphy, Rob Herring, devicetree Hi Marek, Thank you for the patch. One note below. On 7/12/20 11:06 PM, Marek Behún wrote: > Add device-tree bindings documentation for Turris Omnia RGB LEDs. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: devicetree@vger.kernel.org > --- > .../leds/cznic,turris-omnia-leds.yaml | 82 +++++++++++++++++++ > 1 file changed, 82 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml > > diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml > new file mode 100644 > index 000000000000..9817ea3ac69b > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml > @@ -0,0 +1,82 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/cznic,turris-omnia-leds.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: CZ.NIC's Turris Omnia LEDs driver > + > +maintainers: > + - Marek Behún <marek.behun@nic.cz> > + > +description: > + This module adds support for the RGB LEDs found on the fron panel of the > + Turris Omnia router. There are 12 RGB LEDs, they are controlled by device's > + microcontroller with which the system communicates via I2C. Each LED is > + described as a subnode of this I2C device. > + > +properties: > + compatible: > + const: cznic,turris-omnia-leds > + > + reg: > + description: I2C slave address of the microcontroller. > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^multi-led[0-9a-f]$": > + type: object > + allOf: > + - $ref: leds-class-multicolor.yaml# > + description: > + This node represents one of the RGB LED devices on Turris Omnia. > + > + properties: > + reg: > + minimum: 0 > + maximum: 11 > + description: > + This property identifies one of the LEDs on the front panel of the > + Turris Omnia router. > + > + required: > + - reg > + > +examples: > + - | > + > + #include <dt-bindings/leds/common.h> > + > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led-controller@2b { > + compatible = "cznic,turris-omnia-leds"; > + reg = <0x2b>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + multi-led@0 { > + reg = <0>; > + color = <LED_COLOR_ID_MULTI>; > + function = LED_FUNCTION_POWER; Please provide child nodes for each color LED. Let's stick to the bindings closely and not make any deviations from the beginning. > + linux,default-trigger = "heartbeat"; > + }; > + > + multi-led@a { > + reg = <0xa>; > + color = <LED_COLOR_ID_MULTI>; > + function = LED_FUNCTION_INDICATOR; > + function-enumerator = <1>; > + }; > + }; > + }; > + > +... > -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding 2020-07-12 21:27 ` Jacek Anaszewski @ 2020-07-12 21:40 ` Marek Behun 2020-07-12 22:11 ` Jacek Anaszewski 0 siblings, 1 reply; 6+ messages in thread From: Marek Behun @ 2020-07-12 21:40 UTC (permalink / raw) To: Jacek Anaszewski Cc: linux-leds, Pavel Machek, Dan Murphy, Rob Herring, devicetree On Sun, 12 Jul 2020 23:27:07 +0200 Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > > + multi-led@0 { > > + reg = <0>; > > + color = <LED_COLOR_ID_MULTI>; > > + function = LED_FUNCTION_POWER; > > Please provide child nodes for each color LED. Let's stick > to the bindings closely and not make any deviations from > the beginning. Why? It would make sense if there were devices using this controller having other configuration, but on Omnia, all LEDs are RGB. Also, if I do this, should I also make the driver check in the probe function whether the per-channel child nodes are correct? Eg. if they are always three: one for red, one for green and one for blue? Or should the driver ignore this and only the device tree binding specify it? Because the way the driver is written now, it only registers multi-color RGB LEDs. Marek ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding 2020-07-12 21:40 ` Marek Behun @ 2020-07-12 22:11 ` Jacek Anaszewski 2020-07-13 15:06 ` Rob Herring 0 siblings, 1 reply; 6+ messages in thread From: Jacek Anaszewski @ 2020-07-12 22:11 UTC (permalink / raw) To: Marek Behun; +Cc: linux-leds, Pavel Machek, Dan Murphy, Rob Herring, devicetree On 7/12/20 11:40 PM, Marek Behun wrote: > On Sun, 12 Jul 2020 23:27:07 +0200 > Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > >>> + multi-led@0 { >>> + reg = <0>; >>> + color = <LED_COLOR_ID_MULTI>; >>> + function = LED_FUNCTION_POWER; >> >> Please provide child nodes for each color LED. Let's stick >> to the bindings closely and not make any deviations from >> the beginning. > > Why? It would make sense if there were devices using this controller > having other configuration, but on Omnia, all LEDs are RGB. > > Also, if I do this, should I also make the driver check in the probe > function whether the per-channel child nodes are correct? Eg. if they > are always three: one for red, one for green and one for blue? Or > should the driver ignore this and only the device tree binding specify > it? > > Because the way the driver is written now, it only registers > multi-color RGB LEDs. This is not RGB framework, but multicolor framework. It is not justified to pretend that RGB is default. Unless you would state that clearly in the comment in DT, but that should be agreed upon with Rob. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding 2020-07-12 22:11 ` Jacek Anaszewski @ 2020-07-13 15:06 ` Rob Herring 2020-07-13 21:28 ` Jacek Anaszewski 0 siblings, 1 reply; 6+ messages in thread From: Rob Herring @ 2020-07-13 15:06 UTC (permalink / raw) To: Jacek Anaszewski Cc: Marek Behun, linux-leds, Pavel Machek, Dan Murphy, devicetree On Mon, Jul 13, 2020 at 12:11:51AM +0200, Jacek Anaszewski wrote: > On 7/12/20 11:40 PM, Marek Behun wrote: > > On Sun, 12 Jul 2020 23:27:07 +0200 > > Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > > > > > > + multi-led@0 { > > > > + reg = <0>; > > > > + color = <LED_COLOR_ID_MULTI>; > > > > + function = LED_FUNCTION_POWER; > > > > > > Please provide child nodes for each color LED. Let's stick > > > to the bindings closely and not make any deviations from > > > the beginning. > > > > Why? It would make sense if there were devices using this controller > > having other configuration, but on Omnia, all LEDs are RGB. > > > > Also, if I do this, should I also make the driver check in the probe > > function whether the per-channel child nodes are correct? Eg. if they > > are always three: one for red, one for green and one for blue? Or > > should the driver ignore this and only the device tree binding specify > > it? > > > > Because the way the driver is written now, it only registers > > multi-color RGB LEDs. > > This is not RGB framework, but multicolor framework. It is not justified > to pretend that RGB is default. Unless you would state that clearly in > the comment in DT, but that should be agreed upon with Rob. If the LEDs are fixed in h/w and never vary for this controller, then they don't need to be in DT. However, is it really possible that a channel only supports 1 color of LED? I don't think so. Rob ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding 2020-07-13 15:06 ` Rob Herring @ 2020-07-13 21:28 ` Jacek Anaszewski 0 siblings, 0 replies; 6+ messages in thread From: Jacek Anaszewski @ 2020-07-13 21:28 UTC (permalink / raw) To: Rob Herring; +Cc: Marek Behun, linux-leds, Pavel Machek, Dan Murphy, devicetree On 7/13/20 5:06 PM, Rob Herring wrote: > On Mon, Jul 13, 2020 at 12:11:51AM +0200, Jacek Anaszewski wrote: >> On 7/12/20 11:40 PM, Marek Behun wrote: >>> On Sun, 12 Jul 2020 23:27:07 +0200 >>> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: >>> >>>>> + multi-led@0 { >>>>> + reg = <0>; >>>>> + color = <LED_COLOR_ID_MULTI>; >>>>> + function = LED_FUNCTION_POWER; >>>> >>>> Please provide child nodes for each color LED. Let's stick >>>> to the bindings closely and not make any deviations from >>>> the beginning. >>> >>> Why? It would make sense if there were devices using this controller >>> having other configuration, but on Omnia, all LEDs are RGB. >>> >>> Also, if I do this, should I also make the driver check in the probe >>> function whether the per-channel child nodes are correct? Eg. if they >>> are always three: one for red, one for green and one for blue? Or >>> should the driver ignore this and only the device tree binding specify >>> it? >>> >>> Because the way the driver is written now, it only registers >>> multi-color RGB LEDs. >> >> This is not RGB framework, but multicolor framework. It is not justified >> to pretend that RGB is default. Unless you would state that clearly in >> the comment in DT, but that should be agreed upon with Rob. > > If the LEDs are fixed in h/w and never vary for this controller, then > they don't need to be in DT. However, is it really possible that a > channel only supports 1 color of LED? I don't think so. Theoretically we could allow for registering two LEDs as LED multi color device and the other one as monochrome LED class device but I admit that this would be a bit convoluted and entail unnecessary complexity in the driver. So I'm OK with no child nodes for these bindings, but let's add some comment justifying that. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-13 21:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200712210601.5239-1-marek.behun@nic.cz>
2020-07-12 21:06 ` [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún
2020-07-12 21:27 ` Jacek Anaszewski
2020-07-12 21:40 ` Marek Behun
2020-07-12 22:11 ` Jacek Anaszewski
2020-07-13 15:06 ` Rob Herring
2020-07-13 21:28 ` Jacek Anaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).