| Message ID | 20201005203451.9985-11-post@lespocky.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | leds: pwm: Make automatic labels work | expand |
Hello Alexander, On 10/5/20 10:34 PM, Alexander Dahl wrote: > The node names for devices using the pwm-leds driver follow a certain > naming scheme (now). Parent node name is not enforced, but recommended > by DT project. > > DTC arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml > CHECK arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml > /home/alex/build/linux/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml: led-rgb: 'led-blue', 'led-green', 'led-red' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+' > From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml > > Signed-off-by: Alexander Dahl <post@lespocky.de> Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de> But got two questions below: > --- > > Notes: > v6 -> v7: > * split up patch (one per sub arch) > * added actual warnings to commit message > > arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts > index 5700e6b700d3..25d548cb975b 100644 > --- a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts > +++ b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts > @@ -36,34 +36,34 @@ > stdout-path = &uart4; > }; > > - led-act { > + led-controller-1 { > compatible = "gpio-leds"; > > - led-green { > + led-1 { > label = "mc1:green:act"; > gpios = <&gpioa 13 GPIO_ACTIVE_LOW>; > linux,default-trigger = "heartbeat"; > }; > }; > > - led-rgb { > + led-controller-2 { Is a single RGB LED really a controller? > compatible = "pwm-leds"; > > - led-red { > + led-2 { Shouldn't this have been led-1 as well or is the numbering "global" ? > label = "mc1:red:rgb"; > pwms = <&leds_pwm 1 1000000 0>; > max-brightness = <255>; > active-low; > }; > > - led-green { > + led-3 { > label = "mc1:green:rgb"; > pwms = <&leds_pwm 2 1000000 0>; > max-brightness = <255>; > active-low; > }; > > - led-blue { > + led-4 { > label = "mc1:blue:rgb"; > pwms = <&leds_pwm 3 1000000 0>; > max-brightness = <255>; >
Hello Ahmad, thanks for your feedback, comments below. On Tue, Oct 27, 2020 at 08:03:40AM +0100, Ahmad Fatoum wrote: > Hello Alexander, > > On 10/5/20 10:34 PM, Alexander Dahl wrote: > > The node names for devices using the pwm-leds driver follow a certain > > naming scheme (now). Parent node name is not enforced, but recommended > > by DT project. > > > > DTC arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml > > CHECK arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml > > /home/alex/build/linux/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml: led-rgb: 'led-blue', 'led-green', 'led-red' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+' > > From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml > > > > Signed-off-by: Alexander Dahl <post@lespocky.de> > > Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > > But got two questions below: > > > --- > > > > Notes: > > v6 -> v7: > > * split up patch (one per sub arch) > > * added actual warnings to commit message > > > > arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts > > index 5700e6b700d3..25d548cb975b 100644 > > --- a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts > > +++ b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts > > @@ -36,34 +36,34 @@ > > stdout-path = &uart4; > > }; > > > > - led-act { > > + led-controller-1 { > > compatible = "gpio-leds"; > > > > - led-green { > > + led-1 { > > label = "mc1:green:act"; > > gpios = <&gpioa 13 GPIO_ACTIVE_LOW>; > > linux,default-trigger = "heartbeat"; > > }; > > }; > > > > - led-rgb { > > + led-controller-2 { > > Is a single RGB LED really a controller? I just followed the recommendations by Rob here. > > compatible = "pwm-leds"; > > > > - led-red { > > + led-2 { > > Shouldn't this have been led-1 as well or is the numbering "global" ? Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well? Greets Alex > > > label = "mc1:red:rgb"; > > pwms = <&leds_pwm 1 1000000 0>; > > max-brightness = <255>; > > active-low; > > }; > > > > - led-green { > > + led-3 { > > label = "mc1:green:rgb"; > > pwms = <&leds_pwm 2 1000000 0>; > > max-brightness = <255>; > > active-low; > > }; > > > > - led-blue { > > + led-4 { > > label = "mc1:blue:rgb"; > > pwms = <&leds_pwm 3 1000000 0>; > > max-brightness = <255>; > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hello, On 10/27/20 11:05 AM, Alexander Dahl wrote: > Hello Ahmad, > > thanks for your feedback, comments below. > >>> - led-rgb { >>> + led-controller-2 { >> >> Is a single RGB LED really a controller? > > I just followed the recommendations by Rob here. Do you happen to know if the new multicolor LED support could be used here? I find it unfortunate that the device tree loses information relevant to humans to adhere to a fixed nomenclature. Apparently led-controller isn't even codified in the YAML binding (It's just in the examples). If you respin, please add a comment that this is a single RGB led. I'd prefer to keep the information in the DTB as well though. > >>> compatible = "pwm-leds"; >>> >>> - led-red { >>> + led-2 { >> >> Shouldn't this have been led-1 as well or is the numbering "global" ? > > Also good question. This numbering is for dts only, it usually does > not correspond with LEDs on the board, so it could be numbered per > led-controller as well? I'd prefer that it starts by 1. That way it's aligned with PWM channel ID. Thanks for fixing the dtschema warnings by the way! Cheers, Ahmad > > Greets > Alex > >> >>> label = "mc1:red:rgb"; >>> pwms = <&leds_pwm 1 1000000 0>; >>> max-brightness = <255>; >>> active-low; >>> }; >>> >>> - led-green { >>> + led-3 { >>> label = "mc1:green:rgb"; >>> pwms = <&leds_pwm 2 1000000 0>; >>> max-brightness = <255>; >>> active-low; >>> }; >>> >>> - led-blue { >>> + led-4 { >>> label = "mc1:blue:rgb"; >>> pwms = <&leds_pwm 3 1000000 0>; >>> max-brightness = <255>; >>> >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
Hello Ahmad, Am Dienstag, 27. Oktober 2020, 11:58:10 CET schrieb Ahmad Fatoum: > Hello, > > On 10/27/20 11:05 AM, Alexander Dahl wrote: > > Hello Ahmad, > > > > thanks for your feedback, comments below. > > > >>> - led-rgb { > >>> + led-controller-2 { > >> > >> Is a single RGB LED really a controller? > > > > I just followed the recommendations by Rob here. > > Do you happen to know if the new multicolor LED support could be used here? AFAIK not yet. The multicolor class should be ready and it is used by some drivers for I²C connected LED controllers, but if I understood Pavel correctly, additional work has to be done for a gpio and/or pwm multicolor driver. See this thread from August for example: https://lore.kernel.org/linux-leds/2530787.iFCFyWWcSu@g550jk/ > > I find it unfortunate that the device tree loses information relevant to > humans to adhere to a fixed nomenclature. Apparently led-controller isn't > even codified in the YAML binding (It's just in the examples). If you > respin, please add a comment that this is a single RGB led. I'd prefer to > keep the information in the DTB as well though. The "new" attributes 'function' and 'color' attributes should cover this information. IIRC those were introduced sometime before v5.4 and documentation is in the leds/common.yaml binding. I don't see it in the scope of this patch series, but if we would merge this warning fix first, the information is lost, so maybe those attributes should be added before? My heuristics on that would be looking at the label and if there's a distinct color in it, add the color property. I could do that for all pwm LEDs known to the tree currently. That would be a bigger task for GPIO leds though. ;-) > > >>> compatible = "pwm-leds"; > >>> > >>> - led-red { > >>> + led-2 { > >> > >> Shouldn't this have been led-1 as well or is the numbering "global" ? > > > > Also good question. This numbering is for dts only, it usually does > > not correspond with LEDs on the board, so it could be numbered per > > led-controller as well? > > I'd prefer that it starts by 1. That way it's aligned with PWM channel > ID. Ack. > > Thanks for fixing the dtschema warnings by the way! Well, I "introduced" them by converting the leds-pwm binding to yaml (not merged yet), so I could as well fix the warnings then? ;-) Greets Alex > > Cheers, > Ahmad > > > Greets > > Alex > > > >>> label = "mc1:red:rgb"; > >>> pwms = <&leds_pwm 1 1000000 0>; > >>> max-brightness = <255>; > >>> active-low; > >>> > >>> }; > >>> > >>> - led-green { > >>> + led-3 { > >>> > >>> label = "mc1:green:rgb"; > >>> pwms = <&leds_pwm 2 1000000 0>; > >>> max-brightness = <255>; > >>> active-low; > >>> > >>> }; > >>> > >>> - led-blue { > >>> + led-4 { > >>> > >>> label = "mc1:blue:rgb"; > >>> pwms = <&leds_pwm 3 1000000 0>; > >>> max-brightness = <255>;
Hei hei, On Tue, Oct 27, 2020 at 11:58:10AM +0100, Ahmad Fatoum wrote: > Hello, > > On 10/27/20 11:05 AM, Alexander Dahl wrote: > > Hello Ahmad, > > > > thanks for your feedback, comments below. > > > > >>> - led-rgb { > >>> + led-controller-2 { > >> > >> Is a single RGB LED really a controller? > > > > I just followed the recommendations by Rob here. > > Do you happen to know if the new multicolor LED support could be used here? > > I find it unfortunate that the device tree loses information relevant to humans > to adhere to a fixed nomenclature. Apparently led-controller isn't even codified > in the YAML binding (It's just in the examples). If you respin, please add a > comment that this is a single RGB led. I'd prefer to keep the information > in the DTB as well though. Slightly off-topic, but while I was working on the patch based on your feedback I tried to find some information on that Linux Automation MC-1 board. However I could not find any? Is there some website, some datasheet or maybe a schematic online? The vendor prefix says "Linux Automation GmbH", but I find only that USB-SD-Mux on their page? Greets Alex > > > > > > >>> compatible = "pwm-leds"; > >>> > >>> - led-red { > >>> + led-2 { > >> > >> Shouldn't this have been led-1 as well or is the numbering "global" ? > > > > Also good question. This numbering is for dts only, it usually does > > not correspond with LEDs on the board, so it could be numbered per > > led-controller as well? > > I'd prefer that it starts by 1. That way it's aligned with PWM channel > ID. > > Thanks for fixing the dtschema warnings by the way! > > Cheers, > Ahmad > > > > > Greets > > Alex > > > >> > >>> label = "mc1:red:rgb"; > >>> pwms = <&leds_pwm 1 1000000 0>; > >>> max-brightness = <255>; > >>> active-low; > >>> }; > >>> > >>> - led-green { > >>> + led-3 { > >>> label = "mc1:green:rgb"; > >>> pwms = <&leds_pwm 2 1000000 0>; > >>> max-brightness = <255>; > >>> active-low; > >>> }; > >>> > >>> - led-blue { > >>> + led-4 { > >>> label = "mc1:blue:rgb"; > >>> pwms = <&leds_pwm 3 1000000 0>; > >>> max-brightness = <255>; > >>> > >> > >> -- > >> Pengutronix e.K. | | > >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | > >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hello Alexander, On 10/28/20 8:34 AM, Alexander Dahl wrote: > Hello Ahmad, > > Am Dienstag, 27. Oktober 2020, 11:58:10 CET schrieb Ahmad Fatoum: >> Hello, >> >> On 10/27/20 11:05 AM, Alexander Dahl wrote: >>> Hello Ahmad, >>> >>> thanks for your feedback, comments below. >>> >>>>> - led-rgb { >>>>> + led-controller-2 { >>>> >>>> Is a single RGB LED really a controller? >>> >>> I just followed the recommendations by Rob here. >> >> Do you happen to know if the new multicolor LED support could be used here? > > AFAIK not yet. The multicolor class should be ready and it is used by some > drivers for I²C connected LED controllers, but if I understood Pavel > correctly, additional work has to be done for a gpio and/or pwm multicolor > driver. See this thread from August for example: > > https://lore.kernel.org/linux-leds/2530787.iFCFyWWcSu@g550jk/ I see. Thanks for the info. >> I find it unfortunate that the device tree loses information relevant to >> humans to adhere to a fixed nomenclature. Apparently led-controller isn't >> even codified in the YAML binding (It's just in the examples). If you >> respin, please add a comment that this is a single RGB led. I'd prefer to >> keep the information in the DTB as well though. > > The "new" attributes 'function' and 'color' attributes should cover this > information. IIRC those were introduced sometime before v5.4 and documentation > is in the leds/common.yaml binding. I don't see it in the scope of this patch > series, but if we would merge this warning fix first, the information is lost, > so maybe those attributes should be added before? Does it? The label already says it's a green LED, but the information that it's a single physical LED 'bulb' is lost. > > My heuristics on that would be looking at the label and if there's a distinct > color in it, add the color property. I could do that for all pwm LEDs known to > the tree currently. That would be a bigger task for GPIO leds though. ;-) I would be ok with just the led-containing node hinting that it's a single RGB led. Cheers, Ahmad > >> >>>>> compatible = "pwm-leds"; >>>>> >>>>> - led-red { >>>>> + led-2 { >>>> >>>> Shouldn't this have been led-1 as well or is the numbering "global" ? >>> >>> Also good question. This numbering is for dts only, it usually does >>> not correspond with LEDs on the board, so it could be numbered per >>> led-controller as well? >> >> I'd prefer that it starts by 1. That way it's aligned with PWM channel >> ID. > > Ack. > >> >> Thanks for fixing the dtschema warnings by the way! > > Well, I "introduced" them by converting the leds-pwm binding to yaml (not > merged yet), so I could as well fix the warnings then? ;-) > > Greets > Alex > >> >> Cheers, >> Ahmad >> >>> Greets >>> Alex >>> >>>>> label = "mc1:red:rgb"; >>>>> pwms = <&leds_pwm 1 1000000 0>; >>>>> max-brightness = <255>; >>>>> active-low; >>>>> >>>>> }; >>>>> >>>>> - led-green { >>>>> + led-3 { >>>>> >>>>> label = "mc1:green:rgb"; >>>>> pwms = <&leds_pwm 2 1000000 0>; >>>>> max-brightness = <255>; >>>>> active-low; >>>>> >>>>> }; >>>>> >>>>> - led-blue { >>>>> + led-4 { >>>>> >>>>> label = "mc1:blue:rgb"; >>>>> pwms = <&leds_pwm 3 1000000 0>; >>>>> max-brightness = <255>; > >
Hello, On 10/31/20 2:54 PM, Alexander Dahl wrote: > Hei hei, > > On Tue, Oct 27, 2020 at 11:58:10AM +0100, Ahmad Fatoum wrote: >> Hello, >> >> On 10/27/20 11:05 AM, Alexander Dahl wrote: >>> Hello Ahmad, >>> >>> thanks for your feedback, comments below. >>> >> >>>>> - led-rgb { >>>>> + led-controller-2 { >>>> >>>> Is a single RGB LED really a controller? >>> >>> I just followed the recommendations by Rob here. >> >> Do you happen to know if the new multicolor LED support could be used here? >> >> I find it unfortunate that the device tree loses information relevant to humans >> to adhere to a fixed nomenclature. Apparently led-controller isn't even codified >> in the YAML binding (It's just in the examples). If you respin, please add a >> comment that this is a single RGB led. I'd prefer to keep the information >> in the DTB as well though. > > Slightly off-topic, but while I was working on the patch based on your > feedback I tried to find some information on that Linux Automation > MC-1 board. However I could not find any? Is there some website, some > datasheet or maybe a schematic online? The vendor prefix says "Linux > Automation GmbH", but I find only that USB-SD-Mux on their page? Besides the test automation gadgets, Linux Automation offers engineering services ("Design for mainline"; custom design with off-the-shelf components well-supported by mainline Linux) and the MC-1 was the Embedded World fair demonstrator for the concept. There is a blog post[0], a BSP[1] and even a Youtube video[2] on it, but as the MC-1 itself is not what's being sold, there is no technical documentation of the HW publicly available. If you got any questions regarding the device tree though, just send me an email. :-) [0]: https://www.pengutronix.de/en/software/distrokit.html [1]: https://www.pengutronix.de/de/blog/2020-02-26-embedded_world_2020.html [2]: https://www.youtube.com/watch?v=qs0ljuH3ZkQ Cheers, Ahmad > > Greets > Alex > >> >> >> >>> >>>>> compatible = "pwm-leds"; >>>>> >>>>> - led-red { >>>>> + led-2 { >>>> >>>> Shouldn't this have been led-1 as well or is the numbering "global" ? >>> >>> Also good question. This numbering is for dts only, it usually does >>> not correspond with LEDs on the board, so it could be numbered per >>> led-controller as well? >> >> I'd prefer that it starts by 1. That way it's aligned with PWM channel >> ID. >> >> Thanks for fixing the dtschema warnings by the way! >> >> Cheers, >> Ahmad >> >>> >>> Greets >>> Alex >>> >>>> >>>>> label = "mc1:red:rgb"; >>>>> pwms = <&leds_pwm 1 1000000 0>; >>>>> max-brightness = <255>; >>>>> active-low; >>>>> }; >>>>> >>>>> - led-green { >>>>> + led-3 { >>>>> label = "mc1:green:rgb"; >>>>> pwms = <&leds_pwm 2 1000000 0>; >>>>> max-brightness = <255>; >>>>> active-low; >>>>> }; >>>>> >>>>> - led-blue { >>>>> + led-4 { >>>>> label = "mc1:blue:rgb"; >>>>> pwms = <&leds_pwm 3 1000000 0>; >>>>> max-brightness = <255>; >>>>> >>>> >>>> -- >>>> Pengutronix e.K. | | >>>> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >>>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >>>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >>> >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
Hi Alex On 10/31/20 2:54 PM, Alexander Dahl wrote: > Hei hei, > > On Tue, Oct 27, 2020 at 11:58:10AM +0100, Ahmad Fatoum wrote: >> Hello, >> >> On 10/27/20 11:05 AM, Alexander Dahl wrote: >>> Hello Ahmad, >>> >>> thanks for your feedback, comments below. >>> >> >>>>> - led-rgb { >>>>> + led-controller-2 { >>>> >>>> Is a single RGB LED really a controller? >>> >>> I just followed the recommendations by Rob here. >> >> Do you happen to know if the new multicolor LED support could be used here? >> >> I find it unfortunate that the device tree loses information relevant to humans >> to adhere to a fixed nomenclature. Apparently led-controller isn't even codified >> in the YAML binding (It's just in the examples). If you respin, please add a >> comment that this is a single RGB led. I'd prefer to keep the information >> in the DTB as well though. > > Slightly off-topic, but while I was working on the patch based on your > feedback I tried to find some information on that Linux Automation > MC-1 board. However I could not find any? Is there some website, some > datasheet or maybe a schematic online? The vendor prefix says "Linux > Automation GmbH", but I find only that USB-SD-Mux on their page? > > Greets > Alex I saw that Ahmad Acked this patch but regarding your discussion it seems there are opening questions. Are you going to send an update of it or can I take it ? regards alex > >> >> >> >>> >>>>> compatible = "pwm-leds"; >>>>> >>>>> - led-red { >>>>> + led-2 { >>>> >>>> Shouldn't this have been led-1 as well or is the numbering "global" ? >>> >>> Also good question. This numbering is for dts only, it usually does >>> not correspond with LEDs on the board, so it could be numbered per >>> led-controller as well? >> >> I'd prefer that it starts by 1. That way it's aligned with PWM channel >> ID. >> >> Thanks for fixing the dtschema warnings by the way! >> >> Cheers, >> Ahmad >> >>> >>> Greets >>> Alex >>> >>>> >>>>> label = "mc1:red:rgb"; >>>>> pwms = <&leds_pwm 1 1000000 0>; >>>>> max-brightness = <255>; >>>>> active-low; >>>>> }; >>>>> >>>>> - led-green { >>>>> + led-3 { >>>>> label = "mc1:green:rgb"; >>>>> pwms = <&leds_pwm 2 1000000 0>; >>>>> max-brightness = <255>; >>>>> active-low; >>>>> }; >>>>> >>>>> - led-blue { >>>>> + led-4 { >>>>> label = "mc1:blue:rgb"; >>>>> pwms = <&leds_pwm 3 1000000 0>; >>>>> max-brightness = <255>; >>>>> >>>> >>>> -- >>>> Pengutronix e.K. | | >>>> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >>>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >>>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >>> >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > > _______________________________________________ > Linux-stm32 mailing list > Linux-stm32@st-md-mailman.stormreply.com > https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32 >
Hello Alex, On Mon, Nov 09, 2020 at 12:02:18PM +0100, Alexandre Torgue wrote: > Hi Alex > > On 10/31/20 2:54 PM, Alexander Dahl wrote: > > Hei hei, > > > > On Tue, Oct 27, 2020 at 11:58:10AM +0100, Ahmad Fatoum wrote: > > > Hello, > > > > > > On 10/27/20 11:05 AM, Alexander Dahl wrote: > > > > Hello Ahmad, > > > > > > > > thanks for your feedback, comments below. > > > > > > > > > > > > > - led-rgb { > > > > > > + led-controller-2 { > > > > > > > > > > Is a single RGB LED really a controller? > > > > > > > > I just followed the recommendations by Rob here. > > > > > > Do you happen to know if the new multicolor LED support could be used here? > > > > > > I find it unfortunate that the device tree loses information relevant to humans > > > to adhere to a fixed nomenclature. Apparently led-controller isn't even codified > > > in the YAML binding (It's just in the examples). If you respin, please add a > > > comment that this is a single RGB led. I'd prefer to keep the information > > > in the DTB as well though. > > > > Slightly off-topic, but while I was working on the patch based on your > > feedback I tried to find some information on that Linux Automation > > MC-1 board. However I could not find any? Is there some website, some > > datasheet or maybe a schematic online? The vendor prefix says "Linux > > Automation GmbH", but I find only that USB-SD-Mux on their page? > > > > Greets > > Alex > > I saw that Ahmad Acked this patch but regarding your discussion it seems > there are opening questions. Are you going to send an update of it or can I > take it ? I'll send an update, I already reworked this patch. I'm still waiting for an Ack for the first patch of the series before sending the next iteration, though. :-/ Greets Alex > > regards > alex > > > > > > > > > > > > > > > > > > > > > > > > compatible = "pwm-leds"; > > > > > > - led-red { > > > > > > + led-2 { > > > > > > > > > > Shouldn't this have been led-1 as well or is the numbering "global" ? > > > > > > > > Also good question. This numbering is for dts only, it usually does > > > > not correspond with LEDs on the board, so it could be numbered per > > > > led-controller as well? > > > > > > I'd prefer that it starts by 1. That way it's aligned with PWM channel > > > ID. > > > > > > Thanks for fixing the dtschema warnings by the way! > > > > > > Cheers, > > > Ahmad > > > > > > > > > > > Greets > > > > Alex > > > > > > > > > > > > > > > label = "mc1:red:rgb"; > > > > > > pwms = <&leds_pwm 1 1000000 0>; > > > > > > max-brightness = <255>; > > > > > > active-low; > > > > > > }; > > > > > > - led-green { > > > > > > + led-3 { > > > > > > label = "mc1:green:rgb"; > > > > > > pwms = <&leds_pwm 2 1000000 0>; > > > > > > max-brightness = <255>; > > > > > > active-low; > > > > > > }; > > > > > > - led-blue { > > > > > > + led-4 { > > > > > > label = "mc1:blue:rgb"; > > > > > > pwms = <&leds_pwm 3 1000000 0>; > > > > > > max-brightness = <255>; > > > > > > > > > > > > > > > > -- > > > > > Pengutronix e.K. | | > > > > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > > > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > > > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > > > > > > > > > -- > > > Pengutronix e.K. | | > > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > > > > > _______________________________________________ > > Linux-stm32 mailing list > > Linux-stm32@st-md-mailman.stormreply.com > > https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts index 5700e6b700d3..25d548cb975b 100644 --- a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +++ b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts @@ -36,34 +36,34 @@ stdout-path = &uart4; }; - led-act { + led-controller-1 { compatible = "gpio-leds"; - led-green { + led-1 { label = "mc1:green:act"; gpios = <&gpioa 13 GPIO_ACTIVE_LOW>; linux,default-trigger = "heartbeat"; }; }; - led-rgb { + led-controller-2 { compatible = "pwm-leds"; - led-red { + led-2 { label = "mc1:red:rgb"; pwms = <&leds_pwm 1 1000000 0>; max-brightness = <255>; active-low; }; - led-green { + led-3 { label = "mc1:green:rgb"; pwms = <&leds_pwm 2 1000000 0>; max-brightness = <255>; active-low; }; - led-blue { + led-4 { label = "mc1:blue:rgb"; pwms = <&leds_pwm 3 1000000 0>; max-brightness = <255>;
The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project. DTC arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml CHECK arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml /home/alex/build/linux/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml: led-rgb: 'led-blue', 'led-green', 'led-red' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+' From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml Signed-off-by: Alexander Dahl <post@lespocky.de> --- Notes: v6 -> v7: * split up patch (one per sub arch) * added actual warnings to commit message arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)