Re: [PATCH] ARM: dts: rockchip: Adding LEDs handling via leds-gpio for the Radxa Rock2 Square

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxx>
> > ---
> >  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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux