| Message ID | 20240816110402.840-1-naoki@radxa.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: rockchip: use "pwm-leds" for multicolor PWM LEDs on Radxa E25 | expand |
Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki: > to make multicolor PWM LEDs behavior more consistent with vendor > kernel[1], use "pwm-leds" for it on Radxa E25. sorry, but that is definitly not a valid reason. A devicetree does describe actual hardware and is not a space for configuration choices. So that whole notion to "match a kernel" is not correct. Looking at https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started the specification table clearly designates the board's LED as "RGB LED" - so one LED and multicolor . Heiko > [1] https://github.com/radxa/kernel/blob/linux-5.10-gen-rkr4.1/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts#L100-L121 > > Fixes: 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25") > Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> > --- > .../boot/dts/rockchip/rk3568-radxa-e25.dts | 36 ++++++++++--------- > 1 file changed, 20 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts > index 72ad74c38a2b..0b527f67bdbd 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts > @@ -12,26 +12,30 @@ aliases { > }; > > pwm-leds { > - compatible = "pwm-leds-multicolor"; > + compatible = "pwm-leds"; > > - multi-led { > - color = <LED_COLOR_ID_RGB>; > + led-red { > + color = <LED_COLOR_ID_RED>; > + default-state = "on"; > + function = LED_FUNCTION_STATUS; > max-brightness = <255>; > + pwms = <&pwm1 0 1000000 0>; > + }; > > - led-red { > - color = <LED_COLOR_ID_RED>; > - pwms = <&pwm1 0 1000000 0>; > - }; > - > - led-green { > - color = <LED_COLOR_ID_GREEN>; > - pwms = <&pwm2 0 1000000 0>; > - }; > + led-green { > + color = <LED_COLOR_ID_GREEN>; > + default-state = "on"; > + function = LED_FUNCTION_STATUS; > + max-brightness = <255>; > + pwms = <&pwm2 0 1000000 0>; > + }; > > - led-blue { > - color = <LED_COLOR_ID_BLUE>; > - pwms = <&pwm12 0 1000000 0>; > - }; > + led-blue { > + color = <LED_COLOR_ID_BLUE>; > + default-state = "on"; > + function = LED_FUNCTION_STATUS; > + max-brightness = <255>; > + pwms = <&pwm12 0 1000000 0>; > }; > }; > >
Hi, On 8/16/24 20:26, Heiko Stübner wrote: > Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki: >> to make multicolor PWM LEDs behavior more consistent with vendor >> kernel[1], use "pwm-leds" for it on Radxa E25. > > sorry, but that is definitly not a valid reason. I see. I'll not change it. > A devicetree does describe actual hardware and is not a space for > configuration choices. So that whole notion to "match a kernel" > is not correct. > > Looking at > https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started > the specification table clearly designates the board's LED as > "RGB LED" - so one LED and multicolor . I understand following behavior is not possible on mainline. https://github.com/radxa-pkg/rsetup/blob/main/config/00-rgb0-rainbow.conf this is not what we(Radxa) want, but we need to follow the rule. (btw wiki.radxa.com is outdated, it's not used anymore) Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. > Heiko > > >> [1] https://github.com/radxa/kernel/blob/linux-5.10-gen-rkr4.1/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts#L100-L121 >> >> Fixes: 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25") >> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >> --- >> .../boot/dts/rockchip/rk3568-radxa-e25.dts | 36 ++++++++++--------- >> 1 file changed, 20 insertions(+), 16 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts >> index 72ad74c38a2b..0b527f67bdbd 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts >> @@ -12,26 +12,30 @@ aliases { >> }; >> >> pwm-leds { >> - compatible = "pwm-leds-multicolor"; >> + compatible = "pwm-leds"; >> >> - multi-led { >> - color = <LED_COLOR_ID_RGB>; >> + led-red { >> + color = <LED_COLOR_ID_RED>; >> + default-state = "on"; >> + function = LED_FUNCTION_STATUS; >> max-brightness = <255>; >> + pwms = <&pwm1 0 1000000 0>; >> + }; >> >> - led-red { >> - color = <LED_COLOR_ID_RED>; >> - pwms = <&pwm1 0 1000000 0>; >> - }; >> - >> - led-green { >> - color = <LED_COLOR_ID_GREEN>; >> - pwms = <&pwm2 0 1000000 0>; >> - }; >> + led-green { >> + color = <LED_COLOR_ID_GREEN>; >> + default-state = "on"; >> + function = LED_FUNCTION_STATUS; >> + max-brightness = <255>; >> + pwms = <&pwm2 0 1000000 0>; >> + }; >> >> - led-blue { >> - color = <LED_COLOR_ID_BLUE>; >> - pwms = <&pwm12 0 1000000 0>; >> - }; >> + led-blue { >> + color = <LED_COLOR_ID_BLUE>; >> + default-state = "on"; >> + function = LED_FUNCTION_STATUS; >> + max-brightness = <255>; >> + pwms = <&pwm12 0 1000000 0>; >> }; >> }; >> >> > > > > >
On 16/08/2024 13:57, FUKAUMI Naoki wrote: > Hi, > > On 8/16/24 20:26, Heiko Stübner wrote: >> Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki: >>> to make multicolor PWM LEDs behavior more consistent with vendor >>> kernel[1], use "pwm-leds" for it on Radxa E25. >> >> sorry, but that is definitly not a valid reason. > > I see. I'll not change it. > >> A devicetree does describe actual hardware and is not a space for >> configuration choices. So that whole notion to "match a kernel" >> is not correct. >> >> Looking at >> https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started >> the specification table clearly designates the board's LED as >> "RGB LED" - so one LED and multicolor . > > I understand following behavior is not possible on mainline. > > https://github.com/radxa-pkg/rsetup/blob/main/config/00-rgb0-rainbow.conf > > this is not what we(Radxa) want, but we need to follow the rule. > > (btw wiki.radxa.com is outdated, it's not used anymore) Your Radxa vendor kernel and whatever you have *MUST* align to the upstream, not the other way. The upstream is the correct source of doing things, not the other way. Best regards, Krzysztof
Hi, On 8/16/24 21:25, Krzysztof Kozlowski wrote: > On 16/08/2024 13:57, FUKAUMI Naoki wrote: >> Hi, >> >> On 8/16/24 20:26, Heiko Stübner wrote: >>> Am Freitag, 16. August 2024, 13:04:02 CEST schrieb FUKAUMI Naoki: >>>> to make multicolor PWM LEDs behavior more consistent with vendor >>>> kernel[1], use "pwm-leds" for it on Radxa E25. >>> >>> sorry, but that is definitly not a valid reason. >> >> I see. I'll not change it. >> >>> A devicetree does describe actual hardware and is not a space for >>> configuration choices. So that whole notion to "match a kernel" >>> is not correct. >>> >>> Looking at >>> https://wiki.radxa.com/Rock3/CM/CM3I/E25/getting_started >>> the specification table clearly designates the board's LED as >>> "RGB LED" - so one LED and multicolor . >> >> I understand following behavior is not possible on mainline. >> >> https://github.com/radxa-pkg/rsetup/blob/main/config/00-rgb0-rainbow.conf >> >> this is not what we(Radxa) want, but we need to follow the rule. >> >> (btw wiki.radxa.com is outdated, it's not used anymore) > > Your Radxa vendor kernel and whatever you have *MUST* align to the > upstream, not the other way. The upstream is the correct source of doing > things, not the other way. could you tell me how to make rainbow color effect with "pwm-leds-multicolor" driver? as far as I know, "pattern" trigger can change (only) brightness, not color. Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. > Best regards, > Krzysztof
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts index 72ad74c38a2b..0b527f67bdbd 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts +++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts @@ -12,26 +12,30 @@ aliases { }; pwm-leds { - compatible = "pwm-leds-multicolor"; + compatible = "pwm-leds"; - multi-led { - color = <LED_COLOR_ID_RGB>; + led-red { + color = <LED_COLOR_ID_RED>; + default-state = "on"; + function = LED_FUNCTION_STATUS; max-brightness = <255>; + pwms = <&pwm1 0 1000000 0>; + }; - led-red { - color = <LED_COLOR_ID_RED>; - pwms = <&pwm1 0 1000000 0>; - }; - - led-green { - color = <LED_COLOR_ID_GREEN>; - pwms = <&pwm2 0 1000000 0>; - }; + led-green { + color = <LED_COLOR_ID_GREEN>; + default-state = "on"; + function = LED_FUNCTION_STATUS; + max-brightness = <255>; + pwms = <&pwm2 0 1000000 0>; + }; - led-blue { - color = <LED_COLOR_ID_BLUE>; - pwms = <&pwm12 0 1000000 0>; - }; + led-blue { + color = <LED_COLOR_ID_BLUE>; + default-state = "on"; + function = LED_FUNCTION_STATUS; + max-brightness = <255>; + pwms = <&pwm12 0 1000000 0>; }; };
to make multicolor PWM LEDs behavior more consistent with vendor kernel[1], use "pwm-leds" for it on Radxa E25. [1] https://github.com/radxa/kernel/blob/linux-5.10-gen-rkr4.1/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts#L100-L121 Fixes: 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25") Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> --- .../boot/dts/rockchip/rk3568-radxa-e25.dts | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-)