* [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
@ 2015-12-30 11:16 Romain Perier
2015-12-30 15:20 ` Heiko Stübner
0 siblings, 1 reply; 7+ messages in thread
From: Romain Perier @ 2015-12-30 11:16 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Romain Perier <romain.perier@gmail.com>
---
arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
index c5453a0..a33020f 100644
--- a/arch/arm/boot/dts/rk3288-rock2-square.dts
+++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
@@ -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";
--
2.5.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
2015-12-30 11:16 [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square Romain Perier
@ 2015-12-30 15:20 ` Heiko Stübner
2015-12-30 15:45 ` Sjoerd Simons
0 siblings, 1 reply; 7+ messages in thread
From: Heiko Stübner @ 2015-12-30 15:20 UTC (permalink / raw)
To: linux-arm-kernel
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
;-)
> Signed-off-by: Romain Perier <romain.perier@gmail.com>
> ---
> arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts
> b/arch/arm/boot/dts/rk3288-rock2-square.dts index c5453a0..a33020f 100644
> --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
> +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
> @@ -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";
> + };
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
> + };
> +
> +
> sound {
> compatible = "simple-audio-card";
> simple-audio-card,name = "SPDIF";
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
2015-12-30 15:20 ` Heiko Stübner
@ 2015-12-30 15:45 ` Sjoerd Simons
2015-12-30 15:53 ` Heiko Stübner
0 siblings, 1 reply; 7+ messages in thread
From: Sjoerd Simons @ 2015-12-30 15:45 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2015-12-30 at 16:20 +0100, Heiko St?bner wrote:
> 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?
> ;-)
>
> > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> > ---
> > ?arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++
> > ?1 file changed, 17 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > b/arch/arm/boot/dts/rk3288-rock2-square.dts index c5453a0..a33020f
> > 100644
> > --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
> > @@ -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";
> > + };
>
> 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.
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.
> Thanks
> Heiko
>
> > + };
> > +
> > +
> > ? sound {
> > ? compatible = "simple-audio-card";
> > ? simple-audio-card,name = "SPDIF";
>
--
Sjoerd Simons
Collabora Ltd.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
2015-12-30 15:45 ` Sjoerd Simons
@ 2015-12-30 15:53 ` Heiko Stübner
2015-12-30 16:07 ` Sjoerd Simons
2015-12-30 16:07 ` Romain Perier
0 siblings, 2 replies; 7+ messages in thread
From: Heiko Stübner @ 2015-12-30 15:53 UTC (permalink / raw)
To: linux-arm-kernel
Am Mittwoch, 30. Dezember 2015, 16:45:43 schrieb Sjoerd Simons:
> On Wed, 2015-12-30 at 16:20 +0100, Heiko St?bner wrote:
> > Am Mittwoch, 30. Dezember 2015, 12:16:59 schrieb Romain Perier:
> >
> > even a oneliner is generally preferred, compared to no commit message
> > at all
> > ;-)
> >
> > > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> > > ---
> > > arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++
> > > 1 file changed, 17 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > > b/arch/arm/boot/dts/rk3288-rock2-square.dts index c5453a0..a33020f
> > > 100644
> > > --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > > +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
> > > @@ -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";
> > > + };
> >
> > 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.
>
> 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.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
2015-12-30 15:53 ` Heiko Stübner
@ 2015-12-30 16:07 ` Sjoerd Simons
2015-12-30 16:07 ` Romain Perier
1 sibling, 0 replies; 7+ messages in thread
From: Sjoerd Simons @ 2015-12-30 16:07 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2015-12-30 at 16:53 +0100, Heiko St?bner wrote:
> Am Mittwoch, 30. Dezember 2015, 16:45:43 schrieb Sjoerd Simons:
> > On Wed, 2015-12-30 at 16:20 +0100, Heiko St?bner wrote:
> > >?
> 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.
> ----
Looks good to me, with that commit message feel free to add:
Reviewed-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> Heiko
>
--
Sjoerd Simons
Collabora Ltd.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
2015-12-30 15:53 ` Heiko Stübner
2015-12-30 16:07 ` Sjoerd Simons
@ 2015-12-30 16:07 ` Romain Perier
2015-12-30 16:43 ` Heiko Stübner
1 sibling, 1 reply; 7+ messages in thread
From: Romain Perier @ 2015-12-30 16:07 UTC (permalink / raw)
To: linux-arm-kernel
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 <heiko@sntech.de>:
>> > even a oneliner is generally preferred, compared to no commit message
>> > at all
>> > ;-)
Well, lack of inspiration :P
No, just kidding, I agree.
>> >
>> > > Signed-off-by: Romain Perier <romain.perier@gmail.com>
>> > > ---
>> > > arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++
>> > > 1 file changed, 17 insertions(+)
>> > >
>> > > diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts
>> > > b/arch/arm/boot/dts/rk3288-rock2-square.dts index c5453a0..a33020f
>> > > 100644
>> > > --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
>> > > +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
>> > > @@ -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";
>> > > + };
>> >
>> > 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 ...).
I used the schematic and the vendor devicetree to retrieve these informations.
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
2015-12-30 16:07 ` Romain Perier
@ 2015-12-30 16:43 ` Heiko Stübner
0 siblings, 0 replies; 7+ messages in thread
From: Heiko Stübner @ 2015-12-30 16:43 UTC (permalink / raw)
To: linux-arm-kernel
Am Mittwoch, 30. Dezember 2015, 17:07:54 schrieb Romain Perier:
> 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 <heiko@sntech.de>:
> >> > even a oneliner is generally preferred, compared to no commit message
> >> > at all
> >> > ;-)
>
> Well, lack of inspiration :P
> No, just kidding, I agree.
>
> >> > > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> >> > > ---
> >> > >
> >> > > arch/arm/boot/dts/rk3288-rock2-square.dts | 17 +++++++++++++++++
> >> > > 1 file changed, 17 insertions(+)
> >> > >
> >> > > diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts
> >> > > b/arch/arm/boot/dts/rk3288-rock2-square.dts index c5453a0..a33020f
> >> > > 100644
> >> > > --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
> >> > > +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
> >> > > @@ -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";
> >> > > + };
> >> >
> >> > 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 ...).
>
> I used the schematic and the vendor devicetree to retrieve these
> informations.
> > 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)
yep, bringing everything together makes sense :-)
Heiko
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-30 16:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-30 11:16 [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square Romain Perier
2015-12-30 15:20 ` Heiko Stübner
2015-12-30 15:45 ` Sjoerd Simons
2015-12-30 15:53 ` Heiko Stübner
2015-12-30 16:07 ` Sjoerd Simons
2015-12-30 16:07 ` Romain Perier
2015-12-30 16:43 ` Heiko Stübner
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).