[PATCH 1/2] clk: sunxi-ng: Fix CPUX clock for the A23/A33

[PATCH 1/2] clk: sunxi-ng: Fix CPUX clock for the A23/A33

[Icenowy Zheng]

The CPUX clock of A23/33 CCU driver forgot to add CLK_SET_RATE_PARENT
flag, which makes its frequency fail to change (as part of cpu thermal
throttle).

Fix it by add this flag.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
Patch 4.9-rc too.
 drivers/clk/sunxi-ng/ccu-sun8i-a23.c | 2 +-
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
index 2646d98..44c4775 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
@@ -163,7 +163,7 @@ static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_de_clk, "pll-de",
 static const char * const cpux_parents[] = { "osc32k", "osc24M",
 					     "pll-cpux" , "pll-cpux" };
 static SUNXI_CCU_MUX(cpux_clk, "cpux", cpux_parents,
-		     0x050, 16, 2, CLK_IS_CRITICAL);
+		     0x050, 16, 2, CLK_IS_CRITICAL | CLK_SET_RATE_PARENT);
 
 static SUNXI_CCU_M(axi_clk, "axi", "cpux", 0x050, 0, 2, 0);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
index 96b40ca..59cfdc8 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
@@ -170,7 +170,7 @@ static SUNXI_CCU_N_WITH_GATE_LOCK(pll_ddr1_clk, "pll-ddr1",
 static const char * const cpux_parents[] = { "osc32k", "osc24M",
 					     "pll-cpux" , "pll-cpux" };
 static SUNXI_CCU_MUX(cpux_clk, "cpux", cpux_parents,
-		     0x050, 16, 2, CLK_IS_CRITICAL);
+		     0x050, 16, 2, CLK_IS_CRITICAL | CLK_SET_RATE_PARENT);
 
 static SUNXI_CCU_M(axi_clk, "axi", "cpux", 0x050, 0, 2, 0);
 
-- 
2.10.1

[PATCH 2/2] clk: sunxi-ng: fix up PLL_CPUX adjusting for A23/A33

[Icenowy Zheng]

When adjusting PLL_CPUX on A23/A33, the PLL is driven too high, and the
system stucks.

Add a notifier to avoid this situation.

The code is ported from ccu-sun6i-a31.c.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
Patch 4.9-rc too.
 drivers/clk/sunxi-ng/ccu-sun8i-a23.c | 10 ++++++++++
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
index 44c4775..41a8594 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
@@ -709,6 +709,13 @@ static const struct sunxi_ccu_desc sun8i_a23_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sun8i_a23_ccu_resets),
 };
 
+static struct ccu_mux_nb sun8i_a23_cpu_nb = {
+	.common		= &cpux_clk.common,
+	.cm		= &cpux_clk.mux,
+	.delay_us	= 1, /* > 8 clock cycles at 24 MHz */
+	.bypass_index	= 1, /* index of 24 MHz oscillator */
+};
+
 static void __init sun8i_a23_ccu_setup(struct device_node *node)
 {
 	void __iomem *reg;
@@ -732,6 +739,9 @@ static void __init sun8i_a23_ccu_setup(struct device_node *node)
 	writel(val, reg + SUN8I_A23_PLL_MIPI_REG);
 
 	sunxi_ccu_probe(node, reg, &sun8i_a23_ccu_desc);
+
+	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+				  &sun8i_a23_cpu_nb);
 }
 CLK_OF_DECLARE(sun8i_a23_ccu, "allwinner,sun8i-a23-ccu",
 	       sun8i_a23_ccu_setup);
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
index 59cfdc8..3efbb6e 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
@@ -752,6 +752,13 @@ static const struct sunxi_ccu_desc sun8i_a33_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sun8i_a33_ccu_resets),
 };
 
+static struct ccu_mux_nb sun8i_a33_cpu_nb = {
+	.common		= &cpux_clk.common,
+	.cm		= &cpux_clk.mux,
+	.delay_us	= 1, /* > 8 clock cycles at 24 MHz */
+	.bypass_index	= 1, /* index of 24 MHz oscillator */
+};
+
 static void __init sun8i_a33_ccu_setup(struct device_node *node)
 {
 	void __iomem *reg;
@@ -775,6 +782,9 @@ static void __init sun8i_a33_ccu_setup(struct device_node *node)
 	writel(val, reg + SUN8I_A33_PLL_MIPI_REG);
 
 	sunxi_ccu_probe(node, reg, &sun8i_a33_ccu_desc);
+
+	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+				  &sun8i_a33_cpu_nb);
 }
 CLK_OF_DECLARE(sun8i_a33_ccu, "allwinner,sun8i-a33-ccu",
 	       sun8i_a33_ccu_setup);
-- 
2.10.1

[PATCH 2/2] clk: sunxi-ng: fix up PLL_CPUX adjusting for A23/A33

[Quentin Schulz]

Hi Icenowy,

This patch (2/2) should be before the first one.

The first patch allows adjusting of the PLL and the second fixes a
problem with the adjustment of the PLL.

You should fix the problem before allowing the adjustment of the PLL.
That way, if someone builds the kernel between your two patches, the
kernel is supposed to be working.

Tested in correct order on A33.

Thanks!

Quentin

On 06/11/2016 18:29, Icenowy Zheng wrote:
> When adjusting PLL_CPUX on A23/A33, the PLL is driven too high, and the
> system stucks.
> 
> Add a notifier to avoid this situation.
> 
> The code is ported from ccu-sun6i-a31.c.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
> Patch 4.9-rc too.
>  drivers/clk/sunxi-ng/ccu-sun8i-a23.c | 10 ++++++++++
>  drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 10 ++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
> index 44c4775..41a8594 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
> @@ -709,6 +709,13 @@ static const struct sunxi_ccu_desc sun8i_a23_ccu_desc = {
>  	.num_resets	= ARRAY_SIZE(sun8i_a23_ccu_resets),
>  };
>  
> +static struct ccu_mux_nb sun8i_a23_cpu_nb = {
> +	.common		= &cpux_clk.common,
> +	.cm		= &cpux_clk.mux,
> +	.delay_us	= 1, /* > 8 clock cycles at 24 MHz */
> +	.bypass_index	= 1, /* index of 24 MHz oscillator */
> +};
> +
>  static void __init sun8i_a23_ccu_setup(struct device_node *node)
>  {
>  	void __iomem *reg;
> @@ -732,6 +739,9 @@ static void __init sun8i_a23_ccu_setup(struct device_node *node)
>  	writel(val, reg + SUN8I_A23_PLL_MIPI_REG);
>  
>  	sunxi_ccu_probe(node, reg, &sun8i_a23_ccu_desc);
> +
> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> +				  &sun8i_a23_cpu_nb);
>  }
>  CLK_OF_DECLARE(sun8i_a23_ccu, "allwinner,sun8i-a23-ccu",
>  	       sun8i_a23_ccu_setup);
> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
> index 59cfdc8..3efbb6e 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
> @@ -752,6 +752,13 @@ static const struct sunxi_ccu_desc sun8i_a33_ccu_desc = {
>  	.num_resets	= ARRAY_SIZE(sun8i_a33_ccu_resets),
>  };
>  
> +static struct ccu_mux_nb sun8i_a33_cpu_nb = {
> +	.common		= &cpux_clk.common,
> +	.cm		= &cpux_clk.mux,
> +	.delay_us	= 1, /* > 8 clock cycles at 24 MHz */
> +	.bypass_index	= 1, /* index of 24 MHz oscillator */
> +};
> +
>  static void __init sun8i_a33_ccu_setup(struct device_node *node)
>  {
>  	void __iomem *reg;
> @@ -775,6 +782,9 @@ static void __init sun8i_a33_ccu_setup(struct device_node *node)
>  	writel(val, reg + SUN8I_A33_PLL_MIPI_REG);
>  
>  	sunxi_ccu_probe(node, reg, &sun8i_a33_ccu_desc);
> +
> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> +				  &sun8i_a33_cpu_nb);
>  }
>  CLK_OF_DECLARE(sun8i_a33_ccu, "allwinner,sun8i-a33-ccu",
>  	       sun8i_a33_ccu_setup);
> 

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

[PATCH 1/2] clk: sunxi-ng: Fix CPUX clock for the A23/A33

[Maxime Ripard]

Hi,

On Mon, Nov 07, 2016 at 01:29:31AM +0800, Icenowy Zheng wrote:
> The CPUX clock of A23/33 CCU driver forgot to add CLK_SET_RATE_PARENT
> flag, which makes its frequency fail to change (as part of cpu thermal
> throttle).
> 
> Fix it by add this flag.

Your commit title and log could be better.

The title usually is more about what is done in the patch itself,
something like "clk: sunxi-ng: Allow to change the parent rate for the
CPU clocks" in this case.

And your commit log should explain why it is an issue. Yours is even
wrong here, we could definitely change the rate of these clocks
already. The only thing that was not allowed was to change the rate of
its parents, which is what this patch fixes.

> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
> Patch 4.9-rc too.

I don't see why it should be part of 4.9. No one uses that code in 4.9.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161107/46d235a8/attachment.sig>

[PATCH 2/2] clk: sunxi-ng: fix up PLL_CPUX adjusting for A23/A33

[Maxime Ripard]

Hi,

Same remark for the commit title.

On Mon, Nov 07, 2016 at 01:29:32AM +0800, Icenowy Zheng wrote:
> When adjusting PLL_CPUX on A23/A33, the PLL is driven too high, and the
> system stucks.
> 
> Add a notifier to avoid this situation.
> 
> The code is ported from ccu-sun6i-a31.c.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
> Patch 4.9-rc too.
>  drivers/clk/sunxi-ng/ccu-sun8i-a23.c | 10 ++++++++++

Have you checked that it was also needed on A23? Not all SoCs are in
this situation.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161107/c6eb2990/attachment.sig>

[PATCH 1/2] clk: sunxi-ng: Fix CPUX clock for the A23/A33

[Maxime Ripard]

On Mon, Nov 07, 2016 at 05:36:03PM +0800, Icenowy Zheng wrote:
> > And your commit log should explain why it is an issue. Yours is even 
> > wrong here, we could definitely change the rate of these clocks 
> > already. The only thing that was not allowed was to change the rate of 
> > its parents, which is what this patch fixes. 
> >
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> 
> > > --- 
> > > Patch 4.9-rc too. 
> >
> > I don't see why it should be part of 4.9. No one uses that code in 4.9. 
> 
> It can be triggered by a modified dt in 4.9, by add operating points.

Still, there's no reason to fix something that is not used by anyone,
it can definitely wait until the next release. Linus also said that he
didn't want patches like that last week.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/fed056dc/attachment.sig>