Re: [linux-sunxi] [PATCH v2] arm64: dts: allwinner: h6: Use dummy regulator for Tanix TX6

2020-05-04 Thread Clément Péron
Hi Ondrej,

On Mon, 4 May 2020 at 14:27, Ondřej Jirman  wrote:
>
> Hi Clément,
>



>
> So I guess ignoring the voltage and not disabling this OPP may or may not work
> based on SoC bin.
>
> On Orange Pi One, there's a regulator that supports two voltages (that can't
> support all the listed OPPs for H3), and cpufreq-dt can deal with that
> automagically, if you specify OPP voltages via a tripplet of [prefered min 
> max].
> Kernell will log this in dmesg on boot:
>
> [0.672440] core: _opp_supported_by_regulators: OPP minuV: 132 maxuV: 
> 132, not supported by regulator
> [0.672454] cpu cpu0: _opp_add: OPP not supported by regulators 
> (110400)
> [0.672523] core: _opp_supported_by_regulators: OPP minuV: 132 maxuV: 
> 132, not supported by regulator
> [0.672530] cpu cpu0: _opp_add: OPP not supported by regulators 
> (12)
> [0.672621] core: _opp_supported_by_regulators: OPP minuV: 134 maxuV: 
> 134, not supported by regulator
> [0.672628] cpu cpu0: _opp_add: OPP not supported by regulators 
> (129600)
> [0.672712] core: _opp_supported_by_regulators: OPP minuV: 140 maxuV: 
> 140, not supported by regulator
> [0.672719] cpu cpu0: _opp_add: OPP not supported by regulators 
> (136800)
>
> And the list of available OPPs will be reduced at runtime to a supportable
> set by the CPU regulator.
>
> If you look at:
>
>   
> https://megous.com/git/linux/commit/?h=ths-5.7=d231770195913cf543c0cf9539deee2ecec06680
>
> you'll see a bunch of OPPs for H3 that are specified as a range. So
> for example if you want 480MHz, and your regulator can't produce
> 1.04V exactly, cpufreq will set the voltage to something supportable
> in the range.
>
> I think the proper fix is to fix the OPP table for H6, so that it uses
> voltage ranges for each OPP and not a single fixed voltage, to support
> boards that don't have the standard PMIC that goes with H6.

Thanks for the suggestion and I agree with you, this is a good way to
keep the same OPP table for all the H6 devices and handle both board
with PMIC and with fixed regulator.

I will propose a patch.

Thanks clement

>
> regards,
> o.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/CAJiuCcdSmgp75ByEDDtH0AtqGsUyc9QrAcD9xqLZduh2ijnrqQ%40mail.gmail.com.


Re: [linux-sunxi] [PATCH v2] arm64: dts: allwinner: h6: Use dummy regulator for Tanix TX6

2020-05-04 Thread Ondřej Jirman
Hi Clément,

On Tue, Apr 28, 2020 at 04:26:29PM +0200, Clément Péron wrote:
> Tanix TX6 has a fixed regulator. As DVFS is instructed to change
> voltage to meet OPP table, the DVFS is not working as expected.
> 
> Avoid to introduce a new dedicated OPP Table where voltage are
> equals to the fixed regulator as it will only duplicate all the OPPs.
> Instead remove the fixed regulator so the DVFS framework will create
> dummy regulator and will have the same behavior.
> 
> Add some comments to explain this in the device-tree.
> 
> Reported-by: Piotr Oniszczuk 
> Fixes: add1e27fb703 ("arm64: dts: allwinner: h6: Enable CPU opp tables for 
> Tanix TX6")
> Signed-off-by: Clément Péron 
> ---
>  .../boot/dts/allwinner/sun50i-h6-tanix-tx6.dts | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts 
> b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
> index be81330db14f..3e96fcb317ea 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
> @@ -48,7 +48,15 @@
>  };
>  
>   {
> - cpu-supply = <_vdd_cpu_gpu>;
> + /*
> +  * Don't specify the CPU regulator, as it's a fixed
> +  * regulator DVFS will not work as it is intructed
> +  * to reach a voltage which can't be reached.
> +  * Not specifying a regulator will create a dummy
> +  * regulator allowing all OPPs.
> +  *
> +  * cpu-supply = <_vdd_cpu_gpu>;
> +  */

reg_vdd_cpu_gpu has 

regulator-min-microvolt = <1135000>;
regulator-max-microvolt = <1135000>;

top OPP is:

opp@18 {
clock-latency-ns = <244144>; /* 8 32k periods */
opp-hz = /bits/ 64 <18>;

opp-microvolt-speed0 = <116>;
opp-microvolt-speed1 = <110>;
opp-microvolt-speed2 = <110>;
};

So I guess ignoring the voltage and not disabling this OPP may or may not work
based on SoC bin.

On Orange Pi One, there's a regulator that supports two voltages (that can't
support all the listed OPPs for H3), and cpufreq-dt can deal with that
automagically, if you specify OPP voltages via a tripplet of [prefered min max].
Kernell will log this in dmesg on boot:

[0.672440] core: _opp_supported_by_regulators: OPP minuV: 132 maxuV: 
132, not supported by regulator
[0.672454] cpu cpu0: _opp_add: OPP not supported by regulators (110400)
[0.672523] core: _opp_supported_by_regulators: OPP minuV: 132 maxuV: 
132, not supported by regulator
[0.672530] cpu cpu0: _opp_add: OPP not supported by regulators (12)
[0.672621] core: _opp_supported_by_regulators: OPP minuV: 134 maxuV: 
134, not supported by regulator
[0.672628] cpu cpu0: _opp_add: OPP not supported by regulators (129600)
[0.672712] core: _opp_supported_by_regulators: OPP minuV: 140 maxuV: 
140, not supported by regulator
[0.672719] cpu cpu0: _opp_add: OPP not supported by regulators (136800)

And the list of available OPPs will be reduced at runtime to a supportable
set by the CPU regulator.

If you look at:

  
https://megous.com/git/linux/commit/?h=ths-5.7=d231770195913cf543c0cf9539deee2ecec06680

you'll see a bunch of OPPs for H3 that are specified as a range. So
for example if you want 480MHz, and your regulator can't produce
1.04V exactly, cpufreq will set the voltage to something supportable
in the range.

I think the proper fix is to fix the OPP table for H6, so that it uses
voltage ranges for each OPP and not a single fixed voltage, to support
boards that don't have the standard PMIC that goes with H6.

regards,
o.

>  };
>  
>   {
> @@ -68,7 +76,13 @@
>  };
>  
>   {
> - mali-supply = <_vdd_cpu_gpu>;
> + /*
> +  * Don't specify the GPU regulator, see comment
> +  * above for the CPU supply.
> +  *
> +  * mali-supply = <_vdd_cpu_gpu>;
> +  */
> +
>   status = "okay";
>  };
>  
> -- 
> 2.20.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> To view this discussion on the web, visit 
> https://groups.google.com/d/msgid/linux-sunxi/20200428142629.8950-1-peron.clem%40gmail.com.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20200504122742.er2jd67bvrn2rfgp%40core.my.home.


[linux-sunxi] [PATCH v2] arm64: dts: allwinner: h6: Use dummy regulator for Tanix TX6

2020-04-28 Thread Clément Péron
Tanix TX6 has a fixed regulator. As DVFS is instructed to change
voltage to meet OPP table, the DVFS is not working as expected.

Avoid to introduce a new dedicated OPP Table where voltage are
equals to the fixed regulator as it will only duplicate all the OPPs.
Instead remove the fixed regulator so the DVFS framework will create
dummy regulator and will have the same behavior.

Add some comments to explain this in the device-tree.

Reported-by: Piotr Oniszczuk 
Fixes: add1e27fb703 ("arm64: dts: allwinner: h6: Enable CPU opp tables for 
Tanix TX6")
Signed-off-by: Clément Péron 
---
 .../boot/dts/allwinner/sun50i-h6-tanix-tx6.dts | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
index be81330db14f..3e96fcb317ea 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
@@ -48,7 +48,15 @@
 };
 
  {
-   cpu-supply = <_vdd_cpu_gpu>;
+   /*
+* Don't specify the CPU regulator, as it's a fixed
+* regulator DVFS will not work as it is intructed
+* to reach a voltage which can't be reached.
+* Not specifying a regulator will create a dummy
+* regulator allowing all OPPs.
+*
+* cpu-supply = <_vdd_cpu_gpu>;
+*/
 };
 
  {
@@ -68,7 +76,13 @@
 };
 
  {
-   mali-supply = <_vdd_cpu_gpu>;
+   /*
+* Don't specify the GPU regulator, see comment
+* above for the CPU supply.
+*
+* mali-supply = <_vdd_cpu_gpu>;
+*/
+
status = "okay";
 };
 
-- 
2.20.1

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20200428142629.8950-1-peron.clem%40gmail.com.