[RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[RFC] ARM: dts: sunxi: Add regulators and board-specific operating points 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. With the
regulators enabled, we can define board-specific operating points. The defined
CPU voltages are more conservative (based on the values used by the vendor),
so they should be more failsafe across all boards of this kind out there.

I'm posting this as RFC as I would like to make a few more remarks and raise
questions along the way (plus, I'm anything but an experienced developer, so a
a critical review might help).

I checked the regulator definitions against the schematics released by LeMaker.
I also compared that to the DT and schematics of Cubiboard 2 and Cubietruck. Of
course, I also tested the patch on the actual hardware and it works fine for me.
The CPU voltages are slightly higher than those set in sun7i-a20.dtsi and even
though they work well for me, I thought it might be safer to use the more
conservative values used by LeMaker in their old fex file. Would you agree?
Besides, it it ok to have this in one patch or should it be splitted in two
(one for the regulators and one for the opp)? Did I miss something important?

Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>
---
 arch/arm/boot/dts/sun7i-a20-bananapi.dts | 47 +++++++++++++++++++++++++++++---
 1 file changed, 43 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..2bcbb0e 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -92,6 +92,22 @@
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+	operating-points = <
+		/* kHz	  uV */
+		1008000	1450000
+		960000	1425000
+		912000	1425000
+		864000	1350000
+		720000	1250000
+		528000	1150000
+		312000	1100000
+		144000	1050000
+		>;
+	cooling-max-level = <7>;
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -119,13 +135,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 +194,33 @@
 	};
 };
 
+#include "axp209.dtsi"
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1050000>;
+	regulator-max-microvolt = <1450000>;
+	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: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Hans de Goede]

Hi,

On 27-07-15 03:28, 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. With the
> regulators enabled, we can define board-specific operating points. The defined
> CPU voltages are more conservative (based on the values used by the vendor),
> so they should be more failsafe across all boards of this kind out there.
>
> I'm posting this as RFC as I would like to make a few more remarks and raise
> questions along the way (plus, I'm anything but an experienced developer, so a
> a critical review might help).
>
> I checked the regulator definitions against the schematics released by LeMaker.
> I also compared that to the DT and schematics of Cubiboard 2 and Cubietruck. Of
> course, I also tested the patch on the actual hardware and it works fine for me.
> The CPU voltages are slightly higher than those set in sun7i-a20.dtsi and even
> though they work well for me, I thought it might be safer to use the more
> conservative values used by LeMaker in their old fex file. Would you agree?
> Besides, it it ok to have this in one patch or should it be splitted in two
> (one for the regulators and one for the opp)? Did I miss something important?
>
> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>

I've a simular patch here:

https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1

I did not submit this upstream yet as it is part of a series to enable the otg
controller on the bananapi which needs axp-usb-power-supply support for which
the actual powersupply driver changes are still pending.

As you can see other then you adding the cpu operating points are patches are
identical, which is good :)

IMHO we should just stick with the standard operating points unless we know
that there are stability issues with them (such as e.g. on the A10 OlinuxIno Lime).

Regards,

Hans



> ---
>   arch/arm/boot/dts/sun7i-a20-bananapi.dts | 47 +++++++++++++++++++++++++++++---
>   1 file changed, 43 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..2bcbb0e 100644
> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> @@ -92,6 +92,22 @@
>   	status = "okay";
>   };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +	operating-points = <
> +		/* kHz	  uV */
> +		1008000	1450000
> +		960000	1425000
> +		912000	1425000
> +		864000	1350000
> +		720000	1250000
> +		528000	1150000
> +		312000	1100000
> +		144000	1050000
> +		>;
> +	cooling-max-level = <7>;
> +};
> +
>   &ehci0 {
>   	status = "okay";
>   };
> @@ -119,13 +135,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 +194,33 @@
>   	};
>   };
>
> +#include "axp209.dtsi"
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1050000>;
> +	regulator-max-microvolt = <1450000>;
> +	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: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[public_timo.s]

Hi,

Hans de Goede schrieb am 27.07.2015 10:07:

> I've a simular patch here:
> 
> https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1
> 
> I did not submit this upstream yet as it is part of a series to enable the otg
> controller on the bananapi which needs axp-usb-power-supply support for which
> the actual powersupply driver changes are still pending.
Oops, I see. Are you planning to submit this for 4.3 or later?

> As you can see other then you adding the cpu operating points are patches are
> identical, which is good :)
Yep, that and you chose a slightly higher maximum voltage for the CPU.

> IMHO we should just stick with the standard operating points unless we know
> that there are stability issues with them (such as e.g. on the A10 OlinuxIno
> Lime).
I'd be fine with that as I don't have any stability issues with the lower
voltages. What about the 1008MHz operating point that I "reintroduced"? It was
dropped here [1] because there was no regulator support. Can this be reenabled
on board level (which means overriding the defaults inherited from
sun7i-a20.dtsi) or should this be done at SOC level for all boards (which
means we have to add regulator nodes for all boards in the first place)?


Regards,

Timo


[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=370a9b5fb04a0d5cc7b7699c788616d6976f4476

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Hans de Goede]

Hi,

On 27-07-15 14:09, public_timo.s@silentcreek.de wrote:
> Hi,
>
> Hans de Goede schrieb am 27.07.2015 10:07:
>
>> I've a simular patch here:
>>
>> https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1
>>
>> I did not submit this upstream yet as it is part of a series to enable the otg
>> controller on the bananapi which needs axp-usb-power-supply support for which
>> the actual powersupply driver changes are still pending.
> Oops, I see. Are you planning to submit this for 4.3 or later?

I plan to submit this for 4.3.

>> As you can see other then you adding the cpu operating points are patches are
>> identical, which is good :)
> Yep, that and you chose a slightly higher maximum voltage for the CPU.
>
>> IMHO we should just stick with the standard operating points unless we know
>> that there are stability issues with them (such as e.g. on the A10 OlinuxIno
>> Lime).
> I'd be fine with that as I don't have any stability issues with the lower
> voltages. What about the 1008MHz operating point that I "reintroduced"? It was
> dropped here [1]  because there was no regulator support.

That is in essence an overclocked setting, the max CPU voltage officially is
1.4V, I do not think that we should provide any overclocked settings in the
official dts files. If people really want to overclock they will have to
modify there dts themselves IMHO.

 > Can this be reenabled
> on board level (which means overriding the defaults inherited from
> sun7i-a20.dtsi) or should this be done at SOC level for all boards (which
> means we have to add regulator nodes for all boards in the first place)?

Technically this is possible, but I do not think that it is a good idea.

Regards,

Hans

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Timo Sigurdsson]

Hi,

Hans de Goede schrieb am 27.07.2015 14:43:
>>> I've a simular patch here:
>>>
>>> https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1
>>>
>>> I did not submit this upstream yet as it is part of a series to enable the
>>> otg
>>> controller on the bananapi which needs axp-usb-power-supply support for which
>>> the actual powersupply driver changes are still pending.
>> Oops, I see. Are you planning to submit this for 4.3 or later?
> 
> I plan to submit this for 4.3.
Ok, then I guess we can drop my patch.

>>> IMHO we should just stick with the standard operating points unless we know
>>> that there are stability issues with them (such as e.g. on the A10 OlinuxIno
>>> Lime).
>> I'd be fine with that as I don't have any stability issues with the lower
>> voltages. What about the 1008MHz operating point that I "reintroduced"? It was
>> dropped here [1]  because there was no regulator support.
> 
> That is in essence an overclocked setting, the max CPU voltage officially is
> 1.4V, I do not think that we should provide any overclocked settings in the
> official dts files. If people really want to overclock they will have to
> modify there dts themselves IMHO.

Personally, I would be fine with that. Even though I think, it might be good to
have them in the official files just for convenience and because people who are 
used to the sunxi-3.4 kernels are used to having the 1008MHz opp (and it was in
mainline for a short while, too). For those who don't want to use that setting,
it's easier to limit the maximum in userspace compared to compiling a new
device tree blob. But I do understand your point, so I guess it's just
something that maintainers have to make a decision for. As I said, either way
is fine for me.

> > Can this be reenabled
>> on board level (which means overriding the defaults inherited from
>> sun7i-a20.dtsi) or should this be done at SOC level for all boards (which
>> means we have to add regulator nodes for all boards in the first place)?
> 
> Technically this is possible, but I do not think that it is a good idea.

I guess the same applies here, too. It's something maintainers should have a
common understanding on. I don't know how much variation there is among the
A20 boards in terms of frequencies and voltages. If there is a lot, I'd say
it would be desireable to have board-specific opp. The downside I see in my
approach is that it impacts readability of the dts(i) files when settings
are overridden further down the tree.

Thanks and regards,

Timo

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Maxime Ripard]

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

On Tue, Jul 28, 2015 at 11:02:09AM +0200, Timo Sigurdsson wrote:
> Hi,
> 
> Hans de Goede schrieb am 27.07.2015 14:43:
> >>> I've a simular patch here:
> >>>
> >>> https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1
> >>>
> >>> I did not submit this upstream yet as it is part of a series to enable the
> >>> otg
> >>> controller on the bananapi which needs axp-usb-power-supply support for which
> >>> the actual powersupply driver changes are still pending.
> >> Oops, I see. Are you planning to submit this for 4.3 or later?
> > 
> > I plan to submit this for 4.3.
>
> Ok, then I guess we can drop my patch.

Please don't.

> >>> IMHO we should just stick with the standard operating points unless we know
> >>> that there are stability issues with them (such as e.g. on the A10 OlinuxIno
> >>> Lime).
> >> I'd be fine with that as I don't have any stability issues with the lower
> >> voltages. What about the 1008MHz operating point that I "reintroduced"? It was
> >> dropped here [1]  because there was no regulator support.
> > 
> > That is in essence an overclocked setting, the max CPU voltage officially is
> > 1.4V, I do not think that we should provide any overclocked settings in the
> > official dts files. If people really want to overclock they will have to
> > modify there dts themselves IMHO.
> 
> Personally, I would be fine with that. Even though I think, it might
> be good to have them in the official files just for convenience and
> because people who are used to the sunxi-3.4 kernels are used to
> having the 1008MHz opp (and it was in mainline for a short while,
> too).

It was used in mainline, and reverted because it was not stable
enough.

There's a lot of things we do differently in mainline, it's one of
them. If someone can provide an OPP for 1008MHz that is stable for all
the boards and within the operating limits of the SoC, I'd be totally
fine with that, but we didn't find it so far.

> For those who don't want to use that setting, it's easier to
> limit the maximum in userspace compared to compiling a new device
> tree blob.

Except that the kernel should not rely on the userspace to be stable
and harmless for the hardware. It should just work reliably by itself.

> But I do understand your point, so I guess it's just something that
> maintainers have to make a decision for. As I said, either way is
> fine for me.
> 
> > > Can this be reenabled
> >> on board level (which means overriding the defaults inherited from
> >> sun7i-a20.dtsi) or should this be done at SOC level for all boards (which
> >> means we have to add regulator nodes for all boards in the first place)?
> > 
> > Technically this is possible, but I do not think that it is a good idea.
> 
> I guess the same applies here, too. It's something maintainers should have a
> common understanding on. I don't know how much variation there is among the
> A20 boards in terms of frequencies and voltages.

If you look at the FEX file, a lot. But most of them are just
variations of the same OPP.

I value much more a set of OPPs that has been tested on a wide range
of device, that we know are working reliably on all of them, over a
FEX file providing a set of OPPs that for some of them have never even
been tested before because the 3.4 kernel simply ignored them.

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] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Timo Sigurdsson]

Hi,

Maxime Ripard schrieb am 28.07.2015 14:55:
>> > I plan to submit this for 4.3.
>>
>> Ok, then I guess we can drop my patch.
> 
> Please don't.

Ok.

> It was used in mainline, and reverted because it was not stable
> enough.

Well, the explanation given was, it was not stable because of the missing
regulator support. I don't know whether anyboady actually ever reported
it wouldn't run fine at 1008MHz with 1.45V. So the actual point should be
whether we wan't voltages that are out of the specified range in the
official kernel. The consensus seems to be no, with good reasons for that.
So, I won't object this.

> There's a lot of things we do differently in mainline, it's one of
> them. If someone can provide an OPP for 1008MHz that is stable for all
> the boards and within the operating limits of the SoC, I'd be totally
> fine with that, but we didn't find it so far.
> 
>> For those who don't want to use that setting, it's easier to
>> limit the maximum in userspace compared to compiling a new device
>> tree blob.
> 
> Except that the kernel should not rely on the userspace to be stable
> and harmless for the hardware. It should just work reliably by itself.

Same as above.

Regards,

Timo

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Maxime Ripard]

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

Hi,

On Mon, Jul 27, 2015 at 02:43:20PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 27-07-15 14:09, public_timo.s@silentcreek.de wrote:
> >Hi,
> >
> >Hans de Goede schrieb am 27.07.2015 10:07:
> >
> >>I've a simular patch here:
> >>
> >>https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1
> >>
> >>I did not submit this upstream yet as it is part of a series to enable the otg
> >>controller on the bananapi which needs axp-usb-power-supply support for which
> >>the actual powersupply driver changes are still pending.
> >
> >Oops, I see. Are you planning to submit this for 4.3 or later?
> 
> I plan to submit this for 4.3.

I don't feel like holding patches that were posted before you did
because you did them some time ago and never submitted them is
reasonnable and / or encouraging for new submitters of patches.

I'd really like to get more sunxi-people contributing, and that starts
with that kind of trivial stuff. Holding them back because one of the
usual (and experienced) developpers is a bit counter-productive (and
I'm sure you still have a lot of patches to submit anyway ;)).

> >>As you can see other then you adding the cpu operating points are patches are
> >>identical, which is good :)
> >Yep, that and you chose a slightly higher maximum voltage for the CPU.
> >
> >>IMHO we should just stick with the standard operating points unless we know
> >>that there are stability issues with them (such as e.g. on the A10 OlinuxIno
> >>Lime).
> >I'd be fine with that as I don't have any stability issues with the lower
> >voltages. What about the 1008MHz operating point that I "reintroduced"? It was
> >dropped here [1]  because there was no regulator support.
> 
> That is in essence an overclocked setting, the max CPU voltage officially is
> 1.4V, I do not think that we should provide any overclocked settings in the
> official dts files. If people really want to overclock they will have to
> modify there dts themselves IMHO.

I fully agree.

> > Can this be reenabled on board level (which means overriding the
> >defaults inherited from sun7i-a20.dtsi) or should this be done at
> >SOC level for all boards (which means we have to add regulator
> >nodes for all boards in the first place)?
> 
> Technically this is possible, but I do not think that it is a good idea.

And here as well.

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] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Hans de Goede]

Hi,

On 07/28/2015 02:49 PM, Maxime Ripard wrote:
> Hi,
>
> On Mon, Jul 27, 2015 at 02:43:20PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 27-07-15 14:09, public_timo.s@silentcreek.de wrote:
>>> Hi,
>>>
>>> Hans de Goede schrieb am 27.07.2015 10:07:
>>>
>>>> I've a simular patch here:
>>>>
>>>> https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1
>>>>
>>>> I did not submit this upstream yet as it is part of a series to enable the otg
>>>> controller on the bananapi which needs axp-usb-power-supply support for which
>>>> the actual powersupply driver changes are still pending.
>>>
>>> Oops, I see. Are you planning to submit this for 4.3 or later?
>>
>> I plan to submit this for 4.3.
>
> I don't feel like holding patches that were posted before you did
> because you did them some time ago and never submitted them is
> reasonnable and / or encouraging for new submitters of patches.
>
> I'd really like to get more sunxi-people contributing, and that starts
> with that kind of trivial stuff. Holding them back because one of the
> usual (and experienced) developpers is a bit counter-productive (and
> I'm sure you still have a lot of patches to submit anyway ;)).

I've no problem with Timo submitting a cleaned up version of his
patch and you taking that instead. I just wanted to point out that
I do have a similar patch pending.

Regards,

Hans

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Timo Sigurdsson]

Hi,

Hans de Goede schrieb am 28.07.2015 16:24:
> I've no problem with Timo submitting a cleaned up version of his
> patch and you taking that instead. I just wanted to point out that
> I do have a similar patch pending.

Ok, I will do that. It might take a couple of days, though, as I will be moving
moving in the next few days as well.

However, another question first then: What maximum voltage for the dcdc2
regulator should we use then? You used 1.5V, I used 1.45V so far, which is what
Cubieboard 2 and Cubietruck use.

But after the discussion about overvoltaged settings, I'm wondering whether we
should limit the regulartor (and not only the opp) to 1.40V as well? Or is
1.45V ok for everybody here?


Thanks and regards,

Timo

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Hans de Goede]

Hi,

On 07/28/2015 05:09 PM, Timo Sigurdsson wrote:
> Hi,
>
> Hans de Goede schrieb am 28.07.2015 16:24:
>> I've no problem with Timo submitting a cleaned up version of his
>> patch and you taking that instead. I just wanted to point out that
>> I do have a similar patch pending.
>
> Ok, I will do that. It might take a couple of days, though, as I will be moving
> moving in the next few days as well.
>
> However, another question first then: What maximum voltage for the dcdc2
> regulator should we use then? You used 1.5V, I used 1.45V so far, which is what
> Cubieboard 2 and Cubietruck use.
>
> But after the discussion about overvoltaged settings, I'm wondering whether we
> should limit the regulartor (and not only the opp) to 1.40V as well? Or is
> 1.45V ok for everybody here?

The datasheets lists a max cpu-voltage of 1.40V, so lets stick with that.

Regards,

Hans

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Timo Sigurdsson]

Hi Stefan,

you didn't include me in your answer, hence the late reply...

Stefan Monnier schrieb am 29.07.2015 02:02:
>> IMHO for a common maximum opp that's a good approach. But for the lowest
>> frequency setting, it would seem more logical to me, to raise the voltage
>> to a point where all boards will run fine with them, unless those boards
>> cannot handle the frequency regardless of the higher voltage.

> I generally agre, tho IIRC some measurements seem to indicate that the
> lowest frequency settings did not bring much (if any) reduction in power
> consumption, making them rather useless.

That's an interesting point. However, that would be a different discussion. I
wouldn't mind if the 144MHz would be removed - my point was just that if it's 
supposed to be a common setting, it should work on all boards. I posted a patch
today to fix this. [1] That might be a good place to discuss whether this
setting should be removed entirely.

Regards,

Timo

[1] https://groups.google.com/forum/#!topic/linux-sunxi/fIfbdn7mrQA

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Timo Sigurdsson]

Hi,

Maxime Ripard schrieb am 28.07.2015 14:49:
> I don't feel like holding patches that were posted before you did
> because you did them some time ago and never submitted them is
> reasonnable and / or encouraging for new submitters of patches.
> 
> I'd really like to get more sunxi-people contributing, and that starts
> with that kind of trivial stuff. Holding them back because one of the
> usual (and experienced) developpers is a bit counter-productive (and
> I'm sure you still have a lot of patches to submit anyway ;)).

Just for the record: I wouldn't be discouraged by that. The only thing that
matters to me here is that regulator support will be added, regardless of 
who submitted the patch. But anyway, I will rework the patch along the 
statements made during this discussion.

Regards,

Timo

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[public_timo.s]

Hi again,

one more thing...

Hans de Goede schrieb am 27.07.2015 10:07:
> IMHO we should just stick with the standard operating points unless we know
> that there are stability issues with them (such as e.g. on the A10 OlinuxIno
> Lime).

If we stick to the standard operating points, shouldn't we honor their voltages
as well? If we set the minimum voltage for dcdc2 to 1.0V (as you did) or 1.05V
(as I did), this means that the lowest operating point of 144MHz@0.9V defined
in sun7i-a20.dtsi (and with my definition the 312MHz opp as well) will not be
available because cpufreq-dt simply ignores operating points that aren't within
the set voltage range for that regulator.


Regards,

Timo

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Hans de Goede]

Hi,

On 27-07-15 14:36, public_timo.s@silentcreek.de wrote:
> Hi again,
>
> one more thing...
>
> Hans de Goede schrieb am 27.07.2015 10:07:
>> IMHO we should just stick with the standard operating points unless we know
>> that there are stability issues with them (such as e.g. on the A10 OlinuxIno
>> Lime).
>
> If we stick to the standard operating points, shouldn't we honor their voltages
> as well? If we set the minimum voltage for dcdc2 to 1.0V (as you did) or 1.05V
> (as I did), this means that the lowest operating point of 144MHz@0.9V defined
> in sun7i-a20.dtsi (and with my definition the 312MHz opp as well) will not be
> available because cpufreq-dt simply ignores operating points that aren't within
> the set voltage range for that regulator.

Ah, good point. I don't know what is the right thing todo here.

ChenYu (in the CC), since you did most of the original work here, do
you know why we have an op at 0.9 volt, but none of our boards allow the
voltage to go that low in the regulator settings ?

Regards,

Hans

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Timo Sigurdsson]

Hi,

Chen-Yu Tsai schrieb am 27.07.2015 15:14:
>> ChenYu (in the CC), since you did most of the original work here, do
>> you know why we have an op at 0.9 volt, but none of our boards allow the
>> voltage to go that low in the regulator settings ?
> 
> I'm on vacation now, so apologies for the bad formatting.
> 
> The OPPs are from a common subset
> of the settings from the fex files available
> at the time. Some fex files actually set
> min and max voltage thus eliminating
> the highest and lowest OPPs.
> 
> At least that is how Maxime and I
> interpreted them.

IMHO for a common maximum opp that's a good approach. But for the lowest
frequency setting, it would seem more logical to me, to raise the voltage
to a point where all boards will run fine with them, unless those boards 
cannot handle the frequency regardless of the higher voltage.

Regards,

Timo

Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Maxime Ripard]

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

On Tue, Jul 28, 2015 at 11:02:16AM +0200, Timo Sigurdsson wrote:
> Hi,
> 
> Chen-Yu Tsai schrieb am 27.07.2015 15:14:
> >> ChenYu (in the CC), since you did most of the original work here, do
> >> you know why we have an op at 0.9 volt, but none of our boards allow the
> >> voltage to go that low in the regulator settings ?
> > 
> > I'm on vacation now, so apologies for the bad formatting.
> > 
> > The OPPs are from a common subset
> > of the settings from the fex files available
> > at the time. Some fex files actually set
> > min and max voltage thus eliminating
> > the highest and lowest OPPs.
> > 
> > At least that is how Maxime and I
> > interpreted them.
> 
> IMHO for a common maximum opp that's a good approach. But for the lowest
> frequency setting, it would seem more logical to me, to raise the voltage
> to a point where all boards will run fine with them, unless those boards 
> cannot handle the frequency regardless of the higher voltage.

Agreed.

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] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi

[Timo Sigurdsson]

Hi,

Maxime Ripard schrieb am 28.07.2015 14:55:
>> IMHO for a common maximum opp that's a good approach. But for the lowest
>> frequency setting, it would seem more logical to me, to raise the voltage
>> to a point where all boards will run fine with them, unless those boards 
>> cannot handle the frequency regardless of the higher voltage.
> 
> Agreed.

Ok, then I will write another patch for this as well, unless Chen-Yu or
someone else objects.

Regards,

Timo