Re: [PATCH 1/6] clk: sunxi-ng: fix PLL_CPUX adjusting on A33

Re: [PATCH 1/6] clk: sunxi-ng: fix PLL_CPUX adjusting on A33

[Maxime Ripard]

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

On Tue, Dec 13, 2016 at 11:22:47PM +0800, Icenowy Zheng wrote:
> When adjusting PLL_CPUX on A33, the PLL is temporarily driven too high,
> and the system hangs.
> 
> Add a notifier to avoid this situation by temporarily switching to a
> known stable 24 MHz oscillator.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Applied, thanks!
Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/6] clk: sunxi-ng: set the parent rate when adjustin CPUX clock on A33

[Maxime Ripard]

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

On Tue, Dec 13, 2016 at 11:22:48PM +0800, Icenowy Zheng wrote:
> The CPUX clock on A33, which is for the Cortex-A7 cores, is designed to
> be changeable by changing the rate of PLL_CPUX.
> 
> Add CLK_SET_RATE_PARENT flag to this clock.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/6] clk: sunxi-ng: set the parent rate when adjustin CPUX clock on A33

[Maxime Ripard]

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

On Wed, Dec 14, 2016 at 04:54:14AM +0800, Icenowy Zheng wrote:
> 
> 
> 13.12.2016, 23:44, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> > On Tue, Dec 13, 2016 at 11:22:48PM +0800, Icenowy Zheng wrote:
> >>  The CPUX clock on A33, which is for the Cortex-A7 cores, is designed to
> >>  be changeable by changing the rate of PLL_CPUX.
> >>
> >>  Add CLK_SET_RATE_PARENT flag to this clock.
> >>
> >>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> >
> > Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Excuse me, have you merged this patch?

Yes, sorry, that's what I meant :)

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 3/6] ARM: dts: sun8i: add a cpu0 label to cpu@0 node on A23/33

[Maxime Ripard]

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

On Tue, Dec 13, 2016 at 11:22:49PM +0800, Icenowy Zheng wrote:
> A "cpu0" label is needed on cpu@0 for cpufreq-dt to work.
> 
> Add such a label, in order to prepare for cpufreq support of A23/33.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Applied, thanks!
Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 3/6] ARM: dts: sun8i: add a cpu0 label to cpu@0 node on A23/33

[Sudeep Holla]

On 13/12/16 15:22, Icenowy Zheng wrote:
> A "cpu0" label is needed on cpu@0 for cpufreq-dt to work.
> 

IIUC any label should be fine and I don't see anything in the driver
looking for such label name. All I see is it looks for cpu0 regulator
for *legacy* DTs

> Add such a label, in order to prepare for cpufreq support of A23/33.
> 

You need this as you add the same label in the following patches. The
commit message sounds like cpufreq-dt search for that label by name.

-- 
Regards,
Sudeep

Re: [PATCH 3/6] ARM: dts: sun8i: add a cpu0 label to cpu@0 node on A23/33

[Chen-Yu Tsai]

On Wed, Dec 14, 2016 at 12:09 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 13/12/16 15:22, Icenowy Zheng wrote:
>> A "cpu0" label is needed on cpu@0 for cpufreq-dt to work.
>>
>
> IIUC any label should be fine and I don't see anything in the driver
> looking for such label name. All I see is it looks for cpu0 regulator
> for *legacy* DTs
>
>> Add such a label, in order to prepare for cpufreq support of A23/33.
>>
>
> You need this as you add the same label in the following patches. The
> commit message sounds like cpufreq-dt search for that label by name.

I think a more proper explanation would be:

The cpu's supply regulator is specified at the board level, hence we
need to add a label to it to reference it without replicating the whole
tree structure.

ChenYu

Re: [PATCH 3/6] ARM: dts: sun8i: add a cpu0 label to cpu@0 node on A23/33

[Sudeep Holla]

On 13/12/16 16:31, Chen-Yu Tsai wrote:
> On Wed, Dec 14, 2016 at 12:09 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 13/12/16 15:22, Icenowy Zheng wrote:
>>> A "cpu0" label is needed on cpu@0 for cpufreq-dt to work.
>>>
>>
>> IIUC any label should be fine and I don't see anything in the driver
>> looking for such label name. All I see is it looks for cpu0 regulator
>> for *legacy* DTs
>>
>>> Add such a label, in order to prepare for cpufreq support of A23/33.
>>>
>>
>> You need this as you add the same label in the following patches. The
>> commit message sounds like cpufreq-dt search for that label by name.
> 
> I think a more proper explanation would be:
> 
> The cpu's supply regulator is specified at the board level, hence we
> need to add a label to it to reference it without replicating the whole
> tree structure.

Thanks for clarifying, was confused based on the commit log.

-- 
Regards,
Sudeep

Re: [PATCH 4/6] ARM: dts: sun8i: add opp-v2 table for A33

[Maxime Ripard]

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

On Tue, Dec 13, 2016 at 11:22:50PM +0800, Icenowy Zheng wrote:
> An operating point table is needed for the cpu frequency adjusting to
> work.
> 
> The operating point table is converted from the common value in
> extracted script.fex from many A33 board/tablets.
> 
> 1.344GHz is set as a turbo-mode operating point, as it's described as
> "extremity_freq" in the FEX file. (the "max_freq" is 1.2GHz)
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  arch/arm/boot/dts/sun8i-a33.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> index 504996cbee29..035c058324b8 100644
> --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> @@ -46,7 +46,45 @@
>  #include <dt-bindings/dma/sun4i-a10.h>
>  
>  / {
> +	cpu0_opp_table: opp_table0 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp@648000000 {
> +			opp-hz = /bits/ 64 <648000000>;
> +			opp-microvolt = <1040000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};

Please add new lines between the nodes.

> +		opp@816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <1100000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp@1008000000 {
> +			opp-hz = /bits/ 64 <1008000000>;
> +			opp-microvolt = <1200000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp@1200000000 {
> +			opp-hz = /bits/ 64 <1200000000>;
> +			opp-microvolt = <1320000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp@1344000000 {
> +			opp-hz = /bits/ 64 <1344000000>;
> +			opp-microvolt = <1460000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			turbo-mode;
> +		};

As far as I know, this OPP is not used by Allwinner, is not usable in
any A33 board so far (both the A33-olinuxino and the SinA33 do not
allow such a voltage on their CPU regulator), and overvolting and
overclocking is something that is very risky, and might lead to
stability issues.

Please remove this OPP.

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 5/6] ARM: dts: sun8i: set cpu-supply in reference tablet DTSI

[Maxime Ripard]

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

On Tue, Dec 13, 2016 at 11:22:51PM +0800, Icenowy Zheng wrote:
> All reference design A33 tablets uses DCDC2 of AXP223 as the power
> supply of the Cortex-A7 cores.
> 
> Set the cpu-supply in the DTSI of sun8i reference tablets.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Applied, thanks

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 6/6] ARM: dts: sun8i: raise the max voltage of DCDC2 in sun8i reference tablets

[Maxime Ripard]

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

On Tue, Dec 13, 2016 at 11:22:52PM +0800, Icenowy Zheng wrote:
> The "extremity_freq" frequency described in the original FEX files uses
> a voltage of 1.46v, which is beyond the current maximum voltage value of
> DCDC2 (Cortex-A7 supply) in the sun8i reference tablet DTSI file.
> 
> Raise the maximum value to 1.46v.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
> index 7ac8bb4bc95a..325ca5bd67a5 100644
> --- a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
> +++ b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
> @@ -180,7 +180,7 @@
>  &reg_dcdc2 {
>  	regulator-always-on;
>  	regulator-min-microvolt = <900000>;
> -	regulator-max-microvolt = <1400000>;
> +	regulator-max-microvolt = <1460000>;

This is outside of the voltage range tolerated by the CPU. NAK.

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]