| Message ID | 20220208191236.660172-2-sven@svenschwermer.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v6,1/3] dt-bindings: leds: Optional multi-led unit address | expand |
Hello Sven, Am Tue, Feb 08, 2022 at 08:12:35PM +0100 schrieb sven@svenschwermer.de: > From: Sven Schwermer <sven.schwermer@disruptive-technologies.com> > > This allows to group multiple PWM-connected monochrome LEDs into > multicolor LEDs, e.g. RGB LEDs. > > Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com> > --- > > Notes: > Changes in v6: > * Fix device tree binding schema > > Changes in v5: > * (no changes) > > Changes in v4: > * (no changes) > > Changes in v3: > * Remove multi-led unit name > > .../bindings/leds/leds-pwm-multicolor.yaml | 69 +++++++++++++++++++ > 1 file changed, 69 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml > > diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml > new file mode 100644 > index 000000000000..f7ce29c8ae63 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml > @@ -0,0 +1,69 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-pwm-multicolor.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Multi-color LEDs connected to PWM > + > +maintainers: > + - Sven Schwermer <sven.schwermer@disruptive-technologies.com> > + > +description: | > + This driver combines several monochrome PWM LEDs into one multi-color > + LED using the multicolor LED class. > + > +properties: > + compatible: > + const: pwm-leds-multicolor > + multi-led: > + type: object > + patternProperties: > + "^led-[0-9a-z]+$": > + type: object > + $ref: common.yaml# > + properties: > + pwms: > + maxItems: 1 > + pwm-names: true > + color: true > + required: > + - pwms > + - color > + additionalProperties: false > +required: > + - compatible > +allOf: > + - $ref: leds-class-multicolor.yaml# > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/leds/common.h> > + > + rgb-led { I think this should be 'led-controller'. See Documentation/devicetree/bindings/leds/common.yaml for reference. > + compatible = "pwm-leds-multicolor"; > + > + multi-led { > + color = <LED_COLOR_ID_RGB>; > + function = LED_FUNCTION_INDICATOR; > + max-brightness = <65535>; > + > + led-red { > + pwms = <&pwm1 0 1000000>; > + color = <LED_COLOR_ID_RED>; > + }; > + > + led-green { > + pwms = <&pwm2 0 1000000>; > + color = <LED_COLOR_ID_GREEN>; > + }; > + > + led-blue { > + pwms = <&pwm3 0 1000000>; > + color = <LED_COLOR_ID_BLUE>; > + }; Not sure if those node names should be more generic like led-0, led-1 etc.? At least the color information is redundant here. This would make it more similar to bindings of other LED drivers. And how is it supposed to be named if you have multiple "multi-led"s, e.g. one on three PWM channels, and another one on three different PWM channels? Greets Alex > + }; > + }; > + > +... > -- > 2.35.1 >
On Tue, Feb 08, 2022 at 08:12:35PM +0100, sven@svenschwermer.de wrote: > From: Sven Schwermer <sven.schwermer@disruptive-technologies.com> > > This allows to group multiple PWM-connected monochrome LEDs into > multicolor LEDs, e.g. RGB LEDs. > > Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com> > --- > > Notes: > Changes in v6: > * Fix device tree binding schema > > Changes in v5: > * (no changes) > > Changes in v4: > * (no changes) > > Changes in v3: > * Remove multi-led unit name > > .../bindings/leds/leds-pwm-multicolor.yaml | 69 +++++++++++++++++++ > 1 file changed, 69 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml > > diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml > new file mode 100644 > index 000000000000..f7ce29c8ae63 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml > @@ -0,0 +1,69 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-pwm-multicolor.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Multi-color LEDs connected to PWM > + > +maintainers: > + - Sven Schwermer <sven.schwermer@disruptive-technologies.com> > + > +description: | > + This driver combines several monochrome PWM LEDs into one multi-color > + LED using the multicolor LED class. > + > +properties: > + compatible: > + const: pwm-leds-multicolor blank line > + multi-led: > + type: object blank line > + patternProperties: > + "^led-[0-9a-z]+$": > + type: object > + $ref: common.yaml# blank line > + properties: > + pwms: > + maxItems: 1 blank line and so on. (There not a hard rule, but roughly it's blank lines between DT property sub-schemas, top-level keywords, and properties/patternProperties/required.) > + pwm-names: true > + color: true > + required: > + - pwms > + - color > + additionalProperties: false While the top level goes at the end, I find it is easier to read putting the indented cases before properties/patternProperties. > +required: > + - compatible > +allOf: > + - $ref: leds-class-multicolor.yaml# > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/leds/common.h> > + > + rgb-led { > + compatible = "pwm-leds-multicolor"; > + > + multi-led { > + color = <LED_COLOR_ID_RGB>; > + function = LED_FUNCTION_INDICATOR; > + max-brightness = <65535>; > + > + led-red { > + pwms = <&pwm1 0 1000000>; > + color = <LED_COLOR_ID_RED>; > + }; > + > + led-green { > + pwms = <&pwm2 0 1000000>; > + color = <LED_COLOR_ID_GREEN>; > + }; > + > + led-blue { > + pwms = <&pwm3 0 1000000>; > + color = <LED_COLOR_ID_BLUE>; > + }; > + }; > + }; > + > +... > -- > 2.35.1 > >
Hi Alex, On 2/9/22 10:17, Alexander Dahl wrote: >> + rgb-led { > > I think this should be 'led-controller'. See > Documentation/devicetree/bindings/leds/common.yaml for reference. Sure, I don't have a preference. >> + multi-led { >> + color = <LED_COLOR_ID_RGB>; >> + function = LED_FUNCTION_INDICATOR; >> + max-brightness = <65535>; >> + >> + led-red { >> + pwms = <&pwm1 0 1000000>; >> + color = <LED_COLOR_ID_RED>; >> + }; >> + >> + led-green { >> + pwms = <&pwm2 0 1000000>; >> + color = <LED_COLOR_ID_GREEN>; >> + }; >> + >> + led-blue { >> + pwms = <&pwm3 0 1000000>; >> + color = <LED_COLOR_ID_BLUE>; >> + }; > > Not sure if those node names should be more generic like led-0, led-1 > etc.? At least the color information is redundant here. This would > make it more similar to bindings of other LED drivers. I don't see how naming them led-{0,1,2} would be better in any way, please elaborate. > And how is it supposed to be named if you have multiple > "multi-led"s, e.g. one on three PWM channels, and another one on three > different PWM channels? I'm not 100% sure what you mean. If you want multiple instances of these multi-color PWM LEDs, you'd do something like this: indicator-led-controller { compatible = "pwm-leds-multicolor"; multi-led { color = <LED_COLOR_ID_RGB>; function = LED_FUNCTION_INDICATOR; max-brightness = <65535>; led-red { pwms = <&pwm1 0 1000000>; color = <LED_COLOR_ID_RED>; }; led-green { pwms = <&pwm2 0 1000000>; color = <LED_COLOR_ID_GREEN>; }; led-blue { pwms = <&pwm3 0 1000000>; color = <LED_COLOR_ID_BLUE>; }; }; }; status-led-controller { compatible = "pwm-leds-multicolor"; multi-led { color = <LED_COLOR_ID_MULTI>; function = LED_FUNCTION_STATUS; max-brightness = <255>; led-red { pwms = <&pwm4 0 1000000>; color = <LED_COLOR_ID_RED>; }; led-amber { pwms = <&pwm5 0 1000000>; color = <LED_COLOR_ID_AMBER>; }; }; };
Hi Alex, On 2/10/22 10:33, Alexander Dahl wrote: >> I don't see how naming them led-{0,1,2} would be better in any way, please >> elaborate. > > - consistency with other LED driver bindings > - spot the number of "sub"-LEDs more easily > - prevent all kinds of different names people will come up with, if > all is allowed instead of a clear scheme > > The color is in the color property anyways. Let's hear what others on this list say. I don't feel too strongly about this either way. > I would have expected something like this: > > led-controller-0 { > compatible = "pwm-leds-multicolor"; > > multi-led-0 { > color = <LED_COLOR_ID_RGB>; > function = LED_FUNCTION_INDICATOR; > max-brightness = <65535>; > > led-0 { > pwms = <&pwm1 0 1000000>; > color = <LED_COLOR_ID_RED>; > }; > > led-1 { > pwms = <&pwm2 0 1000000>; > color = <LED_COLOR_ID_GREEN>; > }; > > led-2 { > pwms = <&pwm3 0 1000000>; > color = <LED_COLOR_ID_BLUE>; > }; > }; > > multi-led-1 { > color = <LED_COLOR_ID_RGB>; > function = LED_FUNCTION_INDICATOR; > max-brightness = <65535>; > > led-0 { > pwms = <&pwm1 0 1000000>; > color = <LED_COLOR_ID_RED>; > }; > > led-1 { > pwms = <&pwm2 0 1000000>; > color = <LED_COLOR_ID_GREEN>; > }; > > led-2 { > pwms = <&pwm3 0 1000000>; > color = <LED_COLOR_ID_BLUE>; > }; > }; > }; That would make the driver more complex. I'm not sure that would be worth it. Additionally, we'd have to change the node name pattern for the multi LED class. Other opinions? Best regards, Sven
diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml new file mode 100644 index 000000000000..f7ce29c8ae63 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-pwm-multicolor.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Multi-color LEDs connected to PWM + +maintainers: + - Sven Schwermer <sven.schwermer@disruptive-technologies.com> + +description: | + This driver combines several monochrome PWM LEDs into one multi-color + LED using the multicolor LED class. + +properties: + compatible: + const: pwm-leds-multicolor + multi-led: + type: object + patternProperties: + "^led-[0-9a-z]+$": + type: object + $ref: common.yaml# + properties: + pwms: + maxItems: 1 + pwm-names: true + color: true + required: + - pwms + - color + additionalProperties: false +required: + - compatible +allOf: + - $ref: leds-class-multicolor.yaml# +additionalProperties: false + +examples: + - | + #include <dt-bindings/leds/common.h> + + rgb-led { + compatible = "pwm-leds-multicolor"; + + multi-led { + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_INDICATOR; + max-brightness = <65535>; + + led-red { + pwms = <&pwm1 0 1000000>; + color = <LED_COLOR_ID_RED>; + }; + + led-green { + pwms = <&pwm2 0 1000000>; + color = <LED_COLOR_ID_GREEN>; + }; + + led-blue { + pwms = <&pwm3 0 1000000>; + color = <LED_COLOR_ID_BLUE>; + }; + }; + }; + +...