linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: heiko@sntech.de (Heiko Stübner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square
Date: Wed, 30 Dec 2015 16:53:51 +0100	[thread overview]
Message-ID: <3020768.zersTrE99c@diego> (raw)
In-Reply-To: <1451490343.11453.28.camel@collabora.co.uk>

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

  reply	other threads:[~2015-12-30 15:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2015-12-30 16:07       ` Sjoerd Simons
2015-12-30 16:07       ` Romain Perier
2015-12-30 16:43         ` Heiko Stübner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3020768.zersTrE99c@diego \
    --to=heiko@sntech.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).