diff mbox

[v2] ARM: dts: sunxi: Add regulators for LeMaker BananaPi

Message ID 1438532305-5884-1-git-send-email-public_timo.s@silentcreek.de (mailing list archive)
State New, archived
Headers show

Commit Message

Timo Sigurdsson Aug. 2, 2015, 4:18 p.m. UTC
sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
driver, so add them to allow for voltage-scaling with cpufreq-dt.

Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
---
Changes since v1 (RFC):

- Dropped the changes to the cpufreq operating points and renamed the patch
accordingly
- Limited the CPU voltage range so it doesn't exceed the SOC specifications
---
 arch/arm/boot/dts/sun7i-a20-bananapi.dts | 35 ++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

Comments

Maxime Ripard Aug. 3, 2015, 9:47 a.m. UTC | #1
On Sun, Aug 02, 2015 at 06:18:25PM +0200, Timo Sigurdsson wrote:
> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
> driver, so add them to allow for voltage-scaling with cpufreq-dt.
> 
> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
> ---
> Changes since v1 (RFC):
> 
> - Dropped the changes to the cpufreq operating points and renamed the patch
> accordingly
> - Limited the CPU voltage range so it doesn't exceed the SOC specifications
> ---
>  arch/arm/boot/dts/sun7i-a20-bananapi.dts | 35 ++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> index 9f7b472..74382f3 100644
> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> @@ -92,6 +92,10 @@
>  	status = "okay";
>  };
>  
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &ehci0 {
>  	status = "okay";
>  };
> @@ -119,13 +123,9 @@
>  	status = "okay";
>  
>  	axp209: pmic@34 {
> -		compatible = "x-powers,axp209";
>  		reg = <0x34>;
>  		interrupt-parent = <&nmi_intc>;
>  		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
>  	};
>  };
>  
> @@ -182,6 +182,33 @@
>  	};
>  };
>  
> +#include "axp209.dtsi"
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-int-dll";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};

What regulator provides the 3.3V regulator used in the rest of this DT
then (MMC, GMAC) ?

Maxime
Hans de Goede Aug. 3, 2015, 1:11 p.m. UTC | #2
Hi,

On 03-08-15 11:47, Maxime Ripard wrote:
> On Sun, Aug 02, 2015 at 06:18:25PM +0200, Timo Sigurdsson wrote:
>> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
>> driver, so add them to allow for voltage-scaling with cpufreq-dt.
>>
>> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
>> ---
>> Changes since v1 (RFC):
>>
>> - Dropped the changes to the cpufreq operating points and renamed the patch
>> accordingly
>> - Limited the CPU voltage range so it doesn't exceed the SOC specifications
>> ---
>>   arch/arm/boot/dts/sun7i-a20-bananapi.dts | 35 ++++++++++++++++++++++++++++----
>>   1 file changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> index 9f7b472..74382f3 100644
>> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> @@ -92,6 +92,10 @@
>>   	status = "okay";
>>   };
>>
>> +&cpu0 {
>> +	cpu-supply = <&reg_dcdc2>;
>> +};
>> +
>>   &ehci0 {
>>   	status = "okay";
>>   };
>> @@ -119,13 +123,9 @@
>>   	status = "okay";
>>
>>   	axp209: pmic@34 {
>> -		compatible = "x-powers,axp209";
>>   		reg = <0x34>;
>>   		interrupt-parent = <&nmi_intc>;
>>   		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
>> -
>> -		interrupt-controller;
>> -		#interrupt-cells = <1>;
>>   	};
>>   };
>>
>> @@ -182,6 +182,33 @@
>>   	};
>>   };
>>
>> +#include "axp209.dtsi"
>> +
>> +&reg_dcdc2 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <1000000>;
>> +	regulator-max-microvolt = <1400000>;
>> +	regulator-name = "vdd-cpu";
>> +};
>> +
>> +&reg_dcdc3 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <1000000>;
>> +	regulator-max-microvolt = <1400000>;
>> +	regulator-name = "vdd-int-dll";
>> +};
>> +
>> +&reg_ldo1 {
>> +	regulator-name = "vdd-rtc";
>> +};
>> +
>> +&reg_ldo2 {
>> +	regulator-always-on;
>> +	regulator-min-microvolt = <3000000>;
>> +	regulator-max-microvolt = <3000000>;
>> +	regulator-name = "avcc";
>> +};
>
> What regulator provides the 3.3V regulator used in the rest of this DT
> then (MMC, GMAC) ?

A separate fixed regulator, like most (all?) other axp209 using boards,
e.g. the cubieboard has a TCS4199 regulator for this, and the banana
boards use a XL8206/UP1746 for this, according to the schematics I
have.

Regards,

Hans

>
> Maxime
>
Timo Sigurdsson Aug. 4, 2015, 9:02 a.m. UTC | #3
Hi Maxime,

Maxime Ripard schrieb am 03.08.2015 11:47:
> What regulator provides the 3.3V regulator used in the rest of this DT
> then (MMC, GMAC) ?

For GMAC, there is a reg_gmac_3v3 defined in sun7i-a20-bananapi.dts [1].
MMC uses reg_vcc3v3 included from sunxi-common-regulators.dtsi. Am I
missing something? Is this not sufficient?


Thanks,

Timo

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun7i-a20-bananapi.dts?id=v4.2-rc5#n78
Hans de Goede Aug. 8, 2015, 3:35 p.m. UTC | #4
Hi,

On 02-08-15 18:18, Timo Sigurdsson wrote:
> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
> driver, so add them to allow for voltage-scaling with cpufreq-dt.
>
> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>

Thanks for doing this, looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


p.s.

I've replaced my own patch with this one in my sunxi-wip branch.


> ---
> Changes since v1 (RFC):
>
> - Dropped the changes to the cpufreq operating points and renamed the patch
> accordingly
> - Limited the CPU voltage range so it doesn't exceed the SOC specifications
> ---
>   arch/arm/boot/dts/sun7i-a20-bananapi.dts | 35 ++++++++++++++++++++++++++++----
>   1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> index 9f7b472..74382f3 100644
> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> @@ -92,6 +92,10 @@
>   	status = "okay";
>   };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>   &ehci0 {
>   	status = "okay";
>   };
> @@ -119,13 +123,9 @@
>   	status = "okay";
>
>   	axp209: pmic@34 {
> -		compatible = "x-powers,axp209";
>   		reg = <0x34>;
>   		interrupt-parent = <&nmi_intc>;
>   		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
>   	};
>   };
>
> @@ -182,6 +182,33 @@
>   	};
>   };
>
> +#include "axp209.dtsi"
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-int-dll";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};
> +
>   &reg_usb1_vbus {
>   	status = "okay";
>   };
>
Maxime Ripard Aug. 18, 2015, 3:32 p.m. UTC | #5
On Tue, Aug 04, 2015 at 11:02:24AM +0200, Timo Sigurdsson wrote:
> Hi Maxime,
> 
> Maxime Ripard schrieb am 03.08.2015 11:47:
> > What regulator provides the 3.3V regulator used in the rest of this DT
> > then (MMC, GMAC) ?
> 
> For GMAC, there is a reg_gmac_3v3 defined in sun7i-a20-bananapi.dts [1].
> MMC uses reg_vcc3v3 included from sunxi-common-regulators.dtsi. Am I
> missing something? Is this not sufficient?

Well, this 3.3v regulator doesn't appear out of nowhere. Most likely,
it's either one of the AXP output, or a regulator directly taking the
5v output of the DC plug.

Maxime
Maxime Ripard Aug. 18, 2015, 3:33 p.m. UTC | #6
On Mon, Aug 03, 2015 at 03:11:41PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 03-08-15 11:47, Maxime Ripard wrote:
> >On Sun, Aug 02, 2015 at 06:18:25PM +0200, Timo Sigurdsson wrote:
> >>sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
> >>driver, so add them to allow for voltage-scaling with cpufreq-dt.
> >>
> >>Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
> >>---
> >>Changes since v1 (RFC):
> >>
> >>- Dropped the changes to the cpufreq operating points and renamed the patch
> >>accordingly
> >>- Limited the CPU voltage range so it doesn't exceed the SOC specifications
> >>---
> >>  arch/arm/boot/dts/sun7i-a20-bananapi.dts | 35 ++++++++++++++++++++++++++++----
> >>  1 file changed, 31 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> >>index 9f7b472..74382f3 100644
> >>--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> >>+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> >>@@ -92,6 +92,10 @@
> >>  	status = "okay";
> >>  };
> >>
> >>+&cpu0 {
> >>+	cpu-supply = <&reg_dcdc2>;
> >>+};
> >>+
> >>  &ehci0 {
> >>  	status = "okay";
> >>  };
> >>@@ -119,13 +123,9 @@
> >>  	status = "okay";
> >>
> >>  	axp209: pmic@34 {
> >>-		compatible = "x-powers,axp209";
> >>  		reg = <0x34>;
> >>  		interrupt-parent = <&nmi_intc>;
> >>  		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> >>-
> >>-		interrupt-controller;
> >>-		#interrupt-cells = <1>;
> >>  	};
> >>  };
> >>
> >>@@ -182,6 +182,33 @@
> >>  	};
> >>  };
> >>
> >>+#include "axp209.dtsi"
> >>+
> >>+&reg_dcdc2 {
> >>+	regulator-always-on;
> >>+	regulator-min-microvolt = <1000000>;
> >>+	regulator-max-microvolt = <1400000>;
> >>+	regulator-name = "vdd-cpu";
> >>+};
> >>+
> >>+&reg_dcdc3 {
> >>+	regulator-always-on;
> >>+	regulator-min-microvolt = <1000000>;
> >>+	regulator-max-microvolt = <1400000>;
> >>+	regulator-name = "vdd-int-dll";
> >>+};
> >>+
> >>+&reg_ldo1 {
> >>+	regulator-name = "vdd-rtc";
> >>+};
> >>+
> >>+&reg_ldo2 {
> >>+	regulator-always-on;
> >>+	regulator-min-microvolt = <3000000>;
> >>+	regulator-max-microvolt = <3000000>;
> >>+	regulator-name = "avcc";
> >>+};
> >
> >What regulator provides the 3.3V regulator used in the rest of this DT
> >then (MMC, GMAC) ?
> 
> A separate fixed regulator, like most (all?) other axp209 using boards,
> e.g. the cubieboard has a TCS4199 regulator for this, and the banana
> boards use a XL8206/UP1746 for this, according to the schematics I
> have.

Ack.

Thanks!
Maxime
Maxime Ripard Aug. 18, 2015, 3:36 p.m. UTC | #7
On Sun, Aug 02, 2015 at 06:18:25PM +0200, Timo Sigurdsson wrote:
> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
> driver, so add them to allow for voltage-scaling with cpufreq-dt.
> 
> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>

Queued, thanks!
Maxime
Kevin Hilman Sept. 24, 2015, 5:57 p.m. UTC | #8
On Tue, Aug 18, 2015 at 8:36 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Sun, Aug 02, 2015 at 06:18:25PM +0200, Timo Sigurdsson wrote:
>> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
>> driver, so add them to allow for voltage-scaling with cpufreq-dt.
>>
>> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
>
> Queued, thanks!
> Maxime

kernelci.org started finding boot faiulres[1] on bananapi linux-next
around next-20150918, but it was only failing in some labs and not
others.  I finally bisected it down to this patch, which landed in
linux-next in the form of 2d665a8a8350 ARM: dts: sunxi: Add regulators
for LeMaker BananaPi.  Reverting that commit on top of next-20150923
gets my bananapi booting again.

Note it's kind of an interesting boot failure.  The kernel boots fully
to a shell, but panics after running a few commands.  In particular
'dmesg -n1' seems to trigger it usually[2].

Kevin

[1] http://kernelci.org/boot/sun7i-a20-bananapi/job/next/kernel/next-20150923/defconfig/multi_v7_defconfig/lab/lab-khilman/?_id=5602504359b514be146c326f
[2] http://storage.kernelci.org/next/next-20150923/arm-multi_v7_defconfig/lab-khilman/boot-sun7i-a20-bananapi.html
Timo Sigurdsson Sept. 25, 2015, 3:05 p.m. UTC | #9
Hi Kevin,

Kevin Hilman schrieb am 25. Sept 2015 01:57:

> On Tue, Aug 18, 2015 at 8:36 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> On Sun, Aug 02, 2015 at 06:18:25PM +0200, Timo Sigurdsson wrote:
>>> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209
>>> PMU
>>> driver, so add them to allow for voltage-scaling with cpufreq-dt.
>>>
>>> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
>>
>> Queued, thanks!
>> Maxime
> 
> kernelci.org started finding boot faiulres[1] on bananapi linux-next
> around next-20150918, but it was only failing in some labs and not
> others.  I finally bisected it down to this patch, which landed in
> linux-next in the form of 2d665a8a8350 ARM: dts: sunxi: Add regulators
> for LeMaker BananaPi.  Reverting that commit on top of next-20150923
> gets my bananapi booting again.
> 
> Note it's kind of an interesting boot failure.  The kernel boots fully
> to a shell, but panics after running a few commands.  In particular
> 'dmesg -n1' seems to trigger it usually[2].
> 
> Kevin
> 
> [1]
> http://kernelci.org/boot/sun7i-a20-bananapi/job/next/kernel/next-20150923/defconfig/multi_v7_defconfig/lab/lab-khilman/?_id=5602504359b514be146c326f
> [2]
> http://storage.kernelci.org/next/next-20150923/arm-multi_v7_defconfig/lab-khilman/boot-sun7i-a20-bananapi.html
> 

Thanks for your feedback. I'm traveling at the moment, so I can't do any testing but just guess wildly. I know, though, that I used dmesg frequently when I did my own testing before submitting the patch and could not see such behavior.

Before this commit, the CPU of your BananaPi runs at 1.4 volts constantly. With this commit applied, the CPU voltage should vary between 1.0-1.4 volts depending on the frequency and defined operating points. Hence, one of my guesses would be that your CPU is not stable at the lower voltages. Could you modify the voltages for the defined frequencies in sun7i-a20.dtsi and test if that solves your issue? Say, raise the voltage by 0.1 volts for each operating point (but no higher than 1.4). I actually had a different patch that applied slightly higher voltages taken from the original fex for by LeMaker, but the feedback was, unless there are actual reports about boards not running stable at the current settings, we just keep them instead. So, I'm curious if you happen to have a board that requires slightly higher voltages to run stable.

Regards, 

Timo
Maxime Ripard Sept. 27, 2015, 8:20 a.m. UTC | #10
On Fri, Sep 25, 2015 at 05:05:58PM +0200, Timo Sigurdsson wrote:
> Hi Kevin,
> 
> Kevin Hilman schrieb am 25. Sept 2015 01:57:
> 
> > On Tue, Aug 18, 2015 at 8:36 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> >> On Sun, Aug 02, 2015 at 06:18:25PM +0200, Timo Sigurdsson wrote:
> >>> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209
> >>> PMU
> >>> driver, so add them to allow for voltage-scaling with cpufreq-dt.
> >>>
> >>> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
> >>
> >> Queued, thanks!
> >> Maxime
> > 
> > kernelci.org started finding boot faiulres[1] on bananapi linux-next
> > around next-20150918, but it was only failing in some labs and not
> > others.  I finally bisected it down to this patch, which landed in
> > linux-next in the form of 2d665a8a8350 ARM: dts: sunxi: Add regulators
> > for LeMaker BananaPi.  Reverting that commit on top of next-20150923
> > gets my bananapi booting again.
> > 
> > Note it's kind of an interesting boot failure.  The kernel boots fully
> > to a shell, but panics after running a few commands.  In particular
> > 'dmesg -n1' seems to trigger it usually[2].
> > 
> > Kevin
> > 
> > [1]
> > http://kernelci.org/boot/sun7i-a20-bananapi/job/next/kernel/next-20150923/defconfig/multi_v7_defconfig/lab/lab-khilman/?_id=5602504359b514be146c326f
> > [2]
> > http://storage.kernelci.org/next/next-20150923/arm-multi_v7_defconfig/lab-khilman/boot-sun7i-a20-bananapi.html
> > 
> 
> Thanks for your feedback. I'm traveling at the moment, so I can't do
> any testing but just guess wildly. I know, though, that I used dmesg
> frequently when I did my own testing before submitting the patch and
> could not see such behavior.
> 
> Before this commit, the CPU of your BananaPi runs at 1.4 volts
> constantly. With this commit applied, the CPU voltage should vary
> between 1.0-1.4 volts depending on the frequency and defined
> operating points. Hence, one of my guesses would be that your CPU is
> not stable at the lower voltages. Could you modify the voltages for
> the defined frequencies in sun7i-a20.dtsi and test if that solves
> your issue? Say, raise the voltage by 0.1 volts for each operating
> point (but no higher than 1.4). I actually had a different patch
> that applied slightly higher voltages taken from the original fex
> for by LeMaker, but the feedback was, unless there are actual
> reports about boards not running stable at the current settings, we
> just keep them instead. So, I'm curious if you happen to have a
> board that requires slightly higher voltages to run stable.

I've dropped the patch waiting for you to come back from your holidays
when we will have more time to figure out what's wrong.

Thanks!
Maxime
diff mbox

Patch

diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
index 9f7b472..74382f3 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -92,6 +92,10 @@ 
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -119,13 +123,9 @@ 
 	status = "okay";
 
 	axp209: pmic@34 {
-		compatible = "x-powers,axp209";
 		reg = <0x34>;
 		interrupt-parent = <&nmi_intc>;
 		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-
-		interrupt-controller;
-		#interrupt-cells = <1>;
 	};
 };
 
@@ -182,6 +182,33 @@ 
 	};
 };
 
+#include "axp209.dtsi"
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-int-dll";
+};
+
+&reg_ldo1 {
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
 &reg_usb1_vbus {
 	status = "okay";
 };