[PATCH] dts: sun8i-h3: add sy8106a to orange pi plus

[PATCH] dts: sun8i-h3: add sy8106a to orange pi plus

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

The Orange Pi Plus board lacks voltage scaling capabilities in its
current form. This results in random freezes during boot when cpufreq is
enabled, probably due to wrong voltages.

This patch (a copy/paste of 06139c) does the following things on this
board:
- enable r_i2c
- add sy8106a to the r_i2c bus
- have the sy8106a regulate VDD of cpu

I have tested this on two Orange Pi Plus boards, and they run stable
across a range of voltages (and frequencies).

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts | 29 ++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
index b403e5d78..fd64b5c94 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
@@ -74,6 +74,10 @@
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_vdd_cpux>;
+};
+
 &ehci3 {
 	status = "okay";
 };
@@ -119,6 +123,31 @@
 	};
 };
 
+&r_i2c {
+	status = "okay";
+
+	reg_vdd_cpux: regulator@65 {
+		compatible = "silergy,sy8106a";
+		reg = <0x65>;
+		regulator-name = "vdd-cpux";
+		silergy,fixed-microvolt = <1200000>;
+		/*
+		 * The datasheet uses 1.1V as the minimum value of VDD-CPUX,
+		 * however both the Armbian DVFS table and the official one
+		 * have operating points with voltage under 1.1V, and both
+		 * DVFS table are known to work properly at the lowest
+		 * operating point.
+		 *
+		 * Use 1.0V as the minimum voltage instead.
+		 */
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1400000>;
+		regulator-ramp-delay = <200>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+};
+
 &usbphy {
 	usb3_vbus-supply = <&reg_usb3_vbus>;
 };
-- 
2.17.1

Re: [PATCH] dts: sun8i-h3: add sy8106a to orange pi plus

[Maxime Ripard]

Hi!

On Thu, Sep 27, 2018 at 10:36:36AM +0200, jorik@kippendief.biz wrote:
> From: Jorik Jonker <jorik@kippendief.biz>

The prefix of your commit title should be "ARM: dts: ..."

> The Orange Pi Plus board lacks voltage scaling capabilities in its
> current form. This results in random freezes during boot when cpufreq is
> enabled, probably due to wrong voltages.
> 
> This patch (a copy/paste of 06139c) does the following things on this
> board:
> - enable r_i2c
> - add sy8106a to the r_i2c bus
> - have the sy8106a regulate VDD of cpu
> 
> I have tested this on two Orange Pi Plus boards, and they run stable
> across a range of voltages (and frequencies).

How was this tested?

> Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
> ---
>  arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts | 29 ++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
> index b403e5d78..fd64b5c94 100644
> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
> @@ -74,6 +74,10 @@
>  	};
>  };
>  
> +&cpu0 {
> +	cpu-supply = <&reg_vdd_cpux>;
> +};
> +
>  &ehci3 {
>  	status = "okay";
>  };
> @@ -119,6 +123,31 @@
>  	};
>  };
>  
> +&r_i2c {
> +	status = "okay";
> +
> +	reg_vdd_cpux: regulator@65 {
> +		compatible = "silergy,sy8106a";
> +		reg = <0x65>;
> +		regulator-name = "vdd-cpux";
> +		silergy,fixed-microvolt = <1200000>;
> +		/*
> +		 * The datasheet uses 1.1V as the minimum value of VDD-CPUX,
> +		 * however both the Armbian DVFS table and the official one
> +		 * have operating points with voltage under 1.1V, and both
> +		 * DVFS table are known to work properly at the lowest
> +		 * operating point.
> +		 *
> +		 * Use 1.0V as the minimum voltage instead.
> +		 */

There's not much point to that comment, since it's been that way for
all the other SoCs from Allwinner. However, that would be great to
have it in the commit log.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] dts: sun8i-h3: add sy8106a to orange pi plus

[Jorik Jonker]

Hi,

On Fri, Sep 28, 2018 at 03:55:16PM +0200, Maxime Ripard wrote:
>The prefix of your commit title should be "ARM: dts: ..."

Sure, will fix it in v2.

>> I have tested this on two Orange Pi Plus boards, and they run stable
>> across a range of voltages (and frequencies).
>
>How was this tested?

Basically using intermittent cpuburn runs and monitor freq/voltage/temp.  
I've included a description of my test in the v2 commit log.

>> +		/*
>> +		 * The datasheet uses 1.1V as the minimum value of VDD-CPUX,
>> +		 * however both the Armbian DVFS table and the official one
>> +		 * have operating points with voltage under 1.1V, and both
>> +		 * DVFS table are known to work properly at the lowest
>> +		 * operating point.
>> +		 *
>> +		 * Use 1.0V as the minimum voltage instead.
>> +		 */
>
>There's not much point to that comment, since it's been that way for
>all the other SoCs from Allwinner. However, that would be great to
>have it in the commit log.

Totally agree, but I thought: there must be a good reason this text is 
in the PC dts, so let's put it in the Plus dts as well. I've moved it 
from the patch to the log in the v2 patch, which I will send right away.

Thanks for reviewing!

Jorik

Re: [PATCH] dts: sun8i-h3: add sy8106a to orange pi plus

[Maxime Ripard]

Hi Jorik,

On Sat, Sep 29, 2018 at 03:16:17PM +0200, Jorik Jonker wrote:
> Hi,
> 
> On Fri, Sep 28, 2018 at 03:55:16PM +0200, Maxime Ripard wrote:
> > The prefix of your commit title should be "ARM: dts: ..."
> 
> Sure, will fix it in v2.
> 
> > > I have tested this on two Orange Pi Plus boards, and they run stable
> > > across a range of voltages (and frequencies).
> > 
> > How was this tested?
> 
> Basically using intermittent cpuburn runs and monitor freq/voltage/temp.
> I've included a description of my test in the v2 commit log.

You just missed the 4.20 merge window, unfortunately, but ideally,
having cpufreq-ljt-stress-test from
https://github.com/ssvb/cpuburn-arm would be great.

That won't prevent your patch from going in, but it would be great if
you could test this at some point during the 4.20-rc phase, and fix
things if they are broken.

> > > +		/*
> > > +		 * The datasheet uses 1.1V as the minimum value of VDD-CPUX,
> > > +		 * however both the Armbian DVFS table and the official one
> > > +		 * have operating points with voltage under 1.1V, and both
> > > +		 * DVFS table are known to work properly at the lowest
> > > +		 * operating point.
> > > +		 *
> > > +		 * Use 1.0V as the minimum voltage instead.
> > > +		 */
> > 
> > There's not much point to that comment, since it's been that way for
> > all the other SoCs from Allwinner. However, that would be great to
> > have it in the commit log.
> 
> Totally agree, but I thought: there must be a good reason this text is in
> the PC dts, so let's put it in the Plus dts as well. I've moved it from the
> patch to the log in the v2 patch, which I will send right away.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com