[PATCH 3/5] dt-bindings: leds: Add binding for axp20x-led device driver

[PATCH 3/5] dt-bindings: leds: Add binding for axp20x-led device driver

[Stefan Mavrodiev]

This adds the devicetree bindings for charge led indicator found
on most of X-Powers AXP20X PMICs.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
new file mode 100644
index 000000000000..73fae22ffe29
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
@@ -0,0 +1,74 @@
+Device Tree Bindings for LED support on X-Powers PMIC
+
+Most of the X-Powers PMICs have integrated battery charger with LED indicator.
+The output is open-drain, so the state is either high-Z or output-low. The
+driver is subnode of AXP20X MFD driver, since it uses shared bus with all
+other cells.
+The LED can be controlled either manually or automatic. Then in automatic
+(controlled by the charger) there are two indication modes:
+
+Mode-A
+======
+- output-low:		Charging
+- high-Z		Not charging
+- 1Hz flashing:		Abnormal alarm
+- 4Hz flashing		Overvoltage alarm
+
+Mode-B
+======
+- output-low:		Battery full
+- high-Z		Not charging
+- 1Hz flashing:		Charging
+- 4Hz flashing		Overvoltage or abnormal alarm
+
+The control and the mode can be changed from sysfs.
+
+For AXP20X MFD bindings see:
+Documentation/devicetree/bindings/mfd/axp20x.txt
+
+Required properties:
+- compatible : Must be "x-powers,axp20x-led"
+
+Supported common leds properties, see ./common.txt for more information:
+- label : sets LED label. If omitted, dt device node is used
+- linux,default-trigger : See
+- default-state:
+
+Optional properties:
+- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
+			 If omitted, then the control is set to manual mode.
+			 On invalid value, Mode-A is used.
+
+
+Example:
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		axp20x-led {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			linux,default-trigger = "timer";
+			default-state = "on";
+		};
+	};
+
+or
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		axp20x-led {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			x-powers,charger-mode = "mode-b";
+		};
+	};
-- 
2.17.1

Re: [PATCH 3/5] dt-bindings: leds: Add binding for axp20x-led device driver

[Jacek Anaszewski]

Hi Stefan,

Thank you for the patch.

I have few nits below.

On 1/31/19 9:23 AM, Stefan Mavrodiev wrote:
> This adds the devicetree bindings for charge led indicator found
> on most of X-Powers AXP20X PMICs.
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>   .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
>   1 file changed, 74 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> new file mode 100644
> index 000000000000..73fae22ffe29
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> @@ -0,0 +1,74 @@
> +Device Tree Bindings for LED support on X-Powers PMIC
> +
> +Most of the X-Powers PMICs have integrated battery charger with LED indicator.
> +The output is open-drain, so the state is either high-Z or output-low. The
> +driver is subnode of AXP20X MFD driver, since it uses shared bus with all

s/is subnode/is a subnode/

> +other cells.
> +The LED can be controlled either manually or automatic. Then in automatic

s/or automatic/or automatically/

> +(controlled by the charger) there are two indication modes:
> +
> +Mode-A
> +======
> +- output-low:		Charging
> +- high-Z		Not charging
> +- 1Hz flashing:		Abnormal alarm
> +- 4Hz flashing		Overvoltage alarm
> +
> +Mode-B
> +======
> +- output-low:		Battery full
> +- high-Z		Not charging
> +- 1Hz flashing:		Charging
> +- 4Hz flashing		Overvoltage or abnormal alarm
> +
> +The control and the mode can be changed from sysfs.
> +
> +For AXP20X MFD bindings see:
> +Documentation/devicetree/bindings/mfd/axp20x.txt
> +
> +Required properties:
> +- compatible : Must be "x-powers,axp20x-led"
> +
> +Supported common leds properties, see ./common.txt for more information:

s/leds/LED/

Please remove ", see ./common.txt for more information" in favor
of references indicated below.

> +- label : sets LED label. If omitted, dt device node is used

Please change it to:
- label : See Documentation/devicetree/bindings/leds/common.txt

> +- linux,default-trigger : See
> +- default-state:

- linux,default-trigger : See 
Documentation/devicetree/bindings/leds/common.txt

> +
> +Optional properties:
> +- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
> +			 If omitted, then the control is set to manual mode.
> +			 On invalid value, Mode-A is used.
> +
> +
> +Example:
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		axp20x-led {

s/axp20x-led/led@0/

> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			linux,default-trigger = "timer";
> +			default-state = "on";
> +		};
> +	};
> +
> +or
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		axp20x-led {

s/axp20x-led/led@0/

> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			x-powers,charger-mode = "mode-b";
> +		};
> +	};
> 

-- 
Best regards,
Jacek Anaszewski

[PATCH 4/5] arm64: dts: allwinner: axp803: add charge led node

[Stefan Mavrodiev]

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index c3a618e1279a..af4898602b34 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -76,6 +76,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp803-battery-power-supply",
 			     "x-powers,axp813-battery-power-supply";
-- 
2.17.1

[PATCH 5/5] arm: dts: axpxx: add charge led node

[Stefan Mavrodiev]

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/axp209.dtsi | 5 +++++
 arch/arm/boot/dts/axp22x.dtsi | 5 +++++
 arch/arm/boot/dts/axp81x.dtsi | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 0d9ff12bdf28..f972b6f3ecd0 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -69,6 +69,11 @@
 		#gpio-cells = <2>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp209-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
index 65a07a67aca9..92a0b64252b1 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -62,6 +62,11 @@
 		#io-channel-cells = <1>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp221-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index bd83962d3627..22e243cc40d5 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -74,6 +74,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp813-battery-power-supply";
 		status = "disabled";
-- 
2.17.1

Re: [PATCH 5/5] arm: dts: axpxx: add charge led node

[Chen-Yu Tsai]

On Thu, Jan 31, 2019 at 4:25 PM Stefan Mavrodiev <stefan@olimex.com> wrote:
>
> Add dt node for axp20x-led driver controlling CHGLED.
> Default status is disabled, since it may be not used.
>
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>

Please include a cover letter for such a patch series.

Also, do any boards actually use this? I know the Pine64 does, but the
LED is left to the user to populate. I would really like to have an in
kernel user of this function, so that we can verify it.

ChenYu

> ---
>  arch/arm/boot/dts/axp209.dtsi | 5 +++++
>  arch/arm/boot/dts/axp22x.dtsi | 5 +++++
>  arch/arm/boot/dts/axp81x.dtsi | 5 +++++
>  3 files changed, 15 insertions(+)
>
> diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
> index 0d9ff12bdf28..f972b6f3ecd0 100644
> --- a/arch/arm/boot/dts/axp209.dtsi
> +++ b/arch/arm/boot/dts/axp209.dtsi
> @@ -69,6 +69,11 @@
>                 #gpio-cells = <2>;
>         };
>
> +       axp_led: led {
> +               compatible = "x-powers,axp20x-led";
> +               status = "disabled";
> +       };
> +
>         battery_power_supply: battery-power-supply {
>                 compatible = "x-powers,axp209-battery-power-supply";
>                 status = "disabled";
> diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
> index 65a07a67aca9..92a0b64252b1 100644
> --- a/arch/arm/boot/dts/axp22x.dtsi
> +++ b/arch/arm/boot/dts/axp22x.dtsi
> @@ -62,6 +62,11 @@
>                 #io-channel-cells = <1>;
>         };
>
> +       axp_led: led {
> +               compatible = "x-powers,axp20x-led";
> +               status = "disabled";
> +       };
> +
>         battery_power_supply: battery-power-supply {
>                 compatible = "x-powers,axp221-battery-power-supply";
>                 status = "disabled";
> diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
> index bd83962d3627..22e243cc40d5 100644
> --- a/arch/arm/boot/dts/axp81x.dtsi
> +++ b/arch/arm/boot/dts/axp81x.dtsi
> @@ -74,6 +74,11 @@
>                 };
>         };
>
> +       axp_led: led {
> +               compatible = "x-powers,axp20x-led";
> +               status = "disabled";
> +       };
> +
>         battery_power_supply: battery-power-supply {
>                 compatible = "x-powers,axp813-battery-power-supply";
>                 status = "disabled";
> --
> 2.17.1
>

Re: [PATCH 5/5] arm: dts: axpxx: add charge led node

[Stefan Mavrodiev]

On 2/5/19 6:16 PM, Chen-Yu Tsai wrote:
> On Thu, Jan 31, 2019 at 4:25 PM Stefan Mavrodiev <stefan@olimex.com> wrote:
>> Add dt node for axp20x-led driver controlling CHGLED.
>> Default status is disabled, since it may be not used.
>>
>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> Please include a cover letter for such a patch series.
Sorry, I will.
>
> Also, do any boards actually use this? I know the Pine64 does, but the
> LED is left to the user to populate. I would really like to have an in
> kernel user of this function, so that we can verify it.

All boards made by Olimex has this led populated. The development and the
testing of the driver was done on A64-OLinuXino board.

Should I make another patch, enabling the LED for the mention board?

Best regards,
Stefan Mavrodiev

>
> ChenYu
>
>> ---
>>   arch/arm/boot/dts/axp209.dtsi | 5 +++++
>>   arch/arm/boot/dts/axp22x.dtsi | 5 +++++
>>   arch/arm/boot/dts/axp81x.dtsi | 5 +++++
>>   3 files changed, 15 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
>> index 0d9ff12bdf28..f972b6f3ecd0 100644
>> --- a/arch/arm/boot/dts/axp209.dtsi
>> +++ b/arch/arm/boot/dts/axp209.dtsi
>> @@ -69,6 +69,11 @@
>>                  #gpio-cells = <2>;
>>          };
>>
>> +       axp_led: led {
>> +               compatible = "x-powers,axp20x-led";
>> +               status = "disabled";
>> +       };
>> +
>>          battery_power_supply: battery-power-supply {
>>                  compatible = "x-powers,axp209-battery-power-supply";
>>                  status = "disabled";
>> diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
>> index 65a07a67aca9..92a0b64252b1 100644
>> --- a/arch/arm/boot/dts/axp22x.dtsi
>> +++ b/arch/arm/boot/dts/axp22x.dtsi
>> @@ -62,6 +62,11 @@
>>                  #io-channel-cells = <1>;
>>          };
>>
>> +       axp_led: led {
>> +               compatible = "x-powers,axp20x-led";
>> +               status = "disabled";
>> +       };
>> +
>>          battery_power_supply: battery-power-supply {
>>                  compatible = "x-powers,axp221-battery-power-supply";
>>                  status = "disabled";
>> diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
>> index bd83962d3627..22e243cc40d5 100644
>> --- a/arch/arm/boot/dts/axp81x.dtsi
>> +++ b/arch/arm/boot/dts/axp81x.dtsi
>> @@ -74,6 +74,11 @@
>>                  };
>>          };
>>
>> +       axp_led: led {
>> +               compatible = "x-powers,axp20x-led";
>> +               status = "disabled";
>> +       };
>> +
>>          battery_power_supply: battery-power-supply {
>>                  compatible = "x-powers,axp813-battery-power-supply";
>>                  status = "disabled";
>> --
>> 2.17.1
>>

Re: [PATCH 5/5] arm: dts: axpxx: add charge led node

[Chen-Yu Tsai]

On Wed, Feb 6, 2019 at 2:06 PM Stefan Mavrodiev <stefan@olimex.com> wrote:
>
>
> On 2/5/19 6:16 PM, Chen-Yu Tsai wrote:
> > On Thu, Jan 31, 2019 at 4:25 PM Stefan Mavrodiev <stefan@olimex.com> wrote:
> >> Add dt node for axp20x-led driver controlling CHGLED.
> >> Default status is disabled, since it may be not used.
> >>
> >> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> > Please include a cover letter for such a patch series.
> Sorry, I will.
> >
> > Also, do any boards actually use this? I know the Pine64 does, but the
> > LED is left to the user to populate. I would really like to have an in
> > kernel user of this function, so that we can verify it.
>
> All boards made by Olimex has this led populated. The development and the
> testing of the driver was done on A64-OLinuXino board.
>
> Should I make another patch, enabling the LED for the mention board?

Yes please. I'll dig out my A33-OLinuXino next week to try it out.

ChenYu

> >
> > ChenYu
> >
> >> ---
> >>   arch/arm/boot/dts/axp209.dtsi | 5 +++++
> >>   arch/arm/boot/dts/axp22x.dtsi | 5 +++++
> >>   arch/arm/boot/dts/axp81x.dtsi | 5 +++++
> >>   3 files changed, 15 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
> >> index 0d9ff12bdf28..f972b6f3ecd0 100644
> >> --- a/arch/arm/boot/dts/axp209.dtsi
> >> +++ b/arch/arm/boot/dts/axp209.dtsi
> >> @@ -69,6 +69,11 @@
> >>                  #gpio-cells = <2>;
> >>          };
> >>
> >> +       axp_led: led {
> >> +               compatible = "x-powers,axp20x-led";
> >> +               status = "disabled";
> >> +       };
> >> +
> >>          battery_power_supply: battery-power-supply {
> >>                  compatible = "x-powers,axp209-battery-power-supply";
> >>                  status = "disabled";
> >> diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
> >> index 65a07a67aca9..92a0b64252b1 100644
> >> --- a/arch/arm/boot/dts/axp22x.dtsi
> >> +++ b/arch/arm/boot/dts/axp22x.dtsi
> >> @@ -62,6 +62,11 @@
> >>                  #io-channel-cells = <1>;
> >>          };
> >>
> >> +       axp_led: led {
> >> +               compatible = "x-powers,axp20x-led";
> >> +               status = "disabled";
> >> +       };
> >> +
> >>          battery_power_supply: battery-power-supply {
> >>                  compatible = "x-powers,axp221-battery-power-supply";
> >>                  status = "disabled";
> >> diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
> >> index bd83962d3627..22e243cc40d5 100644
> >> --- a/arch/arm/boot/dts/axp81x.dtsi
> >> +++ b/arch/arm/boot/dts/axp81x.dtsi
> >> @@ -74,6 +74,11 @@
> >>                  };
> >>          };
> >>
> >> +       axp_led: led {
> >> +               compatible = "x-powers,axp20x-led";
> >> +               status = "disabled";
> >> +       };
> >> +
> >>          battery_power_supply: battery-power-supply {
> >>                  compatible = "x-powers,axp813-battery-power-supply";
> >>                  status = "disabled";
> >> --
> >> 2.17.1
> >>