ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
Subsystems (2)
PATCH 1/1 ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
--- arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
arch/arm/boot/dts/rk3288-rock2-square.dts +17 -0
@@ -56,6 +56,23 @@ pinctrl-0 = <&ir_int>; }; + gpio-leds { + compatible = "gpio-leds"; + + heartbeat { + gpios = <&gpio7 15 GPIO_ACTIVE_LOW>; + label = "rock2:green:heartbeat"; + linux,default-trigger = "heartbeat"; + }; + + mmc { + gpios = <&gpio0 11 GPIO_ACTIVE_LOW>; + label = "rock2:blue:mmc"; + linux,default-trigger = "mmc0"; + }; + }; + + sound { compatible = "simple-audio-card"; simple-audio-card,name = "SPDIF";
Signature · 1 line
2.5.0
Heiko Stübner
maintainerPATCH 1/1 ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
Hi Romain,
Am Mittwoch, 30. Dezember 2015, 12:16:59 schrieb Romain Perier:
even a oneliner is generally preferred, compared to no commit message at all ;-)
Quoted 17 lines
Signed-off-by Romain Perier <[email protected]>--- arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)(quoted diff) +0
-0
b/arch/arm/boot/dts/rk3288-rock2-square.dtsindex c5453a0..a33020f 100644arch/arm/boot/dts/rk3288-rock2-square.dts +4
-0@@ -56,6 +56,23 @@ pinctrl-0 = <&ir_int>; }; + gpio-leds { + compatible = "gpio-leds"; + + heartbeat {
arch/arm/boot/dts/rk3288-rock2-square.dts line 72
Quoted 10 lines
arch/arm/boot/dts/rk3288-rock2-square.dts +10
-0+ gpios = <&gpio7 15 GPIO_ACTIVE_LOW>; + label = "rock2:green:heartbeat"; + linux,default-trigger = "heartbeat"; + }; + + mmc { + gpios = <&gpio0 11 GPIO_ACTIVE_LOW>; + label = "rock2:blue:mmc"; + linux,default-trigger = "mmc0"; + };
the rock2 core schematics seem to not list these leds at all (especially when looking at the gpio-side). But looking at the schematics my guess would be led_state1 and led_state2, so the naming should reflect that (rock2:green:state1 ...).
Also I'd like to refrain from encoding user-specific configurations in the devicetree - aka please do a default trigger of "off" for those generic leds.
Thanks
Heiko
Quoted 6 lines
arch/arm/boot/dts/rk3288-rock2-square.dts +3
-0+ }; + + sound { compatible = "simple-audio-card"; simple-audio-card,name = "SPDIF";
PATCH 1/1 ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
Quoted 44 lines · Heiko St?bner
Hi Romain,
Am Mittwoch, 30. Dezember 2015, 12:16:59 schrieb Romain Perier:
even a oneliner is generally preferred, compared to no commit message at all?
;-)Quoted 18 lines
Signed-off-by Romain Perier <[email protected]>---
?arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++ ?1 file changed, 17 insertions(+)(quoted diff) +0
-0
b/arch/arm/boot/dts/rk3288-rock2-square.dtsindex c5453a0..a33020f 100644arch/arm/boot/dts/rk3288-rock2-square.dts +4
-0@@ -56,6 +56,23 @@ ? pinctrl-0 = <&ir_int>; ? }; + gpio-leds { + compatible = "gpio-leds"; + + heartbeat {arch/arm/boot/dts/rk3288-rock2-square.dts line 72
Quoted 10 lines
arch/arm/boot/dts/rk3288-rock2-square.dts +10
-0+ gpios = <&gpio7 15 GPIO_ACTIVE_LOW>; + label = "rock2:green:heartbeat"; + linux,default-trigger = "heartbeat"; + }; + + mmc { + gpios = <&gpio0 11 GPIO_ACTIVE_LOW>; + label = "rock2:blue:mmc"; + linux,default-trigger = "mmc0"; + };the rock2 core schematics seem to not list these leds at all (especially when?
looking at the gpio-side). But looking at the schematics my guess would be?led_state1andled_state2, so the naming should reflect that? (rock2:green:state1 ...).
Quoted 4 lines
Also I'd like to refrain from encoding user-specific configurations in the?
devicetree - aka please do a default trigger of "off" for those generic leds.
Seems a grey area. The vendor kernel configures these LEDs with the same default triggers as Romains patch does, so one could argue these are the intended usages for those LEDs (as such it's hardware description not use-specific configuration?).?
In any case having sensible default triggers (e.g. having the default mainline behaviour act the same as the vendor behaviour) seems quite a lot more useful then keeping these LEDs off and forcing usespace to set them up.
Quoted 10 lines
Thanks
HeikoQuoted 6 lines
arch/arm/boot/dts/rk3288-rock2-square.dts +3
-0+ }; + + ? sound { ? compatible = "simple-audio-card"; ? simple-audio-card,name = "SPDIF";
Signature · 2 lines
Sjoerd Simons Collabora Ltd.
Heiko Stübner
maintainerPATCH 1/1 ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
Am Mittwoch, 30. Dezember 2015, 16:45:43 schrieb Sjoerd Simons:
Quoted 53 lines · Heiko St?bner
Quoted 42 lines
Am Mittwoch, 30. Dezember 2015, 12:16:59 schrieb Romain Perier:
even a oneliner is generally preferred, compared to no commit message at all
;-)Quoted 18 lines
Signed-off-by Romain Perier <[email protected]>--- arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)(quoted diff) +0
-0
b/arch/arm/boot/dts/rk3288-rock2-square.dtsindex c5453a0..a33020f 100644arch/arm/boot/dts/rk3288-rock2-square.dts +4
-0@@ -56,6 +56,23 @@ pinctrl-0 = <&ir_int>; }; + gpio-leds { + compatible = "gpio-leds"; + + heartbeat {arch/arm/boot/dts/rk3288-rock2-square.dts line 72
Quoted 10 lines
arch/arm/boot/dts/rk3288-rock2-square.dts +10
-0+ gpios = <&gpio7 15 GPIO_ACTIVE_LOW>; + label = "rock2:green:heartbeat"; + linux,default-trigger = "heartbeat"; + }; + + mmc { + gpios = <&gpio0 11 GPIO_ACTIVE_LOW>; + label = "rock2:blue:mmc"; + linux,default-trigger = "mmc0"; + };the rock2 core schematics seem to not list these leds at all (especially when
looking at the gpio-side). But looking at the schematics my guess would beled_state1andled_state2, so the naming should reflect that (rock2:green:state1 ...).Quoted 4 lines
Also I'd like to refrain from encoding user-specific configurations in the
devicetree - aka please do a default trigger of "off" for those generic leds.Seems a grey area. The vendor kernel configures these LEDs with the same default triggers as Romains patch does, so one could argue these are the intended usages for those LEDs (as such it's hardware description not use-specific configuration?).
Quoted 4 lines
In any case having sensible default triggers (e.g. having the default mainline behaviour act the same as the vendor behaviour) seems quite a lot more useful then keeping these LEDs off and forcing usespace to set them up.
I only looked at the schematics, so missed that. While personally I'd still like having the "christmas-tree" off by default, Sjoerd's argument for the trigger-choice sounds sensible.
Hint for next time: Describing that makes a good commit message
Anybody opposed to me adding the following commit message?
----
Describe the two user-controllable LEDs on Rock2-square boards. The default-triggers mimic the behaviour of the vendor-kernel to keep functionalities in sync.
----
Heiko
PATCH 1/1 ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
Quoted 10 lines · Heiko St?bner
Am Mittwoch, 30. Dezember 2015, 16:45:43 schrieb Sjoerd Simons:
Quoted 2 lines · Heiko St?bner
Quoted 1 line
?
I only looked at the schematics, so missed that. While personally I'd still?
like having the "christmas-tree" off by default, Sjoerd's argument for the?
trigger-choice sounds sensible.
Hint for next time: Describing that makes a good commit message
Quoted 7 lines
Anybody opposed to me adding the following commit message?
----
Describe the two user-controllable LEDs on Rock2-square boards. The default-triggers mimic the behaviour of the vendor-kernel to keep functionalities in sync.
----
Looks good to me, with that commit message feel free to add:
Quoted 3 lines
Heiko
Signature · 2 lines
Sjoerd Simons Collabora Ltd.
PATCH 1/1 ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
In reply to Heiko StübnerHi all,
I agree with Sjoerd, that's more useful to keep these LEDs in sync with the default vendor behaviour than put their default state to off and ask the user to manipulate them explicitly... in the worst case, he can still change the default state from sysfs...
2015-12-30 16:53 GMT+01:00 Heiko St?bner <[email protected]>:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
even a oneliner is generally preferred, compared to no commit message at all
;-)
Well, lack of inspiration :P
No, just kidding, I agree.
Quoted 28 lines
Quoted 28 lines
Quoted 28 lines
Quoted 28 lines
Signed-off-by Romain Perier <[email protected]>--- arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)(quoted diff) +0
-0
b/arch/arm/boot/dts/rk3288-rock2-square.dtsindex c5453a0..a33020f 100644arch/arm/boot/dts/rk3288-rock2-square.dts +13
-0@@ -56,6 +56,23 @@ pinctrl-0 = <&ir_int>; }; + gpio-leds { + compatible = "gpio-leds"; + + heartbeat { + gpios = <&gpio7 15 GPIO_ACTIVE_LOW>; + label = "rock2:green:heartbeat"; + linux,default-trigger = "heartbeat"; + }; + + mmc { + gpios = <&gpio0 11 GPIO_ACTIVE_LOW>; + label = "rock2:blue:mmc"; + linux,default-trigger = "mmc0";
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/rk3288-rock2-square.dts line 72
Quoted 1 line
arch/arm/boot/dts/rk3288-rock2-square.dts +1
-0+ };the rock2 core schematics seem to not list these leds at all (especially when
looking at the gpio-side). But looking at the schematics my guess would beled_state1andled_state2, so the naming should reflect that (rock2:green:state1 ...).
I used the schematic and the vendor devicetree to retrieve these informations.
Quoted 7 lines
Anybody opposed to me adding the following commit message?
----
Describe the two user-controllable LEDs on Rock2-square boards. The default-triggers mimic the behaviour of the vendor-kernel to keep functionalities in sync.
----
+1
Please do.
Do you want a v2 patch ? (the label of these leds is wrong)
Romain
Heiko Stübner
maintainerPATCH 1/1 ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
Am Mittwoch, 30. Dezember 2015, 17:07:54 schrieb Romain Perier:
Quoted 64 lines
Hi all,
I agree with Sjoerd, that's more useful to keep these LEDs in sync with the default vendor behaviour than put their default state to off and ask the user to manipulate them explicitly... in the worst case, he can still change the default state from sysfs...
2015-12-30 16:53 GMT+01:00 Heiko St?bner <[email protected]>:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
even a oneliner is generally preferred, compared to no commit message at all
;-)Well, lack of inspiration :P
No, just kidding, I agree.Quoted 30 lines
Quoted 30 lines
Quoted 30 lines
Quoted 30 lines
Signed-off-by Romain Perier <[email protected]>---
arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)(quoted diff) +0
-0
b/arch/arm/boot/dts/rk3288-rock2-square.dtsindex c5453a0..a33020f 100644arch/arm/boot/dts/rk3288-rock2-square.dts +13
-0@@ -56,6 +56,23 @@ pinctrl-0 = <&ir_int>; }; + gpio-leds { + compatible = "gpio-leds"; + + heartbeat { + gpios = <&gpio7 15 GPIO_ACTIVE_LOW>; + label = "rock2:green:heartbeat"; + linux,default-trigger = "heartbeat"; + }; + + mmc { + gpios = <&gpio0 11 GPIO_ACTIVE_LOW>; + label = "rock2:blue:mmc"; + linux,default-trigger = "mmc0";Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/rk3288-rock2-square.dts line 74
Quoted 1 line
arch/arm/boot/dts/rk3288-rock2-square.dts +1
-0+ };the rock2 core schematics seem to not list these leds at all (especially when
looking at the gpio-side). But looking at the schematics my guess would beled_state1andled_state2, so the naming should reflect that (rock2:green:state1 ...).I used the schematic and the vendor devicetree to retrieve these informations.
Quoted 7 lines
Anybody opposed to me adding the following commit message?
----
Describe the two user-controllable LEDs on Rock2-square boards. The default-triggers mimic the behaviour of the vendor-kernel to keep functionalities in sync.
----
Quoted 4 lines
+1
Please do.Do you want a v2 patch ? (the label of these leds is wrong)
yep, bringing everything together makes sense :-)
Heiko