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

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

[Timo Sigurdsson]

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";
+};
+
 &reg_usb1_vbus {
 	status = "okay";
 };
-- 
2.1.4

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [linux-sunxi] Re: [PATCH v2] ARM: dts: sunxi: Add regulators for LeMaker BananaPi

[Hans de Goede]

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
>

Re: [linux-sunxi] Re: [PATCH v2] ARM: dts: sunxi: Add regulators for LeMaker BananaPi

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2722 bytes --]

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, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Timo Sigurdsson]

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

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

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, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [linux-sunxi] [PATCH v2] ARM: dts: sunxi: Add regulators for LeMaker BananaPi

[Hans de Goede]

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";
>   };
>

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Kevin Hilman]

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

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

[Timo Sigurdsson]

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

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2918 bytes --]

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Timo Sigurdsson]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

Hi Kevin,

Kevin Hilman schrieb am 24.09.2015 19:57:
> 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

following up on my last email: I'm back from my vacation and I tried to
reproduce your problem, but my board doesn't seem to be affected, so I
cannot trigger it.

I still think that the lower voltages may be the cause of your problem
with that specific board, so could you please test the attached patch on
top of my patch that you first experienced the problem with? Please let 
us know whether this solves your issue or whether we need to dig deeper.

Has anybody else been able to reproduce Kevin's issue?

Kind regards,

Timo

[-- Attachment #2: 0001-Add-board-specific-opp-for-LeMaker-BananaPi.patch --]
[-- Type: text/x-diff, Size: 481 bytes --]

diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
index 74382f3..39b6b67b 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -94,6 +94,16 @@
 
 &cpu0 {
 	cpu-supply = <&reg_dcdc2>;
+	operating-points = <
+		/* kHz	  uV */
+		960000	1400000
+		912000	1400000
+		864000	1350000
+		720000	1250000
+		528000	1150000
+		312000	1100000
+		144000	1050000
+		>;
 };
 
 &ehci0 {
-- 
2.1.4

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

Hi Timo,

On Mon, Oct 05, 2015 at 07:39:57PM +0200, Timo Sigurdsson wrote:
> Hi Kevin,
> 
> Kevin Hilman schrieb am 24.09.2015 19:57:
> > 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
> 
> following up on my last email: I'm back from my vacation and I tried to
> reproduce your problem, but my board doesn't seem to be affected, so I
> cannot trigger it.

weird.

> I still think that the lower voltages may be the cause of your problem
> with that specific board, so could you please test the attached patch on
> top of my patch that you first experienced the problem with? Please let 
> us know whether this solves your issue or whether we need to dig deeper.
> 
> Has anybody else been able to reproduce Kevin's issue?

I've talked to Kevin yesterday about that issue, and he's been away
from his boards for quite some time too, so we'll have to wait a bit
to solve that issue.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Kevin Hilman]

"Timo Sigurdsson" <public_timo.s@silentcreek.de> writes:

> Kevin Hilman schrieb am 24.09.2015 19:57:
>> 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
>
> following up on my last email: I'm back from my vacation and I tried to
> reproduce your problem, but my board doesn't seem to be affected, so I
> cannot trigger it.
>
> I still think that the lower voltages may be the cause of your problem
> with that specific board, so could you please test the attached patch on
> top of my patch that you first experienced the problem with? Please let 
> us know whether this solves your issue or whether we need to dig deeper.

Thanks for the patch.  Looks like it's the OPPs.

I went back to next-20150923 and verified it still fails.  Then, I
applied your patch and saw that it boots just fine.

Kevin

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

[Timo Sigurdsson]

Hi Kevin,
Hi Maxime,

Kevin Hilman schrieb am 07.10.2015 16:36:

> "Timo Sigurdsson" <public_timo.s@silentcreek.de> writes:
>> I still think that the lower voltages may be the cause of your problem
>> with that specific board, so could you please test the attached patch on
>> top of my patch that you first experienced the problem with? Please let 
>> us know whether this solves your issue or whether we need to dig deeper.
> 
> Thanks for the patch.  Looks like it's the OPPs.
> 
> I went back to next-20150923 and verified it still fails.  Then, I
> applied your patch and saw that it boots just fine.

Good. Then we can easily fix this, I guess.

@Maxime: How should we handle this? In its current form, the patch applies
only to the BananaPi dts by overriding the inherited opp from the SoC dtsi.
In an earlier discussion, it was said that this can be done, even though it
might not be the most elegant approach. But then again, I think it
shouldn't be necessary to change the opp in the sun7i-a20.dtsi for all A20
boards since this is - to my knowledge - the first and only report that an
A20 board has stability issues at the lower voltages (although not too many
boards use voltage scaling yet). So, would you prefer to keep this as a
patch for BananaPi only, or change the dtsi for all A20 devices instead?

In case we keep it as it is, what is the correct commit to point to as
"Fixes commit ..."? I'd say it fixes the initial opp commit for A20, since
that's where these voltages were defined. But then again, if we don't
change the dtsi, should I point to my regulator patch instead?


Thanks and regards,

Timo

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2359 bytes --]

Hi Timo,

On Wed, Oct 07, 2015 at 05:49:18PM +0200, Timo Sigurdsson wrote:
> Hi Kevin,
> Hi Maxime,
> 
> Kevin Hilman schrieb am 07.10.2015 16:36:
> 
> > "Timo Sigurdsson" <public_timo.s@silentcreek.de> writes:
> >> I still think that the lower voltages may be the cause of your problem
> >> with that specific board, so could you please test the attached patch on
> >> top of my patch that you first experienced the problem with? Please let 
> >> us know whether this solves your issue or whether we need to dig deeper.
> > 
> > Thanks for the patch.  Looks like it's the OPPs.
> > 
> > I went back to next-20150923 and verified it still fails.  Then, I
> > applied your patch and saw that it boots just fine.
> 
> Good. Then we can easily fix this, I guess.
> 
> @Maxime: How should we handle this? In its current form, the patch applies
> only to the BananaPi dts by overriding the inherited opp from the SoC dtsi.
> In an earlier discussion, it was said that this can be done, even though it
> might not be the most elegant approach. But then again, I think it
> shouldn't be necessary to change the opp in the sun7i-a20.dtsi for all A20
> boards since this is - to my knowledge - the first and only report that an
> A20 board has stability issues at the lower voltages (although not too many
> boards use voltage scaling yet).

If you count only the number of boards, indeed, but if you count the
number of devices actually used in the field, we cover already a
significant portion of them.

> So, would you prefer to keep this as a patch for BananaPi only, or
> change the dtsi for all A20 devices instead?

Yeah, we probably can keep that for bananapi only at the moment, and
try to generalize that afterwards.

> In case we keep it as it is, what is the correct commit to point to as
> "Fixes commit ..."? I'd say it fixes the initial opp commit for A20, since
> that's where these voltages were defined. But then again, if we don't
> change the dtsi, should I point to my regulator patch instead?

I don't think it fixes anything at this point. We droped your commit
that was using the A20 OPPs, so in the history so far we don't have
anything to fix, just enable cpufreq again.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Timo Sigurdsson]

Hi Maxime,

Maxime Ripard schrieb am 07.10.2015 19:49:

> Hi Timo,
> 
> On Wed, Oct 07, 2015 at 05:49:18PM +0200, Timo Sigurdsson wrote:
>> Hi Kevin,
>> Hi Maxime,
>> 
>> Kevin Hilman schrieb am 07.10.2015 16:36:
>> 
>> > "Timo Sigurdsson" <public_timo.s@silentcreek.de> writes:
>> >> I still think that the lower voltages may be the cause of your problem
>> >> with that specific board, so could you please test the attached patch on
>> >> top of my patch that you first experienced the problem with? Please let 
>> >> us know whether this solves your issue or whether we need to dig deeper.
>> > 
>> > Thanks for the patch.  Looks like it's the OPPs.
>> > 
>> > I went back to next-20150923 and verified it still fails.  Then, I
>> > applied your patch and saw that it boots just fine.
>> 
>> Good. Then we can easily fix this, I guess.
>> 
>> @Maxime: How should we handle this? In its current form, the patch applies
>> only to the BananaPi dts by overriding the inherited opp from the SoC dtsi.
>> In an earlier discussion, it was said that this can be done, even though it
>> might not be the most elegant approach. But then again, I think it
>> shouldn't be necessary to change the opp in the sun7i-a20.dtsi for all A20
>> boards since this is - to my knowledge - the first and only report that an
>> A20 board has stability issues at the lower voltages (although not too many
>> boards use voltage scaling yet).
> 
> If you count only the number of boards, indeed, but if you count the
> number of devices actually used in the field, we cover already a
> significant portion of them.
> 
>> So, would you prefer to keep this as a patch for BananaPi only, or
>> change the dtsi for all A20 devices instead?
> 
> Yeah, we probably can keep that for bananapi only at the moment, and
> try to generalize that afterwards.

Ok.

> 
>> In case we keep it as it is, what is the correct commit to point to as
>> "Fixes commit ..."? I'd say it fixes the initial opp commit for A20, since
>> that's where these voltages were defined. But then again, if we don't
>> change the dtsi, should I point to my regulator patch instead?
> 
> I don't think it fixes anything at this point. We droped your commit
> that was using the A20 OPPs, so in the history so far we don't have
> anything to fix, just enable cpufreq again.

Ok. I'll send a third version of the regulator patch then with the
updated opp included.

Thanks,

Timo

Re: [linux-sunxi] Re: [PATCH v2] ARM: dts: sunxi: Add regulators for LeMaker BananaPi

[Hans de Goede]

Hi,

On 10/07/2015 05:01 PM, Stefan Monnier wrote:
>> @Maxime: How should we handle this? In its current form, the patch applies
>> only to the BananaPi dts by overriding the inherited opp from the SoC dtsi.
>> In an earlier discussion, it was said that this can be done, even though it
>> might not be the most elegant approach. But then again, I think it
>> shouldn't be necessary to change the opp in the sun7i-a20.dtsi for all A20
>> boards since this is - to my knowledge - the first and only report that an
>> A20 board has stability issues at the lower voltages (although not too many
>> boards use voltage scaling yet).
>
> IIUC this setting doesn't affect signals external to the A20, so it
> seems like it should be board-agnostic.

This setting configures the voltage at the pmic output pin, not at the SoC
power-input pins. And how much of the voltage actually makes it from the pmic
to the SoC definitely is board-specific. So we are going to need board-specific
opp-s in some cases (like this one for example).

Regards,

Hans