Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1363637 > unrolled thread

[PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support

Started bySrinivas Kandagatla <srinivas.kandagatla@linaro.org>
First post2016-03-23 20:50 +0100
Last post2016-03-30 15:00 +0200
Articles 9 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-23 20:50 +0100
    Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support Nicolas Dechesne <nicolas.dechesne@linaro.org> - 2016-03-24 18:00 +0100
      Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds  support Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-29 15:40 +0200
    Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds  support Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-27 08:00 +0200
      Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds  support Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-29 16:30 +0200
        Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds  support Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-29 17:00 +0200
          Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support Nicolas Dechesne <nicolas.dechesne@linaro.org> - 2016-03-29 20:40 +0200
            Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds  support Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-03-29 23:20 +0200
      Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support Rob Herring <robh@kernel.org> - 2016-03-30 15:00 +0200

#1363637 — [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support

FromSrinivas Kandagatla <srinivas.kandagatla@linaro.org>
Date2016-03-23 20:50 +0100
Subject[PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support
Message-ID<rfWWZ-8bL-15@gated-at.bofh.it>
This patch adds support to 4 user leds, wlan and bt led on board.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../arm/boot/dts/qcom-apq8064-dragonboard-600c.dts | 76 ++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts b/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
index 52f742c..4fecd94 100644
--- a/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-dragonboard-600c.dts
@@ -53,6 +53,33 @@
 					bias-disable;
 				};
 			};
+
+			user_leds: user_leds {
+				mux {
+					pins = "gpio3", "gpio7", "gpio10", "gpio11";
+					function = "gpio";
+				};
+
+				conf {
+					pins = "gpio3", "gpio7", "gpio10", "gpio11";
+					function = "gpio";
+					output-low;
+				};
+			};
+		};
+
+		qcom,ssbi@500000 {
+			pmic@0 {
+				mpps@50 {
+					mpp_leds: mpp_leds {
+						pinconf {
+							pins = "mpp7", "mpp8";
+							function = "digital";
+							output-low;
+						};
+					};
+				};
+			};
 		};
 
 		rpm@108000 {
@@ -165,6 +192,55 @@
 			};
 		};
 
+		leds {
+			pinctrl-names = "default";
+			pinctrl-0 = <&user_leds>, <&mpp_leds>;
+
+			compatible = "gpio-leds";
+
+			led@1 {
+				label = "dragonboard-600c:green:user1";
+				gpios = <&tlmm_pinmux 3 GPIO_ACTIVE_HIGH>;
+				linux,default-trigger = "heartbeat";
+				default-state = "off";
+			};
+
+			led@2 {
+				label = "dragonboard-600c:green:user2";
+				gpios = <&tlmm_pinmux 7 GPIO_ACTIVE_HIGH>;
+				linux,default-trigger = "mmc0";
+				default-state = "off";
+			};
+
+			led@3 {
+				label = "dragonboard-600c:green:user3";
+				gpios = <&tlmm_pinmux 10 GPIO_ACTIVE_HIGH>;
+				linux,default-trigger = "mmc1";
+				default-state = "off";
+			};
+
+			led@4 {
+				label = "apq8016-sbc:green:user4";
+				gpios = <&tlmm_pinmux 11 GPIO_ACTIVE_HIGH>;
+				linux,default-trigger = "none";
+				default-state = "off";
+			};
+
+			led@5 {
+				label = "dragonboard-600c:yellow:wlan";
+				gpios = <&pm8921_mpps 7 GPIO_ACTIVE_HIGH>;
+				linux,default-trigger = "wlan";
+				default-state = "off";
+			};
+
+			led@6 {
+				label = "dragonboard-600c:blue:bt";
+				gpios = <&pm8921_mpps 8 GPIO_ACTIVE_HIGH>;
+				linux,default-trigger = "bt";
+				default-state = "off";
+			};
+		};
+
 		pci@1b500000 {
 			status = "okay";
 			vdda-supply = <&pm8921_s3>;
-- 
2.5.0

[toc] | [next] | [standalone]


#1364346

FromNicolas Dechesne <nicolas.dechesne@linaro.org>
Date2016-03-24 18:00 +0100
Message-ID<rggM2-5rb-31@gated-at.bofh.it>
In reply to#1363637
On Wed, Mar 23, 2016 at 8:48 PM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> +                       led@1 {
> +                               label = "dragonboard-600c:green:user1";
> +                               gpios = <&tlmm_pinmux 3 GPIO_ACTIVE_HIGH>;
> +                               linux,default-trigger = "heartbeat";
> +                               default-state = "off";
> +                       };
> +
> +                       led@2 {
> +                               label = "dragonboard-600c:green:user2";
> +                               gpios = <&tlmm_pinmux 7 GPIO_ACTIVE_HIGH>;
> +                               linux,default-trigger = "mmc0";
> +                               default-state = "off";
> +                       };
> +
> +                       led@3 {
> +                               label = "dragonboard-600c:green:user3";
> +                               gpios = <&tlmm_pinmux 10 GPIO_ACTIVE_HIGH>;
> +                               linux,default-trigger = "mmc1";
> +                               default-state = "off";
> +                       };
> +
> +                       led@4 {
> +                               label = "apq8016-sbc:green:user4";

typo ^^

> +                               gpios = <&tlmm_pinmux 11 GPIO_ACTIVE_HIGH>;
> +                               linux,default-trigger = "none";
> +                               default-state = "off";
> +                       };

also, if i read the 96boards specs correctly, the LED are labeled
3-2-1-0 , if you look at the board with the LEDs on the bottom. With
this DT config, you end up with 0-1-2-3 instead.

also, they should be labeled 0 to 3, not 1 to 4.

[toc] | [prev] | [next] | [standalone]


#1366296 — Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support

FromSrinivas Kandagatla <srinivas.kandagatla@linaro.org>
Date2016-03-29 15:40 +0200
SubjectRe: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support
Message-ID<ri22g-6TC-39@gated-at.bofh.it>
In reply to#1364346
Thanks Nico,

On 24/03/16 16:51, Nicolas Dechesne wrote:
> On Wed, Mar 23, 2016 at 8:48 PM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>> +                       led@1 {
>> +                               label = "dragonboard-600c:green:user1";
>> +                               gpios = <&tlmm_pinmux 3 GPIO_ACTIVE_HIGH>;
>> +                               linux,default-trigger = "heartbeat";
>> +                               default-state = "off";
>> +                       };
>> +
>> +                       led@2 {
>> +                               label = "dragonboard-600c:green:user2";
>> +                               gpios = <&tlmm_pinmux 7 GPIO_ACTIVE_HIGH>;
>> +                               linux,default-trigger = "mmc0";
>> +                               default-state = "off";
>> +                       };
>> +
>> +                       led@3 {
>> +                               label = "dragonboard-600c:green:user3";
>> +                               gpios = <&tlmm_pinmux 10 GPIO_ACTIVE_HIGH>;
>> +                               linux,default-trigger = "mmc1";
>> +                               default-state = "off";
>> +                       };
>> +
>> +                       led@4 {
>> +                               label = "apq8016-sbc:green:user4";
>
> typo ^^
>
Yep, Will fix this.
>> +                               gpios = <&tlmm_pinmux 11 GPIO_ACTIVE_HIGH>;
>> +                               linux,default-trigger = "none";
>> +                               default-state = "off";
>> +                       };
>
> also, if i read the 96boards specs correctly, the LED are labeled
> 3-2-1-0 , if you look at the board with the LEDs on the bottom. With
> this DT config, you end up with 0-1-2-3 instead.
>
> also, they should be labeled 0 to 3, not 1 to 4.

I agree Will fix it. The structure was copied from the 8016.

Its strange that DB410c LEDS labels on the board are from 1 to 4 rather 
than 0 to 3, on the other hand DB600c does not have any labels as such.

For Now I can change DB600c to start with 0 rather than 1.


>

[toc] | [prev] | [next] | [standalone]


#1365058 — Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support

FromBjorn Andersson <bjorn.andersson@linaro.org>
Date2016-03-27 08:00 +0200
SubjectRe: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support
Message-ID<rhbTX-3Lh-1@gated-at.bofh.it>
In reply to#1363637
On Wed 23 Mar 12:48 PDT 2016, Srinivas Kandagatla wrote:

> This patch adds support to 4 user leds, wlan and bt led on board.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

I'm not fond of the overly complicated names; and I think it should at
least be shortened to "db600c:...".

Tested this on my DB600c, seems to work, except the WiFi/BT triggers,
see comments below.

> +		leds {
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&user_leds>, <&mpp_leds>;
> +
> +			compatible = "gpio-leds";
> +
> +			led@1 {
> +				label = "dragonboard-600c:green:user1";
> +				gpios = <&tlmm_pinmux 3 GPIO_ACTIVE_HIGH>;
> +				linux,default-trigger = "heartbeat";
> +				default-state = "off";
> +			};
> +
> +			led@2 {
> +				label = "dragonboard-600c:green:user2";
> +				gpios = <&tlmm_pinmux 7 GPIO_ACTIVE_HIGH>;
> +				linux,default-trigger = "mmc0";
> +				default-state = "off";
> +			};
> +
> +			led@3 {
> +				label = "dragonboard-600c:green:user3";
> +				gpios = <&tlmm_pinmux 10 GPIO_ACTIVE_HIGH>;
> +				linux,default-trigger = "mmc1";
> +				default-state = "off";
> +			};
> +
> +			led@4 {
> +				label = "apq8016-sbc:green:user4";
> +				gpios = <&tlmm_pinmux 11 GPIO_ACTIVE_HIGH>;
> +				linux,default-trigger = "none";
> +				default-state = "off";
> +			};
> +
> +			led@5 {
> +				label = "dragonboard-600c:yellow:wlan";
> +				gpios = <&pm8921_mpps 7 GPIO_ACTIVE_HIGH>;
> +				linux,default-trigger = "wlan";

This should either be "phy0rx", "phy0tx", "phy0assoc" or "phy0radio". TX
does not seem to work, so this should be debugged; "assoc" is probably
the one that makes most sense.

> +				default-state = "off";
> +			};
> +
> +			led@6 {
> +				label = "dragonboard-600c:blue:bt";
> +				gpios = <&pm8921_mpps 8 GPIO_ACTIVE_HIGH>;
> +				linux,default-trigger = "bt";

This should be "hci0-power".

> +				default-state = "off";
> +			};
> +		};
> +

Regards,
Bjorn

[toc] | [prev] | [next] | [standalone]


#1366328 — Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support

FromSrinivas Kandagatla <srinivas.kandagatla@linaro.org>
Date2016-03-29 16:30 +0200
SubjectRe: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support
Message-ID<ri2OB-7x2-5@gated-at.bofh.it>
In reply to#1365058
Thanks Bjorn,

On 27/03/16 06:50, Bjorn Andersson wrote:
> On Wed 23 Mar 12:48 PDT 2016, Srinivas Kandagatla wrote:
>
>> This patch adds support to 4 user leds, wlan and bt led on board.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> I'm not fond of the overly complicated names; and I think it should at
> least be shortened to "db600c:...".

I agree, I will fix this in next version.

>
> Tested this on my DB600c, seems to work, except the WiFi/BT triggers,
> see comments below.
>
>> +		leds {
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&user_leds>, <&mpp_leds>;
>> +
>> +			compatible = "gpio-leds";
>> +
>> +			led@1 {
>> +				label = "dragonboard-600c:green:user1";
>> +				gpios = <&tlmm_pinmux 3 GPIO_ACTIVE_HIGH>;
>> +				linux,default-trigger = "heartbeat";
>> +				default-state = "off";
>> +			};
>> +
>> +			led@2 {
>> +				label = "dragonboard-600c:green:user2";
>> +				gpios = <&tlmm_pinmux 7 GPIO_ACTIVE_HIGH>;
>> +				linux,default-trigger = "mmc0";
>> +				default-state = "off";
>> +			};
>> +
>> +			led@3 {
>> +				label = "dragonboard-600c:green:user3";
>> +				gpios = <&tlmm_pinmux 10 GPIO_ACTIVE_HIGH>;
>> +				linux,default-trigger = "mmc1";
>> +				default-state = "off";
>> +			};
>> +
>> +			led@4 {
>> +				label = "apq8016-sbc:green:user4";
>> +				gpios = <&tlmm_pinmux 11 GPIO_ACTIVE_HIGH>;
>> +				linux,default-trigger = "none";
>> +				default-state = "off";
>> +			};
>> +
>> +			led@5 {
>> +				label = "dragonboard-600c:yellow:wlan";
>> +				gpios = <&pm8921_mpps 7 GPIO_ACTIVE_HIGH>;
>> +				linux,default-trigger = "wlan";
>
> This should either be "phy0rx", "phy0tx", "phy0assoc" or "phy0radio". TX
> does not seem to work, so this should be debugged; "assoc" is probably
> the one that makes most sense.


Am ok, to change, did you get activity leds works with any of these 
strings with WLAN or BT?

phy0rx/tx seems to be bit more generic and atleast the name looks bit 
non-specific to wlan.

These names should be documented somewhere, Its very difficult to find 
which names to use unless you read the code.

It would be nice to just provide a phandle to the device which 
led-trigger should use, which makes it clear and explicit.



>
>> +				default-state = "off";
>> +			};
>> +
>> +			led@6 {
>> +				label = "dragonboard-600c:blue:bt";
>> +				gpios = <&pm8921_mpps 8 GPIO_ACTIVE_HIGH>;
>> +				linux,default-trigger = "bt";
>
> This should be "hci0-power".

Does this trigger work for you?
Name is bit misleading though.

Thanks,
srini
>
>> +				default-state = "off";
>> +			};
>> +		};
>> +
>
> Regards,
> Bjorn
>

[toc] | [prev] | [next] | [standalone]


#1366357 — Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support

FromBjorn Andersson <bjorn.andersson@linaro.org>
Date2016-03-29 17:00 +0200
SubjectRe: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support
Message-ID<ri3hE-7IL-7@gated-at.bofh.it>
In reply to#1366328
On Tue 29 Mar 07:20 PDT 2016, Srinivas Kandagatla wrote:

> Thanks Bjorn,
> 
> On 27/03/16 06:50, Bjorn Andersson wrote:
> >On Wed 23 Mar 12:48 PDT 2016, Srinivas Kandagatla wrote:
[..]
> >>+
> >>+			led@5 {
> >>+				label = "dragonboard-600c:yellow:wlan";
> >>+				gpios = <&pm8921_mpps 7 GPIO_ACTIVE_HIGH>;
> >>+				linux,default-trigger = "wlan";
> >
> >This should either be "phy0rx", "phy0tx", "phy0assoc" or "phy0radio". TX
> >does not seem to work, so this should be debugged; "assoc" is probably
> >the one that makes most sense.
> 
> 
> Am ok, to change, did you get activity leds works with any of these strings
> with WLAN or BT?
> 

I did test them; rx, assoc and radio seems to work. Tx does not tickle
the led, but should probably.

I didn't find the spec for the baord, so I'm not sure what the LEDs
should indicate. But it feels like radio or assoc makes most sense, and
follows what we can do with the BT led.

> phy0rx/tx seems to be bit more generic and atleast the name looks bit
> non-specific to wlan.
> 
> These names should be documented somewhere, Its very difficult to find which
> names to use unless you read the code.
> 
> It would be nice to just provide a phandle to the device which led-trigger
> should use, which makes it clear and explicit.
> 

I agree, I was surprised by their naming. The phandle solution would
have been nice, except that you have no way from sysfs to pick which
node to follow.

> 
> 
> >
> >>+				default-state = "off";
> >>+			};
> >>+
> >>+			led@6 {
> >>+				label = "dragonboard-600c:blue:bt";
> >>+				gpios = <&pm8921_mpps 8 GPIO_ACTIVE_HIGH>;
> >>+				linux,default-trigger = "bt";
> >
> >This should be "hci0-power".
> 
> Does this trigger work for you?
> Name is bit misleading though.
> 

I don't think I tested it, but it is the only bt led trigger exposed by
the bluetooth framework. I don't think we have any other hci devices on
this board, so it should be fine.

Regards,
Bjorn

[toc] | [prev] | [next] | [standalone]


#1366544

FromNicolas Dechesne <nicolas.dechesne@linaro.org>
Date2016-03-29 20:40 +0200
Message-ID<ri6Iy-1K2-13@gated-at.bofh.it>
In reply to#1366357
On Tue, Mar 29, 2016 at 4:51 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>> Am ok, to change, did you get activity leds works with any of these strings
>> with WLAN or BT?
>>
>
> I did test them; rx, assoc and radio seems to work. Tx does not tickle
> the led, but should probably.
>
> I didn't find the spec for the baord, so I'm not sure what the LEDs
> should indicate. But it feels like radio or assoc makes most sense, and
> follows what we can do with the BT led.

this is in the 96boards 'specs' at [1], which states

-
The User LEDs shall be directly programmable from the SoC.
1. WiFi activity LED
2. Bluetooth activity LED
-

[1] http://www.96boards.org/ce-specification

[toc] | [prev] | [next] | [standalone]


#1366715 — Re: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support

FromBjorn Andersson <bjorn.andersson@linaro.org>
Date2016-03-29 23:20 +0200
SubjectRe: [PATCH 10/12] ARM: dts: dragonboard-600c: Add on board leds support
Message-ID<ri9do-3wv-11@gated-at.bofh.it>
In reply to#1366544
On Tue 29 Mar 11:32 PDT 2016, Nicolas Dechesne wrote:

> On Tue, Mar 29, 2016 at 4:51 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >> Am ok, to change, did you get activity leds works with any of these strings
> >> with WLAN or BT?
> >>
> >
> > I did test them; rx, assoc and radio seems to work. Tx does not tickle
> > the led, but should probably.
> >
> > I didn't find the spec for the baord, so I'm not sure what the LEDs
> > should indicate. But it feels like radio or assoc makes most sense, and
> > follows what we can do with the BT led.
> 
> this is in the 96boards 'specs' at [1], which states
> 
> -
> The User LEDs shall be directly programmable from the SoC.
> 1. WiFi activity LED
> 2. Bluetooth activity LED
> -
> 
> [1] http://www.96boards.org/ce-specification

Thanks Nico. I presume "activity" here means it should be off and blink
upon data being transmitted in either direction?

Sounds like we need to plan some work to extend the existing triggers
with a phyXactivity and hciX-activity.


For now I think we should go with phy0assoc and hci-power though, so we
have something related to WiFi and BT.

Regards,
Bjorn

[toc] | [prev] | [next] | [standalone]


#1367185

FromRob Herring <robh@kernel.org>
Date2016-03-30 15:00 +0200
Message-ID<rinT4-5vh-7@gated-at.bofh.it>
In reply to#1365058
On Sun, Mar 27, 2016 at 12:50 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Wed 23 Mar 12:48 PDT 2016, Srinivas Kandagatla wrote:
>
>> This patch adds support to 4 user leds, wlan and bt led on board.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> I'm not fond of the overly complicated names; and I think it should at
> least be shortened to "db600c:...".

These names are exposed to userspace. They should be the same across
all 96boards. Same goes for UART numbering and I2C bus labels on the
connector.

> Tested this on my DB600c, seems to work, except the WiFi/BT triggers,
> see comments below.
>
>> +             leds {
>> +                     pinctrl-names = "default";
>> +                     pinctrl-0 = <&user_leds>, <&mpp_leds>;
>> +
>> +                     compatible = "gpio-leds";
>> +
>> +                     led@1 {

unit-address w/o reg property will warn on new dtc. I'd suggest
something like "userX-led".


>> +                             label = "dragonboard-600c:green:user1";
>> +                             gpios = <&tlmm_pinmux 3 GPIO_ACTIVE_HIGH>;
>> +                             linux,default-trigger = "heartbeat";
>> +                             default-state = "off";
>> +                     };

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web