linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node
@ 2017-12-01 16:27 Chen-Yu Tsai
  2017-12-05  6:15 ` Jagan Teki
  2017-12-05 10:05 ` Maxime Ripard
  0 siblings, 2 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2017-12-01 16:27 UTC (permalink / raw)
  To: linux-arm-kernel

The Bananapi-M64 has 3 LEDS in red, green, and blue. These are toggled
via GPIO lines, which drive transistors that control current across the
LEDS. The red LED is by default on, via an additional pull-up on the
control line. We consider this means that it is a power indicator.
So we set the "default-on" property for it.

The pingroups the GPIO lines belong to require external regulators be
enabled to be able to drive the GPIO high. These regulators also have
other purposes. However the pin controller does not have bindings for
regulators. Here we just set them to always-on.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index 4a8d3f83a36e..a6975670cd1c 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -60,6 +60,26 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	leds {
+		compatible = "gpio-leds";
+
+		pwr-led {
+			label = "bananapi-m64:red:pwr";
+			gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* PD24 */
+			default-state = "on";
+		};
+
+		green {
+			label = "bananapi-m64:green:user";
+			gpios = <&pio 4 14 GPIO_ACTIVE_HIGH>; /* PE14 */
+		};
+
+		blue {
+			label = "bananapi-m64:blue:user";
+			gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* PE15 */
+		};
+	};
+
 	wifi_pwrseq: wifi_pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
@@ -153,6 +173,17 @@
 
 #include "axp803.dtsi"
 
+&reg_aldo1 {
+	/*
+	 * This regulator also drives the PE pingroup GPIOs,
+	 * which also controls two LEDs.
+	 */
+	regulator-always-on;
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "afvcc-csi";
+};
+
 &reg_aldo2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1800000>;
@@ -168,6 +199,11 @@
 };
 
 &reg_dc1sw {
+	/*
+	 * This regulator also indirectly drives the PD pingroup GPIOs,
+	 * which also controls the power LED.
+	 */
+	regulator-always-on;
 	regulator-name = "vcc-phy";
 };
 
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node
  2017-12-01 16:27 [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node Chen-Yu Tsai
@ 2017-12-05  6:15 ` Jagan Teki
  2017-12-05  6:19   ` Chen-Yu Tsai
  2017-12-05 10:05 ` Maxime Ripard
  1 sibling, 1 reply; 6+ messages in thread
From: Jagan Teki @ 2017-12-05  6:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 1, 2017 at 9:57 PM, Chen-Yu Tsai <wens@csie.org> wrote:
> The Bananapi-M64 has 3 LEDS in red, green, and blue. These are toggled
> via GPIO lines, which drive transistors that control current across the
> LEDS. The red LED is by default on, via an additional pull-up on the
> control line. We consider this means that it is a power indicator.
> So we set the "default-on" property for it.
>
> The pingroups the GPIO lines belong to require external regulators be
> enabled to be able to drive the GPIO high. These regulators also have
> other purposes. However the pin controller does not have bindings for
> regulators. Here we just set them to always-on.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> index 4a8d3f83a36e..a6975670cd1c 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> @@ -60,6 +60,26 @@
>                 stdout-path = "serial0:115200n8";
>         };
>
> +       leds {
> +               compatible = "gpio-leds";
> +
> +               pwr-led {
> +                       label = "bananapi-m64:red:pwr";
> +                       gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* PD24 */
> +                       default-state = "on";
> +               };
> +
> +               green {
> +                       label = "bananapi-m64:green:user";
> +                       gpios = <&pio 4 14 GPIO_ACTIVE_HIGH>; /* PE14 */
> +               };
> +
> +               blue {
> +                       label = "bananapi-m64:blue:user";
> +                       gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* PE15 */
> +               };
> +       };
> +
>         wifi_pwrseq: wifi_pwrseq {
>                 compatible = "mmc-pwrseq-simple";
>                 reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
> @@ -153,6 +173,17 @@
>
>  #include "axp803.dtsi"
>
> +&reg_aldo1 {
> +       /*
> +        * This regulator also drives the PE pingroup GPIOs,
> +        * which also controls two LEDs.
> +        */
> +       regulator-always-on;
> +       regulator-min-microvolt = <2800000>;
> +       regulator-max-microvolt = <2800000>;

why 2.8v?

> +       regulator-name = "afvcc-csi";
> +};
> +
>  &reg_aldo2 {
>         regulator-always-on;
>         regulator-min-microvolt = <1800000>;
> @@ -168,6 +199,11 @@
>  };
>
>  &reg_dc1sw {
> +       /*
> +        * This regulator also indirectly drives the PD pingroup GPIOs,
> +        * which also controls the power LED.
> +        */
> +       regulator-always-on;

This make rgmii phy always on, does this make emac power-drop?



-- 
Jagan Teki
Senior Linux Kernel Engineer | Amarula Solutions
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node
  2017-12-05  6:15 ` Jagan Teki
@ 2017-12-05  6:19   ` Chen-Yu Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2017-12-05  6:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 5, 2017 at 2:15 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> On Fri, Dec 1, 2017 at 9:57 PM, Chen-Yu Tsai <wens@csie.org> wrote:
>> The Bananapi-M64 has 3 LEDS in red, green, and blue. These are toggled
>> via GPIO lines, which drive transistors that control current across the
>> LEDS. The red LED is by default on, via an additional pull-up on the
>> control line. We consider this means that it is a power indicator.
>> So we set the "default-on" property for it.
>>
>> The pingroups the GPIO lines belong to require external regulators be
>> enabled to be able to drive the GPIO high. These regulators also have
>> other purposes. However the pin controller does not have bindings for
>> regulators. Here we just set them to always-on.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts | 36 ++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
>> index 4a8d3f83a36e..a6975670cd1c 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
>> @@ -60,6 +60,26 @@
>>                 stdout-path = "serial0:115200n8";
>>         };
>>
>> +       leds {
>> +               compatible = "gpio-leds";
>> +
>> +               pwr-led {
>> +                       label = "bananapi-m64:red:pwr";
>> +                       gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* PD24 */
>> +                       default-state = "on";
>> +               };
>> +
>> +               green {
>> +                       label = "bananapi-m64:green:user";
>> +                       gpios = <&pio 4 14 GPIO_ACTIVE_HIGH>; /* PE14 */
>> +               };
>> +
>> +               blue {
>> +                       label = "bananapi-m64:blue:user";
>> +                       gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* PE15 */
>> +               };
>> +       };
>> +
>>         wifi_pwrseq: wifi_pwrseq {
>>                 compatible = "mmc-pwrseq-simple";
>>                 reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
>> @@ -153,6 +173,17 @@
>>
>>  #include "axp803.dtsi"
>>
>> +&reg_aldo1 {
>> +       /*
>> +        * This regulator also drives the PE pingroup GPIOs,
>> +        * which also controls two LEDs.
>> +        */
>> +       regulator-always-on;
>> +       regulator-min-microvolt = <2800000>;
>> +       regulator-max-microvolt = <2800000>;
>
> why 2.8v?

That is what is written on the schematics. Plus it seems camera
modules are meant to use 2.8v.

>
>> +       regulator-name = "afvcc-csi";
>> +};
>> +
>>  &reg_aldo2 {
>>         regulator-always-on;
>>         regulator-min-microvolt = <1800000>;
>> @@ -168,6 +199,11 @@
>>  };
>>
>>  &reg_dc1sw {
>> +       /*
>> +        * This regulator also indirectly drives the PD pingroup GPIOs,
>> +        * which also controls the power LED.
>> +        */
>> +       regulator-always-on;
>
> This make rgmii phy always on, does this make emac power-drop?

emac power-drop? Please elaborate.

ChenYu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node
  2017-12-01 16:27 [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node Chen-Yu Tsai
  2017-12-05  6:15 ` Jagan Teki
@ 2017-12-05 10:05 ` Maxime Ripard
  2017-12-05 13:00   ` Chen-Yu Tsai
  1 sibling, 1 reply; 6+ messages in thread
From: Maxime Ripard @ 2017-12-05 10:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Sat, Dec 02, 2017 at 12:27:33AM +0800, Chen-Yu Tsai wrote:
> The Bananapi-M64 has 3 LEDS in red, green, and blue. These are toggled
> via GPIO lines, which drive transistors that control current across the
> LEDS. The red LED is by default on, via an additional pull-up on the
> control line. We consider this means that it is a power indicator.
> So we set the "default-on" property for it.
> 
> The pingroups the GPIO lines belong to require external regulators be
> enabled to be able to drive the GPIO high. These regulators also have
> other purposes. However the pin controller does not have bindings for
> regulators. Here we just set them to always-on.

I guess we should take the opportunity to do just that.

We have been deferring this for quite some time now, this is a perfect
occasion to do it once and for all :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171205/67860803/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node
  2017-12-05 10:05 ` Maxime Ripard
@ 2017-12-05 13:00   ` Chen-Yu Tsai
  2017-12-08  9:38     ` Maxime Ripard
  0 siblings, 1 reply; 6+ messages in thread
From: Chen-Yu Tsai @ 2017-12-05 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 5, 2017 at 6:05 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Sat, Dec 02, 2017 at 12:27:33AM +0800, Chen-Yu Tsai wrote:
>> The Bananapi-M64 has 3 LEDS in red, green, and blue. These are toggled
>> via GPIO lines, which drive transistors that control current across the
>> LEDS. The red LED is by default on, via an additional pull-up on the
>> control line. We consider this means that it is a power indicator.
>> So we set the "default-on" property for it.
>>
>> The pingroups the GPIO lines belong to require external regulators be
>> enabled to be able to drive the GPIO high. These regulators also have
>> other purposes. However the pin controller does not have bindings for
>> regulators. Here we just set them to always-on.
>
> I guess we should take the opportunity to do just that.
>
> We have been deferring this for quite some time now, this is a perfect
> occasion to do it once and for all :)

True. But right now I don't see an elegant solution. What is the purpose
of having the regulator tied to pinctrl / GPIO? You can't have the pinctrl
driver asking for the regulator at probe time, as it might not be available
or even have a cyclic dependency (because the PMIC is I2C driven, which
needs an I2C controller, which needs the pinctrl). If you defer fetching
the regulator to a later point, you might end up with an inconsistent state.
Again with the I2C driven regulator example, what happens if the regulator
core decides to turn the regulator off before the pinctrl side has a chance
to take a reference and enable it?


Regards
ChenYu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node
  2017-12-05 13:00   ` Chen-Yu Tsai
@ 2017-12-08  9:38     ` Maxime Ripard
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2017-12-08  9:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 05, 2017 at 09:00:37PM +0800, Chen-Yu Tsai wrote:
> On Tue, Dec 5, 2017 at 6:05 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Sat, Dec 02, 2017 at 12:27:33AM +0800, Chen-Yu Tsai wrote:
> >> The Bananapi-M64 has 3 LEDS in red, green, and blue. These are toggled
> >> via GPIO lines, which drive transistors that control current across the
> >> LEDS. The red LED is by default on, via an additional pull-up on the
> >> control line. We consider this means that it is a power indicator.
> >> So we set the "default-on" property for it.
> >>
> >> The pingroups the GPIO lines belong to require external regulators be
> >> enabled to be able to drive the GPIO high. These regulators also have
> >> other purposes. However the pin controller does not have bindings for
> >> regulators. Here we just set them to always-on.
> >
> > I guess we should take the opportunity to do just that.
> >
> > We have been deferring this for quite some time now, this is a perfect
> > occasion to do it once and for all :)
> 
> True. But right now I don't see an elegant solution. What is the purpose
> of having the regulator tied to pinctrl / GPIO? You can't have the pinctrl
> driver asking for the regulator at probe time, as it might not be available
> or even have a cyclic dependency (because the PMIC is I2C driven, which
> needs an I2C controller, which needs the pinctrl).

Ah, right, you definitely have a circular dependency.

> If you defer fetching the regulator to a later point, you might end
> up with an inconsistent state.  Again with the I2C driven regulator
> example, what happens if the regulator core decides to turn the
> regulator off before the pinctrl side has a chance to take a
> reference and enable it?

I guess you could implement something based on the an asynchronous
regulator_get. You call regulator_get_async (or whatever) in the
pinctrl probe, and get notified as soon as it is registered so that
you can act upon it.

Anyway, this is a bit too much to ask to enable 3 LEDs... I've applied
that patch.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171208/9c1af039/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-08  9:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01 16:27 [PATCH] arm64: dts: allwinner: a64: bananapi-m64: Add LED device node Chen-Yu Tsai
2017-12-05  6:15 ` Jagan Teki
2017-12-05  6:19   ` Chen-Yu Tsai
2017-12-05 10:05 ` Maxime Ripard
2017-12-05 13:00   ` Chen-Yu Tsai
2017-12-08  9:38     ` Maxime Ripard

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).