[PATCH 0/3] arm64: allwinner: a64: Enable DVFS on A64

[PATCH 0/3] arm64: allwinner: a64: Enable DVFS on A64

[Vasily Khoruzhick]

This series enables DVFS on Allwinner A64. Operating points table is taken
from BSP kernel.

Icenowy Zheng (1):
  clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

Vasily Khoruzhick (2):
  clk: sunxi-ng: a64: export CLK_CPUX clock for DVFS
  arm64: dts: allwinner: a64: enable DVFS

 .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
 .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
 .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
 .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
 .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
 .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |  28 ++++-
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h         |   1 -
 include/dt-bindings/clock/sun50i-a64-ccu.h    |   1 +
 14 files changed, 170 insertions(+), 2 deletions(-)

-- 
2.24.1

[PATCH 0/3] arm64: allwinner: a64: Enable DVFS on A64

[Vasily Khoruzhick]

This series enables DVFS on Allwinner A64. Operating points table is taken
from BSP kernel.

Icenowy Zheng (1):
  clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

Vasily Khoruzhick (2):
  clk: sunxi-ng: a64: export CLK_CPUX clock for DVFS
  arm64: dts: allwinner: a64: enable DVFS

 .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
 .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
 .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
 .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
 .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
 .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |  28 ++++-
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h         |   1 -
 include/dt-bindings/clock/sun50i-a64-ccu.h    |   1 +
 14 files changed, 170 insertions(+), 2 deletions(-)

-- 
2.24.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Vasily Khoruzhick]

From: Icenowy Zheng <icenowy@aosc.io>

The A64 PLL_CPU clock has the same instability if some factor changed
without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
H3.

Add the mux and pll notifiers for A64 CPU clock to workaround the
problem.

Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 ++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 49bd7a4c015c..5f66bf879772 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -921,11 +921,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
 };
 
+static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
+	.common	= &pll_cpux_clk.common,
+	/* copy from pll_cpux_clk */
+	.enable	= BIT(31),
+	.lock	= BIT(28),
+};
+
+static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *reg;
 	u32 val;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg = devm_ioremap_resource(&pdev->dev, res);
@@ -939,7 +954,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
 
 	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
 
-	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	if (ret)
+		return ret;
+
+	/* Gate then ungate PLL CPU after any rate changes */
+	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
+
+	/* Reparent CPU during PLL CPU rate changes */
+	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+				  &sun50i_a64_cpu_nb);
+
+	return 0;
 }
 
 static const struct of_device_id sun50i_a64_ccu_ids[] = {
-- 
2.24.1

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Vasily Khoruzhick]

From: Icenowy Zheng <icenowy@aosc.io>

The A64 PLL_CPU clock has the same instability if some factor changed
without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
H3.

Add the mux and pll notifiers for A64 CPU clock to workaround the
problem.

Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 ++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 49bd7a4c015c..5f66bf879772 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -921,11 +921,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
 };
 
+static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
+	.common	= &pll_cpux_clk.common,
+	/* copy from pll_cpux_clk */
+	.enable	= BIT(31),
+	.lock	= BIT(28),
+};
+
+static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *reg;
 	u32 val;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg = devm_ioremap_resource(&pdev->dev, res);
@@ -939,7 +954,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
 
 	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
 
-	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	if (ret)
+		return ret;
+
+	/* Gate then ungate PLL CPU after any rate changes */
+	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
+
+	/* Reparent CPU during PLL CPU rate changes */
+	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+				  &sun50i_a64_cpu_nb);
+
+	return 0;
 }
 
 static const struct of_device_id sun50i_a64_ccu_ids[] = {
-- 
2.24.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Vasily Khoruzhick]

On Fri, Jan 3, 2020 at 10:35 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> From: Icenowy Zheng <icenowy@aosc.io>
>
> The A64 PLL_CPU clock has the same instability if some factor changed
> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> H3.
>
> Add the mux and pll notifiers for A64 CPU clock to workaround the
> problem.
>
> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>

Ugh, didn't notice that email is wrong here, this patch is not related
to my daytime job. Maxime, if patchset is OK please fix it up to be
"Vasily Khoruzhick <anarsoul@gmail.com>", otherwise I'll resend v2
with correct sign off.


> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 ++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> index 49bd7a4c015c..5f66bf879772 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -921,11 +921,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
>         .num_resets     = ARRAY_SIZE(sun50i_a64_ccu_resets),
>  };
>
> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> +       .common = &pll_cpux_clk.common,
> +       /* copy from pll_cpux_clk */
> +       .enable = BIT(31),
> +       .lock   = BIT(28),
> +};
> +
> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  {
>         struct resource *res;
>         void __iomem *reg;
>         u32 val;
> +       int ret;
>
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         reg = devm_ioremap_resource(&pdev->dev, res);
> @@ -939,7 +954,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
>
>         writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>
> -       return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +       ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +       if (ret)
> +               return ret;
> +
> +       /* Gate then ungate PLL CPU after any rate changes */
> +       ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> +
> +       /* Reparent CPU during PLL CPU rate changes */
> +       ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> +                                 &sun50i_a64_cpu_nb);
> +
> +       return 0;
>  }
>
>  static const struct of_device_id sun50i_a64_ccu_ids[] = {
> --
> 2.24.1
>

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Vasily Khoruzhick]

On Fri, Jan 3, 2020 at 10:35 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> From: Icenowy Zheng <icenowy@aosc.io>
>
> The A64 PLL_CPU clock has the same instability if some factor changed
> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> H3.
>
> Add the mux and pll notifiers for A64 CPU clock to workaround the
> problem.
>
> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>

Ugh, didn't notice that email is wrong here, this patch is not related
to my daytime job. Maxime, if patchset is OK please fix it up to be
"Vasily Khoruzhick <anarsoul@gmail.com>", otherwise I'll resend v2
with correct sign off.


> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 ++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> index 49bd7a4c015c..5f66bf879772 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -921,11 +921,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
>         .num_resets     = ARRAY_SIZE(sun50i_a64_ccu_resets),
>  };
>
> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> +       .common = &pll_cpux_clk.common,
> +       /* copy from pll_cpux_clk */
> +       .enable = BIT(31),
> +       .lock   = BIT(28),
> +};
> +
> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  {
>         struct resource *res;
>         void __iomem *reg;
>         u32 val;
> +       int ret;
>
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         reg = devm_ioremap_resource(&pdev->dev, res);
> @@ -939,7 +954,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
>
>         writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>
> -       return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +       ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +       if (ret)
> +               return ret;
> +
> +       /* Gate then ungate PLL CPU after any rate changes */
> +       ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> +
> +       /* Reparent CPU during PLL CPU rate changes */
> +       ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> +                                 &sun50i_a64_cpu_nb);
> +
> +       return 0;
>  }
>
>  static const struct of_device_id sun50i_a64_ccu_ids[] = {
> --
> 2.24.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

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

On Fri, Jan 03, 2020 at 10:42:35PM -0800, Vasily Khoruzhick wrote:
> On Fri, Jan 3, 2020 at 10:35 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >
> > From: Icenowy Zheng <icenowy@aosc.io>
> >
> > The A64 PLL_CPU clock has the same instability if some factor changed
> > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > H3.
> >
> > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > problem.
> >
> > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
>
> Ugh, didn't notice that email is wrong here, this patch is not related
> to my daytime job. Maxime, if patchset is OK please fix it up to be
> "Vasily Khoruzhick <anarsoul@gmail.com>", otherwise I'll resend v2
> with correct sign off.

Applied with your mail fixed, thanks!
Maxime

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

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 897 bytes --]

On Fri, Jan 03, 2020 at 10:42:35PM -0800, Vasily Khoruzhick wrote:
> On Fri, Jan 3, 2020 at 10:35 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >
> > From: Icenowy Zheng <icenowy@aosc.io>
> >
> > The A64 PLL_CPU clock has the same instability if some factor changed
> > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > H3.
> >
> > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > problem.
> >
> > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
>
> Ugh, didn't notice that email is wrong here, this patch is not related
> to my daytime job. Maxime, if patchset is OK please fix it up to be
> "Vasily Khoruzhick <anarsoul@gmail.com>", otherwise I'll resend v2
> with correct sign off.

Applied with your mail fixed, thanks!
Maxime

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] clk: sunxi-ng: a64: export CLK_CPUX clock for DVFS

[Vasily Khoruzhick]

Export CLK_CPUX so we can reference it in CPU node.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h      | 1 -
 include/dt-bindings/clock/sun50i-a64-ccu.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
index 979929276709..116e6f826d04 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
@@ -36,7 +36,6 @@
 #define CLK_PLL_HSIC			18
 #define CLK_PLL_DE			19
 #define CLK_PLL_DDR1			20
-#define CLK_CPUX			21
 #define CLK_AXI				22
 #define CLK_APB				23
 #define CLK_AHB1			24
diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
index a8ac4cfcdcbc..e512a1c9b0fc 100644
--- a/include/dt-bindings/clock/sun50i-a64-ccu.h
+++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
@@ -46,6 +46,7 @@
 #define CLK_PLL_VIDEO0		7
 #define CLK_PLL_PERIPH0		11
 
+#define CLK_CPUX		21
 #define CLK_BUS_MIPI_DSI	28
 #define CLK_BUS_CE		29
 #define CLK_BUS_DMA		30
-- 
2.24.1

[PATCH 2/3] clk: sunxi-ng: a64: export CLK_CPUX clock for DVFS

[Vasily Khoruzhick]

Export CLK_CPUX so we can reference it in CPU node.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h      | 1 -
 include/dt-bindings/clock/sun50i-a64-ccu.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
index 979929276709..116e6f826d04 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
@@ -36,7 +36,6 @@
 #define CLK_PLL_HSIC			18
 #define CLK_PLL_DE			19
 #define CLK_PLL_DDR1			20
-#define CLK_CPUX			21
 #define CLK_AXI				22
 #define CLK_APB				23
 #define CLK_AHB1			24
diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
index a8ac4cfcdcbc..e512a1c9b0fc 100644
--- a/include/dt-bindings/clock/sun50i-a64-ccu.h
+++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
@@ -46,6 +46,7 @@
 #define CLK_PLL_VIDEO0		7
 #define CLK_PLL_PERIPH0		11
 
+#define CLK_CPUX		21
 #define CLK_BUS_MIPI_DSI	28
 #define CLK_BUS_CE		29
 #define CLK_BUS_DMA		30
-- 
2.24.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] clk: sunxi-ng: a64: export CLK_CPUX clock for DVFS

[Maxime Ripard]

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

On Fri, Jan 03, 2020 at 10:35:04PM -0800, Vasily Khoruzhick wrote:
> Export CLK_CPUX so we can reference it in CPU node.
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Applied, thanks!
Maxime

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

Re: [PATCH 2/3] clk: sunxi-ng: a64: export CLK_CPUX clock for DVFS

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 204 bytes --]

On Fri, Jan 03, 2020 at 10:35:04PM -0800, Vasily Khoruzhick wrote:
> Export CLK_CPUX so we can reference it in CPU node.
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Applied, thanks!
Maxime

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Vasily Khoruzhick]

Add CPU regulator, CPU clock, operation points and thermal trip points
to enable DVFS on A64

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
 .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
 .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
 .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
 .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
 .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
 11 files changed, 142 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 5634245d11db..43f6ac0beed6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -65,6 +65,10 @@ wifi_pwrseq: wifi-pwrseq {
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &csi {
 	status = "okay";
 
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index 208373efee49..cbcab80de8c5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -108,6 +108,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
index 9b9d9157128c..6708acf94d01 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
@@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &de {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
index 01a9a52edae4..f94e60eb4ec2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
@@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &de {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
index f54a415f2e3b..2793afba4884 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
@@ -123,6 +123,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 409523cb0950..04aa452d4400 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -84,6 +84,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 78c82a665c84..365d85c16deb 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -98,6 +98,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index 920103ec0046..e760e8efdf3e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -100,6 +100,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
index 9d20e13f0c02..3d8e2d452ce1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
@@ -51,6 +51,10 @@ &codec_analog {
 	cpvdd-supply = <&reg_eldo1>;
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index 970415106dcf..28edcef8bed1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -104,6 +104,10 @@ &de {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &ehci1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 5e3f16c3b706..7f37c8cd1b6e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -80,6 +80,52 @@ simplefb_hdmi: framebuffer-hdmi {
 		};
 	};
 
+	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 */
+		};
+		opp-816000000 {
+			opp-hz = /bits/ 64 <816000000>;
+			opp-microvolt = <1100000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-912000000 {
+			opp-hz = /bits/ 64 <912000000>;
+			opp-microvolt = <1120000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-960000000 {
+			opp-hz = /bits/ 64 <960000000>;
+			opp-microvolt = <1160000>;
+			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-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <1240000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-1104000000 {
+			opp-hz = /bits/ 64 <1104000000>;
+			opp-microvolt = <1260000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-1152000000 {
+			opp-hz = /bits/ 64 <1152000000>;
+			opp-microvolt = <1300000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -90,6 +136,10 @@ cpu0: cpu@0 {
 			reg = <0>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu1: cpu@1 {
@@ -98,6 +148,10 @@ cpu1: cpu@1 {
 			reg = <1>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu2: cpu@2 {
@@ -106,6 +160,10 @@ cpu2: cpu@2 {
 			reg = <2>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu3: cpu@3 {
@@ -114,6 +172,10 @@ cpu3: cpu@3 {
 			reg = <3>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		L2: l2-cache {
@@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
 			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			thermal-sensors = <&ths 0>;
+
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert0>;
+					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+				map1 {
+					trip = <&cpu_alert1>;
+					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+
+			trips {
+				cpu_alert0: cpu_alert0 {
+					/* milliCelsius */
+					temperature = <75000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				cpu_alert1: cpu_alert1 {
+					/* milliCelsius */
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "hot";
+				};
+
+				cpu_crit: cpu_crit {
+					/* milliCelsius */
+					temperature = <110000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
 		};
 
 		gpu0_thermal: gpu0-thermal {
-- 
2.24.1

[PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Vasily Khoruzhick]

Add CPU regulator, CPU clock, operation points and thermal trip points
to enable DVFS on A64

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
 .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
 .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
 .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
 .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
 .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
 .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
 11 files changed, 142 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 5634245d11db..43f6ac0beed6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -65,6 +65,10 @@ wifi_pwrseq: wifi-pwrseq {
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &csi {
 	status = "okay";
 
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index 208373efee49..cbcab80de8c5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -108,6 +108,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
index 9b9d9157128c..6708acf94d01 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
@@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &de {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
index 01a9a52edae4..f94e60eb4ec2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
@@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &de {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
index f54a415f2e3b..2793afba4884 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
@@ -123,6 +123,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 409523cb0950..04aa452d4400 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -84,6 +84,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 78c82a665c84..365d85c16deb 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -98,6 +98,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index 920103ec0046..e760e8efdf3e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -100,6 +100,10 @@ &codec_analog {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &dai {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
index 9d20e13f0c02..3d8e2d452ce1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
@@ -51,6 +51,10 @@ &codec_analog {
 	cpvdd-supply = <&reg_eldo1>;
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index 970415106dcf..28edcef8bed1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -104,6 +104,10 @@ &de {
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &ehci1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 5e3f16c3b706..7f37c8cd1b6e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -80,6 +80,52 @@ simplefb_hdmi: framebuffer-hdmi {
 		};
 	};
 
+	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 */
+		};
+		opp-816000000 {
+			opp-hz = /bits/ 64 <816000000>;
+			opp-microvolt = <1100000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-912000000 {
+			opp-hz = /bits/ 64 <912000000>;
+			opp-microvolt = <1120000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-960000000 {
+			opp-hz = /bits/ 64 <960000000>;
+			opp-microvolt = <1160000>;
+			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-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <1240000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-1104000000 {
+			opp-hz = /bits/ 64 <1104000000>;
+			opp-microvolt = <1260000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+		opp-1152000000 {
+			opp-hz = /bits/ 64 <1152000000>;
+			opp-microvolt = <1300000>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -90,6 +136,10 @@ cpu0: cpu@0 {
 			reg = <0>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu1: cpu@1 {
@@ -98,6 +148,10 @@ cpu1: cpu@1 {
 			reg = <1>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu2: cpu@2 {
@@ -106,6 +160,10 @@ cpu2: cpu@2 {
 			reg = <2>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu3: cpu@3 {
@@ -114,6 +172,10 @@ cpu3: cpu@3 {
 			reg = <3>;
 			enable-method = "psci";
 			next-level-cache = <&L2>;
+			clocks = <&ccu CLK_CPUX>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		L2: l2-cache {
@@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
 			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			thermal-sensors = <&ths 0>;
+
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert0>;
+					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+				map1 {
+					trip = <&cpu_alert1>;
+					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+
+			trips {
+				cpu_alert0: cpu_alert0 {
+					/* milliCelsius */
+					temperature = <75000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				cpu_alert1: cpu_alert1 {
+					/* milliCelsius */
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "hot";
+				};
+
+				cpu_crit: cpu_crit {
+					/* milliCelsius */
+					temperature = <110000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
 		};
 
 		gpu0_thermal: gpu0-thermal {
-- 
2.24.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Maxime Ripard]

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

Hi!

On Fri, Jan 03, 2020 at 10:35:05PM -0800, Vasily Khoruzhick wrote:
> Add CPU regulator, CPU clock, operation points and thermal trip points
> to enable DVFS on A64
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
>  .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
>  .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
>  .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
>  .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
>  .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
>  .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
>  .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
>  .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
>  .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
>  11 files changed, 142 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> index 5634245d11db..43f6ac0beed6 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> @@ -65,6 +65,10 @@ wifi_pwrseq: wifi-pwrseq {
>  	};
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &csi {
>  	status = "okay";
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> index 208373efee49..cbcab80de8c5 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> @@ -108,6 +108,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> index 9b9d9157128c..6708acf94d01 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
>  	};
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &de {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> index 01a9a52edae4..f94e60eb4ec2 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
>  	};
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &de {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> index f54a415f2e3b..2793afba4884 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> @@ -123,6 +123,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> index 409523cb0950..04aa452d4400 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> @@ -84,6 +84,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> index 78c82a665c84..365d85c16deb 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> @@ -98,6 +98,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> index 920103ec0046..e760e8efdf3e 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> @@ -100,6 +100,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> index 9d20e13f0c02..3d8e2d452ce1 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> @@ -51,6 +51,10 @@ &codec_analog {
>  	cpvdd-supply = <&reg_eldo1>;
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &mmc0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&mmc0_pins>;
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> index 970415106dcf..28edcef8bed1 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> @@ -104,6 +104,10 @@ &de {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &ehci1 {
>  	status = "okay";
>  };

Did you test it on all those boards (and if so, how were the OPP
tested), or was it done simply by looking at the schematics?

> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 5e3f16c3b706..7f37c8cd1b6e 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -80,6 +80,52 @@ simplefb_hdmi: framebuffer-hdmi {
>  		};
>  	};
>
> +	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 */
> +		};
> +		opp-816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <1100000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};

Nit: having new lines between the nodes would be great

> +		opp-912000000 {
> +			opp-hz = /bits/ 64 <912000000>;
> +			opp-microvolt = <1120000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp-960000000 {
> +			opp-hz = /bits/ 64 <960000000>;
> +			opp-microvolt = <1160000>;
> +			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-1056000000 {
> +			opp-hz = /bits/ 64 <1056000000>;
> +			opp-microvolt = <1240000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <1260000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp-1152000000 {
> +			opp-hz = /bits/ 64 <1152000000>;
> +			opp-microvolt = <1300000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +	};
> +

What frequency is setup by U-Boot?

If it's 1008 MHz as usual, then having the OPP above that frequency by
default means that a board that doesn't set cpu-supply (or doesn't set
it up properly) will be unstable, since it will increase the
frequency, but not the voltage, and the voltage isn't enough for that
new frequency.

The way we've done it so far on the SoCs in a similar situation (A33,
A83t) is only to list the OPP below the frequency set up by U-Boot in
the DTSI, and list the frequencies above that in the boards that add
cpu-supply.

Given that it seems that we need to have them listed on number of
boards, I guess we could add a separate DTSI that would be included
only by the boards setting cpu-supply?

>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> @@ -90,6 +136,10 @@ cpu0: cpu@0 {
>  			reg = <0>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		cpu1: cpu@1 {
> @@ -98,6 +148,10 @@ cpu1: cpu@1 {
>  			reg = <1>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		cpu2: cpu@2 {
> @@ -106,6 +160,10 @@ cpu2: cpu@2 {
>  			reg = <2>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		cpu3: cpu@3 {
> @@ -114,6 +172,10 @@ cpu3: cpu@3 {
>  			reg = <3>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		L2: l2-cache {
> @@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
>  			polling-delay-passive = <0>;
>  			polling-delay = <0>;
>  			thermal-sensors = <&ths 0>;
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu_alert0>;
> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +				map1 {
> +					trip = <&cpu_alert1>;
> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
> +
> +			trips {
> +				cpu_alert0: cpu_alert0 {
> +					/* milliCelsius */
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_alert1: cpu_alert1 {
> +					/* milliCelsius */
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +
> +				cpu_crit: cpu_crit {
> +					/* milliCelsius */
> +					temperature = <110000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
>  		};

Where are those tripping points coming from?

Thanks!
Maxime

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

Re: [PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 10572 bytes --]

Hi!

On Fri, Jan 03, 2020 at 10:35:05PM -0800, Vasily Khoruzhick wrote:
> Add CPU regulator, CPU clock, operation points and thermal trip points
> to enable DVFS on A64
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
>  .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
>  .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
>  .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
>  .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
>  .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
>  .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
>  .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
>  .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
>  .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
>  11 files changed, 142 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> index 5634245d11db..43f6ac0beed6 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> @@ -65,6 +65,10 @@ wifi_pwrseq: wifi-pwrseq {
>  	};
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &csi {
>  	status = "okay";
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> index 208373efee49..cbcab80de8c5 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> @@ -108,6 +108,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> index 9b9d9157128c..6708acf94d01 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
>  	};
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &de {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> index 01a9a52edae4..f94e60eb4ec2 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
>  	};
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &de {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> index f54a415f2e3b..2793afba4884 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> @@ -123,6 +123,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> index 409523cb0950..04aa452d4400 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> @@ -84,6 +84,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> index 78c82a665c84..365d85c16deb 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> @@ -98,6 +98,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> index 920103ec0046..e760e8efdf3e 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> @@ -100,6 +100,10 @@ &codec_analog {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &dai {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> index 9d20e13f0c02..3d8e2d452ce1 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> @@ -51,6 +51,10 @@ &codec_analog {
>  	cpvdd-supply = <&reg_eldo1>;
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &mmc0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&mmc0_pins>;
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> index 970415106dcf..28edcef8bed1 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> @@ -104,6 +104,10 @@ &de {
>  	status = "okay";
>  };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +};
> +
>  &ehci1 {
>  	status = "okay";
>  };

Did you test it on all those boards (and if so, how were the OPP
tested), or was it done simply by looking at the schematics?

> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 5e3f16c3b706..7f37c8cd1b6e 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -80,6 +80,52 @@ simplefb_hdmi: framebuffer-hdmi {
>  		};
>  	};
>
> +	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 */
> +		};
> +		opp-816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <1100000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};

Nit: having new lines between the nodes would be great

> +		opp-912000000 {
> +			opp-hz = /bits/ 64 <912000000>;
> +			opp-microvolt = <1120000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp-960000000 {
> +			opp-hz = /bits/ 64 <960000000>;
> +			opp-microvolt = <1160000>;
> +			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-1056000000 {
> +			opp-hz = /bits/ 64 <1056000000>;
> +			opp-microvolt = <1240000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <1260000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +		opp-1152000000 {
> +			opp-hz = /bits/ 64 <1152000000>;
> +			opp-microvolt = <1300000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +	};
> +

What frequency is setup by U-Boot?

If it's 1008 MHz as usual, then having the OPP above that frequency by
default means that a board that doesn't set cpu-supply (or doesn't set
it up properly) will be unstable, since it will increase the
frequency, but not the voltage, and the voltage isn't enough for that
new frequency.

The way we've done it so far on the SoCs in a similar situation (A33,
A83t) is only to list the OPP below the frequency set up by U-Boot in
the DTSI, and list the frequencies above that in the boards that add
cpu-supply.

Given that it seems that we need to have them listed on number of
boards, I guess we could add a separate DTSI that would be included
only by the boards setting cpu-supply?

>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> @@ -90,6 +136,10 @@ cpu0: cpu@0 {
>  			reg = <0>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		cpu1: cpu@1 {
> @@ -98,6 +148,10 @@ cpu1: cpu@1 {
>  			reg = <1>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		cpu2: cpu@2 {
> @@ -106,6 +160,10 @@ cpu2: cpu@2 {
>  			reg = <2>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		cpu3: cpu@3 {
> @@ -114,6 +172,10 @@ cpu3: cpu@3 {
>  			reg = <3>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2>;
> +			clocks = <&ccu CLK_CPUX>;
> +			clock-names = "cpu";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
>  		};
>
>  		L2: l2-cache {
> @@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
>  			polling-delay-passive = <0>;
>  			polling-delay = <0>;
>  			thermal-sensors = <&ths 0>;
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu_alert0>;
> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +				map1 {
> +					trip = <&cpu_alert1>;
> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
> +
> +			trips {
> +				cpu_alert0: cpu_alert0 {
> +					/* milliCelsius */
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_alert1: cpu_alert1 {
> +					/* milliCelsius */
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +
> +				cpu_crit: cpu_crit {
> +					/* milliCelsius */
> +					temperature = <110000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
>  		};

Where are those tripping points coming from?

Thanks!
Maxime

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Vasily Khoruzhick]

On Sat, Jan 4, 2020 at 12:37 AM Maxime Ripard <mripard@kernel.org> wrote:
>
> Hi!
>
> On Fri, Jan 03, 2020 at 10:35:05PM -0800, Vasily Khoruzhick wrote:
> > Add CPU regulator, CPU clock, operation points and thermal trip points
> > to enable DVFS on A64
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
> >  .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
> >  .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
> >  .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
> >  .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
> >  .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
> >  .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
> >  .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
> >  .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
> >  .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
> >  11 files changed, 142 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > index 5634245d11db..43f6ac0beed6 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > @@ -65,6 +65,10 @@ wifi_pwrseq: wifi-pwrseq {
> >       };
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &csi {
> >       status = "okay";
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> > index 208373efee49..cbcab80de8c5 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> > @@ -108,6 +108,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> > index 9b9d9157128c..6708acf94d01 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> > @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
> >       };
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &de {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> > index 01a9a52edae4..f94e60eb4ec2 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> > @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
> >       };
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &de {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> > index f54a415f2e3b..2793afba4884 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> > @@ -123,6 +123,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > index 409523cb0950..04aa452d4400 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > @@ -84,6 +84,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > index 78c82a665c84..365d85c16deb 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > @@ -98,6 +98,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> > index 920103ec0046..e760e8efdf3e 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> > @@ -100,6 +100,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> > index 9d20e13f0c02..3d8e2d452ce1 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> > @@ -51,6 +51,10 @@ &codec_analog {
> >       cpvdd-supply = <&reg_eldo1>;
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &mmc0 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&mmc0_pins>;
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > index 970415106dcf..28edcef8bed1 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > @@ -104,6 +104,10 @@ &de {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &ehci1 {
> >       status = "okay";
> >  };
>
> Did you test it on all those boards (and if so, how were the OPP
> tested), or was it done simply by looking at the schematics?

I tested it only on Pine64-LTS and Pinebook. But all these boards use
AXP803 which always uses DCDC2 as CPU regulator.

> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index 5e3f16c3b706..7f37c8cd1b6e 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -80,6 +80,52 @@ simplefb_hdmi: framebuffer-hdmi {
> >               };
> >       };
> >
> > +     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 */
> > +             };
> > +             opp-816000000 {
> > +                     opp-hz = /bits/ 64 <816000000>;
> > +                     opp-microvolt = <1100000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
>
> Nit: having new lines between the nodes would be great

Will do.

> > +             opp-912000000 {
> > +                     opp-hz = /bits/ 64 <912000000>;
> > +                     opp-microvolt = <1120000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +             opp-960000000 {
> > +                     opp-hz = /bits/ 64 <960000000>;
> > +                     opp-microvolt = <1160000>;
> > +                     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-1056000000 {
> > +                     opp-hz = /bits/ 64 <1056000000>;
> > +                     opp-microvolt = <1240000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +             opp-1104000000 {
> > +                     opp-hz = /bits/ 64 <1104000000>;
> > +                     opp-microvolt = <1260000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +             opp-1152000000 {
> > +                     opp-hz = /bits/ 64 <1152000000>;
> > +                     opp-microvolt = <1300000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +     };
> > +
>
> What frequency is setup by U-Boot?

It's 816 MHz

> If it's 1008 MHz as usual, then having the OPP above that frequency by
> default means that a board that doesn't set cpu-supply (or doesn't set
> it up properly) will be unstable, since it will increase the
> frequency, but not the voltage, and the voltage isn't enough for that
> new frequency.
>
> The way we've done it so far on the SoCs in a similar situation (A33,
> A83t) is only to list the OPP below the frequency set up by U-Boot in
> the DTSI, and list the frequencies above that in the boards that add
> cpu-supply.
>
> Given that it seems that we need to have them listed on number of
> boards, I guess we could add a separate DTSI that would be included
> only by the boards setting cpu-supply?

Currently all the A64 boards that we support use AXP803, so I'm not
sure whether we actually need that. We can just define CPU regulator
for them and use default OPP and that's exactly what I did.

> >       cpus {
> >               #address-cells = <1>;
> >               #size-cells = <0>;
> > @@ -90,6 +136,10 @@ cpu0: cpu@0 {
> >                       reg = <0>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu1: cpu@1 {
> > @@ -98,6 +148,10 @@ cpu1: cpu@1 {
> >                       reg = <1>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu2: cpu@2 {
> > @@ -106,6 +160,10 @@ cpu2: cpu@2 {
> >                       reg = <2>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu3: cpu@3 {
> > @@ -114,6 +172,10 @@ cpu3: cpu@3 {
> >                       reg = <3>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               L2: l2-cache {
> > @@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
> >                       polling-delay-passive = <0>;
> >                       polling-delay = <0>;
> >                       thermal-sensors = <&ths 0>;
> > +
> > +                     cooling-maps {
> > +                             map0 {
> > +                                     trip = <&cpu_alert0>;
> > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };
> > +                             map1 {
> > +                                     trip = <&cpu_alert1>;
> > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };
> > +                     };
> > +
> > +                     trips {
> > +                             cpu_alert0: cpu_alert0 {
> > +                                     /* milliCelsius */
> > +                                     temperature = <75000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "passive";
> > +                             };
> > +
> > +                             cpu_alert1: cpu_alert1 {
> > +                                     /* milliCelsius */
> > +                                     temperature = <90000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "hot";
> > +                             };
> > +
> > +                             cpu_crit: cpu_crit {
> > +                                     /* milliCelsius */
> > +                                     temperature = <110000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "critical";
> > +                             };
> > +                     };
> >               };
>
> Where are those tripping points coming from?

These are taken from A33 dtsi. I couldn't find any recommended
throttling and critical temp in A64 user manual [1].

[1] http://linux-sunxi.org/images/b/b4/Allwinner_A64_User_Manual_V1.1.pdf

> Thanks!
> Maxime

Re: [PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Vasily Khoruzhick]

On Sat, Jan 4, 2020 at 12:37 AM Maxime Ripard <mripard@kernel.org> wrote:
>
> Hi!
>
> On Fri, Jan 03, 2020 at 10:35:05PM -0800, Vasily Khoruzhick wrote:
> > Add CPU regulator, CPU clock, operation points and thermal trip points
> > to enable DVFS on A64
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  .../allwinner/sun50i-a64-amarula-relic.dts    |   4 +
> >  .../dts/allwinner/sun50i-a64-bananapi-m64.dts |   4 +
> >  .../dts/allwinner/sun50i-a64-nanopi-a64.dts   |   4 +
> >  .../dts/allwinner/sun50i-a64-olinuxino.dts    |   4 +
> >  .../dts/allwinner/sun50i-a64-orangepi-win.dts |   4 +
> >  .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   4 +
> >  .../dts/allwinner/sun50i-a64-pinebook.dts     |   4 +
> >  .../allwinner/sun50i-a64-sopine-baseboard.dts |   4 +
> >  .../boot/dts/allwinner/sun50i-a64-sopine.dtsi |   4 +
> >  .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   4 +
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 102 ++++++++++++++++++
> >  11 files changed, 142 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > index 5634245d11db..43f6ac0beed6 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > @@ -65,6 +65,10 @@ wifi_pwrseq: wifi-pwrseq {
> >       };
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &csi {
> >       status = "okay";
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> > index 208373efee49..cbcab80de8c5 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
> > @@ -108,6 +108,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> > index 9b9d9157128c..6708acf94d01 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dts
> > @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
> >       };
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &de {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> > index 01a9a52edae4..f94e60eb4ec2 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> > @@ -87,6 +87,10 @@ wifi_pwrseq: wifi_pwrseq {
> >       };
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &de {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> > index f54a415f2e3b..2793afba4884 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dts
> > @@ -123,6 +123,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > index 409523cb0950..04aa452d4400 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> > @@ -84,6 +84,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > index 78c82a665c84..365d85c16deb 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > @@ -98,6 +98,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> > index 920103ec0046..e760e8efdf3e 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> > @@ -100,6 +100,10 @@ &codec_analog {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &dai {
> >       status = "okay";
> >  };
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> > index 9d20e13f0c02..3d8e2d452ce1 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
> > @@ -51,6 +51,10 @@ &codec_analog {
> >       cpvdd-supply = <&reg_eldo1>;
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &mmc0 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&mmc0_pins>;
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > index 970415106dcf..28edcef8bed1 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > @@ -104,6 +104,10 @@ &de {
> >       status = "okay";
> >  };
> >
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> >  &ehci1 {
> >       status = "okay";
> >  };
>
> Did you test it on all those boards (and if so, how were the OPP
> tested), or was it done simply by looking at the schematics?

I tested it only on Pine64-LTS and Pinebook. But all these boards use
AXP803 which always uses DCDC2 as CPU regulator.

> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index 5e3f16c3b706..7f37c8cd1b6e 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -80,6 +80,52 @@ simplefb_hdmi: framebuffer-hdmi {
> >               };
> >       };
> >
> > +     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 */
> > +             };
> > +             opp-816000000 {
> > +                     opp-hz = /bits/ 64 <816000000>;
> > +                     opp-microvolt = <1100000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
>
> Nit: having new lines between the nodes would be great

Will do.

> > +             opp-912000000 {
> > +                     opp-hz = /bits/ 64 <912000000>;
> > +                     opp-microvolt = <1120000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +             opp-960000000 {
> > +                     opp-hz = /bits/ 64 <960000000>;
> > +                     opp-microvolt = <1160000>;
> > +                     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-1056000000 {
> > +                     opp-hz = /bits/ 64 <1056000000>;
> > +                     opp-microvolt = <1240000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +             opp-1104000000 {
> > +                     opp-hz = /bits/ 64 <1104000000>;
> > +                     opp-microvolt = <1260000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +             opp-1152000000 {
> > +                     opp-hz = /bits/ 64 <1152000000>;
> > +                     opp-microvolt = <1300000>;
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +             };
> > +     };
> > +
>
> What frequency is setup by U-Boot?

It's 816 MHz

> If it's 1008 MHz as usual, then having the OPP above that frequency by
> default means that a board that doesn't set cpu-supply (or doesn't set
> it up properly) will be unstable, since it will increase the
> frequency, but not the voltage, and the voltage isn't enough for that
> new frequency.
>
> The way we've done it so far on the SoCs in a similar situation (A33,
> A83t) is only to list the OPP below the frequency set up by U-Boot in
> the DTSI, and list the frequencies above that in the boards that add
> cpu-supply.
>
> Given that it seems that we need to have them listed on number of
> boards, I guess we could add a separate DTSI that would be included
> only by the boards setting cpu-supply?

Currently all the A64 boards that we support use AXP803, so I'm not
sure whether we actually need that. We can just define CPU regulator
for them and use default OPP and that's exactly what I did.

> >       cpus {
> >               #address-cells = <1>;
> >               #size-cells = <0>;
> > @@ -90,6 +136,10 @@ cpu0: cpu@0 {
> >                       reg = <0>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu1: cpu@1 {
> > @@ -98,6 +148,10 @@ cpu1: cpu@1 {
> >                       reg = <1>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu2: cpu@2 {
> > @@ -106,6 +160,10 @@ cpu2: cpu@2 {
> >                       reg = <2>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               cpu3: cpu@3 {
> > @@ -114,6 +172,10 @@ cpu3: cpu@3 {
> >                       reg = <3>;
> >                       enable-method = "psci";
> >                       next-level-cache = <&L2>;
> > +                     clocks = <&ccu CLK_CPUX>;
> > +                     clock-names = "cpu";
> > +                     operating-points-v2 = <&cpu0_opp_table>;
> > +                     #cooling-cells = <2>;
> >               };
> >
> >               L2: l2-cache {
> > @@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
> >                       polling-delay-passive = <0>;
> >                       polling-delay = <0>;
> >                       thermal-sensors = <&ths 0>;
> > +
> > +                     cooling-maps {
> > +                             map0 {
> > +                                     trip = <&cpu_alert0>;
> > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };
> > +                             map1 {
> > +                                     trip = <&cpu_alert1>;
> > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };
> > +                     };
> > +
> > +                     trips {
> > +                             cpu_alert0: cpu_alert0 {
> > +                                     /* milliCelsius */
> > +                                     temperature = <75000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "passive";
> > +                             };
> > +
> > +                             cpu_alert1: cpu_alert1 {
> > +                                     /* milliCelsius */
> > +                                     temperature = <90000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "hot";
> > +                             };
> > +
> > +                             cpu_crit: cpu_crit {
> > +                                     /* milliCelsius */
> > +                                     temperature = <110000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "critical";
> > +                             };
> > +                     };
> >               };
>
> Where are those tripping points coming from?

These are taken from A33 dtsi. I couldn't find any recommended
throttling and critical temp in A64 user manual [1].

[1] http://linux-sunxi.org/images/b/b4/Allwinner_A64_User_Manual_V1.1.pdf

> Thanks!
> Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Maxime Ripard]

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

Hi,

On Sat, Jan 04, 2020 at 08:24:01AM -0800, Vasily Khoruzhick wrote:
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > @@ -104,6 +104,10 @@ &de {
> > >       status = "okay";
> > >  };
> > >
> > > +&cpu0 {
> > > +     cpu-supply = <&reg_dcdc2>;
> > > +};
> > > +
> > >  &ehci1 {
> > >       status = "okay";
> > >  };
> >
> > Did you test it on all those boards (and if so, how were the OPP
> > tested), or was it done simply by looking at the schematics?
>
> I tested it only on Pine64-LTS and Pinebook. But all these boards use
> AXP803 which always uses DCDC2 as CPU regulator.

Ok. How were those OPPs picked and tested?

> > > +             opp-912000000 {
> > > +                     opp-hz = /bits/ 64 <912000000>;
> > > +                     opp-microvolt = <1120000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +             opp-960000000 {
> > > +                     opp-hz = /bits/ 64 <960000000>;
> > > +                     opp-microvolt = <1160000>;
> > > +                     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-1056000000 {
> > > +                     opp-hz = /bits/ 64 <1056000000>;
> > > +                     opp-microvolt = <1240000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +             opp-1104000000 {
> > > +                     opp-hz = /bits/ 64 <1104000000>;
> > > +                     opp-microvolt = <1260000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +             opp-1152000000 {
> > > +                     opp-hz = /bits/ 64 <1152000000>;
> > > +                     opp-microvolt = <1300000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +     };
> > > +
> >
> > What frequency is setup by U-Boot?
>
> It's 816 MHz
>
> > If it's 1008 MHz as usual, then having the OPP above that frequency by
> > default means that a board that doesn't set cpu-supply (or doesn't set
> > it up properly) will be unstable, since it will increase the
> > frequency, but not the voltage, and the voltage isn't enough for that
> > new frequency.
> >
> > The way we've done it so far on the SoCs in a similar situation (A33,
> > A83t) is only to list the OPP below the frequency set up by U-Boot in
> > the DTSI, and list the frequencies above that in the boards that add
> > cpu-supply.
> >
> > Given that it seems that we need to have them listed on number of
> > boards, I guess we could add a separate DTSI that would be included
> > only by the boards setting cpu-supply?
>
> Currently all the A64 boards that we support use AXP803, so I'm not
> sure whether we actually need that. We can just define CPU regulator
> for them and use default OPP and that's exactly what I did.

Yeah, but none of them have been supporting it since day one. Having
these OPPs now mean that the very first thing you'll need to do is to
bringup the PMIC as well, otherwise the system will crash.

And to the person doing the bringup, this will be far from obvious
that a) we have that requirement b) the crash that they are seeing is
due to cpufreq.

> > >       cpus {
> > >               #address-cells = <1>;
> > >               #size-cells = <0>;
> > > @@ -90,6 +136,10 @@ cpu0: cpu@0 {
> > >                       reg = <0>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu1: cpu@1 {
> > > @@ -98,6 +148,10 @@ cpu1: cpu@1 {
> > >                       reg = <1>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu2: cpu@2 {
> > > @@ -106,6 +160,10 @@ cpu2: cpu@2 {
> > >                       reg = <2>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu3: cpu@3 {
> > > @@ -114,6 +172,10 @@ cpu3: cpu@3 {
> > >                       reg = <3>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               L2: l2-cache {
> > > @@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
> > >                       polling-delay-passive = <0>;
> > >                       polling-delay = <0>;
> > >                       thermal-sensors = <&ths 0>;
> > > +
> > > +                     cooling-maps {
> > > +                             map0 {
> > > +                                     trip = <&cpu_alert0>;
> > > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > +                             };
> > > +                             map1 {
> > > +                                     trip = <&cpu_alert1>;
> > > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     trips {
> > > +                             cpu_alert0: cpu_alert0 {
> > > +                                     /* milliCelsius */
> > > +                                     temperature = <75000>;
> > > +                                     hysteresis = <2000>;
> > > +                                     type = "passive";
> > > +                             };
> > > +
> > > +                             cpu_alert1: cpu_alert1 {
> > > +                                     /* milliCelsius */
> > > +                                     temperature = <90000>;
> > > +                                     hysteresis = <2000>;
> > > +                                     type = "hot";
> > > +                             };
> > > +
> > > +                             cpu_crit: cpu_crit {
> > > +                                     /* milliCelsius */
> > > +                                     temperature = <110000>;
> > > +                                     hysteresis = <2000>;
> > > +                                     type = "critical";
> > > +                             };
> > > +                     };
> > >               };
> >
> > Where are those tripping points coming from?
>
> These are taken from A33 dtsi. I couldn't find any recommended
> throttling and critical temp in A64 user manual [1].
>
> [1] http://linux-sunxi.org/images/b/b4/Allwinner_A64_User_Manual_V1.1.pdf

Ok. Can you mention that in the commit log?

Thanks!
Maxime

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

Re: [PATCH 3/3] arm64: dts: allwinner: a64: enable DVFS

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 8514 bytes --]

Hi,

On Sat, Jan 04, 2020 at 08:24:01AM -0800, Vasily Khoruzhick wrote:
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > @@ -104,6 +104,10 @@ &de {
> > >       status = "okay";
> > >  };
> > >
> > > +&cpu0 {
> > > +     cpu-supply = <&reg_dcdc2>;
> > > +};
> > > +
> > >  &ehci1 {
> > >       status = "okay";
> > >  };
> >
> > Did you test it on all those boards (and if so, how were the OPP
> > tested), or was it done simply by looking at the schematics?
>
> I tested it only on Pine64-LTS and Pinebook. But all these boards use
> AXP803 which always uses DCDC2 as CPU regulator.

Ok. How were those OPPs picked and tested?

> > > +             opp-912000000 {
> > > +                     opp-hz = /bits/ 64 <912000000>;
> > > +                     opp-microvolt = <1120000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +             opp-960000000 {
> > > +                     opp-hz = /bits/ 64 <960000000>;
> > > +                     opp-microvolt = <1160000>;
> > > +                     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-1056000000 {
> > > +                     opp-hz = /bits/ 64 <1056000000>;
> > > +                     opp-microvolt = <1240000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +             opp-1104000000 {
> > > +                     opp-hz = /bits/ 64 <1104000000>;
> > > +                     opp-microvolt = <1260000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +             opp-1152000000 {
> > > +                     opp-hz = /bits/ 64 <1152000000>;
> > > +                     opp-microvolt = <1300000>;
> > > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > > +             };
> > > +     };
> > > +
> >
> > What frequency is setup by U-Boot?
>
> It's 816 MHz
>
> > If it's 1008 MHz as usual, then having the OPP above that frequency by
> > default means that a board that doesn't set cpu-supply (or doesn't set
> > it up properly) will be unstable, since it will increase the
> > frequency, but not the voltage, and the voltage isn't enough for that
> > new frequency.
> >
> > The way we've done it so far on the SoCs in a similar situation (A33,
> > A83t) is only to list the OPP below the frequency set up by U-Boot in
> > the DTSI, and list the frequencies above that in the boards that add
> > cpu-supply.
> >
> > Given that it seems that we need to have them listed on number of
> > boards, I guess we could add a separate DTSI that would be included
> > only by the boards setting cpu-supply?
>
> Currently all the A64 boards that we support use AXP803, so I'm not
> sure whether we actually need that. We can just define CPU regulator
> for them and use default OPP and that's exactly what I did.

Yeah, but none of them have been supporting it since day one. Having
these OPPs now mean that the very first thing you'll need to do is to
bringup the PMIC as well, otherwise the system will crash.

And to the person doing the bringup, this will be far from obvious
that a) we have that requirement b) the crash that they are seeing is
due to cpufreq.

> > >       cpus {
> > >               #address-cells = <1>;
> > >               #size-cells = <0>;
> > > @@ -90,6 +136,10 @@ cpu0: cpu@0 {
> > >                       reg = <0>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu1: cpu@1 {
> > > @@ -98,6 +148,10 @@ cpu1: cpu@1 {
> > >                       reg = <1>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu2: cpu@2 {
> > > @@ -106,6 +160,10 @@ cpu2: cpu@2 {
> > >                       reg = <2>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               cpu3: cpu@3 {
> > > @@ -114,6 +172,10 @@ cpu3: cpu@3 {
> > >                       reg = <3>;
> > >                       enable-method = "psci";
> > >                       next-level-cache = <&L2>;
> > > +                     clocks = <&ccu CLK_CPUX>;
> > > +                     clock-names = "cpu";
> > > +                     operating-points-v2 = <&cpu0_opp_table>;
> > > +                     #cooling-cells = <2>;
> > >               };
> > >
> > >               L2: l2-cache {
> > > @@ -218,6 +280,46 @@ cpu_thermal: cpu0-thermal {
> > >                       polling-delay-passive = <0>;
> > >                       polling-delay = <0>;
> > >                       thermal-sensors = <&ths 0>;
> > > +
> > > +                     cooling-maps {
> > > +                             map0 {
> > > +                                     trip = <&cpu_alert0>;
> > > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > +                             };
> > > +                             map1 {
> > > +                                     trip = <&cpu_alert1>;
> > > +                                     cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                                      <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     trips {
> > > +                             cpu_alert0: cpu_alert0 {
> > > +                                     /* milliCelsius */
> > > +                                     temperature = <75000>;
> > > +                                     hysteresis = <2000>;
> > > +                                     type = "passive";
> > > +                             };
> > > +
> > > +                             cpu_alert1: cpu_alert1 {
> > > +                                     /* milliCelsius */
> > > +                                     temperature = <90000>;
> > > +                                     hysteresis = <2000>;
> > > +                                     type = "hot";
> > > +                             };
> > > +
> > > +                             cpu_crit: cpu_crit {
> > > +                                     /* milliCelsius */
> > > +                                     temperature = <110000>;
> > > +                                     hysteresis = <2000>;
> > > +                                     type = "critical";
> > > +                             };
> > > +                     };
> > >               };
> >
> > Where are those tripping points coming from?
>
> These are taken from A33 dtsi. I couldn't find any recommended
> throttling and critical temp in A64 user manual [1].
>
> [1] http://linux-sunxi.org/images/b/b4/Allwinner_A64_User_Manual_V1.1.pdf

Ok. Can you mention that in the commit log?

Thanks!
Maxime

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/3] Simple DVFS support for Allwinner A64 SoC

[Icenowy Zheng]

This patchset imports simple DVFS support for Allwinner A64 SoC.

As the thermal sensor driver is not yet implemented and some boards
have still no AXP PMIC support, now only two OPPs are present --
648MHz@1.04V and 816MHz@1.1V to prevent overheat or undervoltage.

PATCH 1 is a fix to the CCU driver of A64, and the remaining patches
set up the device tree bits of the DVFS on Pine64.

Icenowy Zheng (3):
  clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock
  arm64: allwinner: a64: add CPU opp table
  arm64: allwinner: a64: set CPU regulator for Pine64

 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |  4 ++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      | 24 +++++++++++++++++++
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c              | 28 +++++++++++++++++++++-
 3 files changed, 55 insertions(+), 1 deletion(-)

-- 
2.13.5

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Icenowy Zheng]

The A64 PLL_CPU clock has the same instability if some factor changed
without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
H3.

Add the mux and pll notifiers for A64 CPU clock to workaround the
problem.

Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 2bb4cabf802f..b55fa69dd0c1 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
 };
 
+static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
+	.common	= &pll_cpux_clk.common,
+	/* copy from pll_cpux_clk */
+	.enable	= BIT(31),
+	.lock	= BIT(28),
+};
+
+static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *reg;
 	u32 val;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg = devm_ioremap_resource(&pdev->dev, res);
@@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
 
 	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
 
-	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	if (ret)
+		return ret;
+
+	/* Gate then ungate PLL CPU after any rate changes */
+	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
+
+	/* Reparent CPU during PLL CPU rate changes */
+	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+				  &sun50i_a64_cpu_nb);
+
+	return 0;
 }
 
 static const struct of_device_id sun50i_a64_ccu_ids[] = {
-- 
2.13.5

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Icenowy Zheng]

The A64 PLL_CPU clock has the same instability if some factor changed
without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
H3.

Add the mux and pll notifiers for A64 CPU clock to workaround the
problem.

Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 2bb4cabf802f..b55fa69dd0c1 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
 };
 
+static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
+	.common	= &pll_cpux_clk.common,
+	/* copy from pll_cpux_clk */
+	.enable	= BIT(31),
+	.lock	= BIT(28),
+};
+
+static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *reg;
 	u32 val;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg = devm_ioremap_resource(&pdev->dev, res);
@@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
 
 	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
 
-	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	if (ret)
+		return ret;
+
+	/* Gate then ungate PLL CPU after any rate changes */
+	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
+
+	/* Reparent CPU during PLL CPU rate changes */
+	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+				  &sun50i_a64_cpu_nb);
+
+	return 0;
 }
 
 static const struct of_device_id sun50i_a64_ccu_ids[] = {
-- 
2.13.5

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Icenowy Zheng]

The A64 PLL_CPU clock has the same instability if some factor changed
without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
H3.

Add the mux and pll notifiers for A64 CPU clock to workaround the
problem.

Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 2bb4cabf802f..b55fa69dd0c1 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
 };
 
+static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
+	.common	= &pll_cpux_clk.common,
+	/* copy from pll_cpux_clk */
+	.enable	= BIT(31),
+	.lock	= BIT(28),
+};
+
+static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *reg;
 	u32 val;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg = devm_ioremap_resource(&pdev->dev, res);
@@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
 
 	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
 
-	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
+	if (ret)
+		return ret;
+
+	/* Gate then ungate PLL CPU after any rate changes */
+	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
+
+	/* Reparent CPU during PLL CPU rate changes */
+	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+				  &sun50i_a64_cpu_nb);
+
+	return 0;
 }
 
 static const struct of_device_id sun50i_a64_ccu_ids[] = {
-- 
2.13.5

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

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

Hi,

On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> The A64 PLL_CPU clock has the same instability if some factor changed
> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> H3.
> 
> Add the mux and pll notifiers for A64 CPU clock to workaround the
> problem.
> 
> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> index 2bb4cabf802f..b55fa69dd0c1 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
>  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>  };
>  
> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> +	.common	= &pll_cpux_clk.common,
> +	/* copy from pll_cpux_clk */
> +	.enable	= BIT(31),
> +	.lock	= BIT(28),
> +};
> +
> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	void __iomem *reg;
>  	u32 val;
> +	int ret;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	reg = devm_ioremap_resource(&pdev->dev, res);
> @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  
>  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>  
> -	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +	if (ret)
> +		return ret;
> +
> +	/* Gate then ungate PLL CPU after any rate changes */
> +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> +
> +	/* Reparent CPU during PLL CPU rate changes */
> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> +				  &sun50i_a64_cpu_nb);
> +
> +	return 0;

So this is the fourth user of the exact same code, can you turn that
into a shared function?

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 --]

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

Hi,

On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> The A64 PLL_CPU clock has the same instability if some factor changed
> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> H3.
> 
> Add the mux and pll notifiers for A64 CPU clock to workaround the
> problem.
> 
> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> index 2bb4cabf802f..b55fa69dd0c1 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
>  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>  };
>  
> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> +	.common	= &pll_cpux_clk.common,
> +	/* copy from pll_cpux_clk */
> +	.enable	= BIT(31),
> +	.lock	= BIT(28),
> +};
> +
> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	void __iomem *reg;
>  	u32 val;
> +	int ret;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	reg = devm_ioremap_resource(&pdev->dev, res);
> @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  
>  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>  
> -	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +	if (ret)
> +		return ret;
> +
> +	/* Gate then ungate PLL CPU after any rate changes */
> +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> +
> +	/* Reparent CPU during PLL CPU rate changes */
> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> +				  &sun50i_a64_cpu_nb);
> +
> +	return 0;

So this is the fourth user of the exact same code, can you turn that
into a shared function?

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/20170928/37210ae1/attachment-0001.sig>

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

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

Hi,

On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> The A64 PLL_CPU clock has the same instability if some factor changed
> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> H3.
> 
> Add the mux and pll notifiers for A64 CPU clock to workaround the
> problem.
> 
> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> index 2bb4cabf802f..b55fa69dd0c1 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc sun50i_a64_ccu_desc = {
>  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>  };
>  
> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> +	.common	= &pll_cpux_clk.common,
> +	/* copy from pll_cpux_clk */
> +	.enable	= BIT(31),
> +	.lock	= BIT(28),
> +};
> +
> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	void __iomem *reg;
>  	u32 val;
> +	int ret;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	reg = devm_ioremap_resource(&pdev->dev, res);
> @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct platform_device *pdev)
>  
>  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>  
> -	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> +	if (ret)
> +		return ret;
> +
> +	/* Gate then ungate PLL CPU after any rate changes */
> +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> +
> +	/* Reparent CPU during PLL CPU rate changes */
> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> +				  &sun50i_a64_cpu_nb);
> +
> +	return 0;

So this is the fourth user of the exact same code, can you turn that
into a shared function?

Thanks!
Maxime

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

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[icenowy]

在 2017-09-28 18:27，Maxime Ripard 写道：
> Hi,
> 
> On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
>> The A64 PLL_CPU clock has the same instability if some factor changed
>> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
>> H3.
>> 
>> Add the mux and pll notifiers for A64 CPU clock to workaround the
>> problem.
>> 
>> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 
>> +++++++++++++++++++++++++++-
>>  1 file changed, 27 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c 
>> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> index 2bb4cabf802f..b55fa69dd0c1 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc 
>> sun50i_a64_ccu_desc = {
>>  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>>  };
>> 
>> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
>> +	.common	= &pll_cpux_clk.common,
>> +	/* copy from pll_cpux_clk */
>> +	.enable	= BIT(31),
>> +	.lock	= BIT(28),
>> +};
>> +
>> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>>  {
>>  	struct resource *res;
>>  	void __iomem *reg;
>>  	u32 val;
>> +	int ret;
>> 
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	reg = devm_ioremap_resource(&pdev->dev, res);
>> @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct 
>> platform_device *pdev)
>> 
>>  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>> 
>> -	return sunxi_ccu_probe(pdev->dev.of_node, reg, 
>> &sun50i_a64_ccu_desc);
>> +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Gate then ungate PLL CPU after any rate changes */
>> +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
>> +
>> +	/* Reparent CPU during PLL CPU rate changes */
>> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
>> +				  &sun50i_a64_cpu_nb);
>> +
>> +	return 0;
> 
> So this is the fourth user of the exact same code, can you turn that
> into a shared function?

I think it's not so worthful to extract the code, as:
- the notifier structs contains info of the clocks
- A31 seems not to need the PLL notifier.

> 
> Thanks!
> Maxime
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[icenowy at aosc.io]

? 2017-09-28 18:27?Maxime Ripard ???
> Hi,
> 
> On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
>> The A64 PLL_CPU clock has the same instability if some factor changed
>> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
>> H3.
>> 
>> Add the mux and pll notifiers for A64 CPU clock to workaround the
>> problem.
>> 
>> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 
>> +++++++++++++++++++++++++++-
>>  1 file changed, 27 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c 
>> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> index 2bb4cabf802f..b55fa69dd0c1 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc 
>> sun50i_a64_ccu_desc = {
>>  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>>  };
>> 
>> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
>> +	.common	= &pll_cpux_clk.common,
>> +	/* copy from pll_cpux_clk */
>> +	.enable	= BIT(31),
>> +	.lock	= BIT(28),
>> +};
>> +
>> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>>  {
>>  	struct resource *res;
>>  	void __iomem *reg;
>>  	u32 val;
>> +	int ret;
>> 
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	reg = devm_ioremap_resource(&pdev->dev, res);
>> @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct 
>> platform_device *pdev)
>> 
>>  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>> 
>> -	return sunxi_ccu_probe(pdev->dev.of_node, reg, 
>> &sun50i_a64_ccu_desc);
>> +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Gate then ungate PLL CPU after any rate changes */
>> +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
>> +
>> +	/* Reparent CPU during PLL CPU rate changes */
>> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
>> +				  &sun50i_a64_cpu_nb);
>> +
>> +	return 0;
> 
> So this is the fourth user of the exact same code, can you turn that
> into a shared function?

I think it's not so worthful to extract the code, as:
- the notifier structs contains info of the clocks
- A31 seems not to need the PLL notifier.

> 
> Thanks!
> Maxime
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[icenowy-h8G6r0blFSE]

在 2017-09-28 18:27，Maxime Ripard 写道：
> Hi,
> 
> On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
>> The A64 PLL_CPU clock has the same instability if some factor changed
>> without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
>> H3.
>> 
>> Add the mux and pll notifiers for A64 CPU clock to workaround the
>> problem.
>> 
>> Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> ---
>>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28 
>> +++++++++++++++++++++++++++-
>>  1 file changed, 27 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c 
>> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> index 2bb4cabf802f..b55fa69dd0c1 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc 
>> sun50i_a64_ccu_desc = {
>>  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>>  };
>> 
>> +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
>> +	.common	= &pll_cpux_clk.common,
>> +	/* copy from pll_cpux_clk */
>> +	.enable	= BIT(31),
>> +	.lock	= BIT(28),
>> +};
>> +
>> +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>>  {
>>  	struct resource *res;
>>  	void __iomem *reg;
>>  	u32 val;
>> +	int ret;
>> 
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	reg = devm_ioremap_resource(&pdev->dev, res);
>> @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct 
>> platform_device *pdev)
>> 
>>  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>> 
>> -	return sunxi_ccu_probe(pdev->dev.of_node, reg, 
>> &sun50i_a64_ccu_desc);
>> +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Gate then ungate PLL CPU after any rate changes */
>> +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
>> +
>> +	/* Reparent CPU during PLL CPU rate changes */
>> +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
>> +				  &sun50i_a64_cpu_nb);
>> +
>> +	return 0;
> 
> So this is the fourth user of the exact same code, can you turn that
> into a shared function?

I think it's not so worthful to extract the code, as:
- the notifier structs contains info of the clocks
- A31 seems not to need the PLL notifier.

> 
> Thanks!
> Maxime
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

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

On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy@aosc.io wrote:
> > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> > > The A64 PLL_CPU clock has the same instability if some factor changed
> > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > > H3.
> > > 
> > > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > > problem.
> > > 
> > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > ---
> > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
> > > +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > index 2bb4cabf802f..b55fa69dd0c1 100644
> > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
> > > sun50i_a64_ccu_desc = {
> > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
> > >  };
> > > 
> > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> > > +	.common	= &pll_cpux_clk.common,
> > > +	/* copy from pll_cpux_clk */
> > > +	.enable	= BIT(31),
> > > +	.lock	= BIT(28),
> > > +};
> > > +
> > > +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
> > >  {
> > >  	struct resource *res;
> > >  	void __iomem *reg;
> > >  	u32 val;
> > > +	int ret;
> > > 
> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >  	reg = devm_ioremap_resource(&pdev->dev, res);
> > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
> > > platform_device *pdev)
> > > 
> > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> > > 
> > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
> > > &sun50i_a64_ccu_desc);
> > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Gate then ungate PLL CPU after any rate changes */
> > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> > > +
> > > +	/* Reparent CPU during PLL CPU rate changes */
> > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> > > +				  &sun50i_a64_cpu_nb);
> > > +
> > > +	return 0;
> > 
> > So this is the fourth user of the exact same code, can you turn that
> > into a shared function?
> 
> I think it's not so worthful to extract the code, as:

It does, because the order is important. If you do not register the
notifiers in the right order, you have a bug, and:

> - the notifier structs contains info of the clocks

this should be passed as a parameter anyway,

> - A31 seems not to need the PLL notifier.

And you don't care about the ordering in that case, since there's just
one. If was talking about the H3, A64, R40 and A33 that all have that
code.

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 --]

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy at aosc.io wrote:
> > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> > > The A64 PLL_CPU clock has the same instability if some factor changed
> > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > > H3.
> > > 
> > > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > > problem.
> > > 
> > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > ---
> > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
> > > +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > index 2bb4cabf802f..b55fa69dd0c1 100644
> > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
> > > sun50i_a64_ccu_desc = {
> > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
> > >  };
> > > 
> > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> > > +	.common	= &pll_cpux_clk.common,
> > > +	/* copy from pll_cpux_clk */
> > > +	.enable	= BIT(31),
> > > +	.lock	= BIT(28),
> > > +};
> > > +
> > > +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
> > >  {
> > >  	struct resource *res;
> > >  	void __iomem *reg;
> > >  	u32 val;
> > > +	int ret;
> > > 
> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >  	reg = devm_ioremap_resource(&pdev->dev, res);
> > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
> > > platform_device *pdev)
> > > 
> > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> > > 
> > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
> > > &sun50i_a64_ccu_desc);
> > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Gate then ungate PLL CPU after any rate changes */
> > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> > > +
> > > +	/* Reparent CPU during PLL CPU rate changes */
> > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> > > +				  &sun50i_a64_cpu_nb);
> > > +
> > > +	return 0;
> > 
> > So this is the fourth user of the exact same code, can you turn that
> > into a shared function?
> 
> I think it's not so worthful to extract the code, as:

It does, because the order is important. If you do not register the
notifiers in the right order, you have a bug, and:

> - the notifier structs contains info of the clocks

this should be passed as a parameter anyway,

> - A31 seems not to need the PLL notifier.

And you don't care about the ordering in that case, since there's just
one. If was talking about the H3, A64, R40 and A33 that all have that
code.

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/20170928/d10bc7f2/attachment.sig>

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

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

On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy-h8G6r0blFSE@public.gmane.org wrote:
> > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> > > The A64 PLL_CPU clock has the same instability if some factor changed
> > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > > H3.
> > > 
> > > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > > problem.
> > > 
> > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > > Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> > > ---
> > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
> > > +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > index 2bb4cabf802f..b55fa69dd0c1 100644
> > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
> > > sun50i_a64_ccu_desc = {
> > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
> > >  };
> > > 
> > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> > > +	.common	= &pll_cpux_clk.common,
> > > +	/* copy from pll_cpux_clk */
> > > +	.enable	= BIT(31),
> > > +	.lock	= BIT(28),
> > > +};
> > > +
> > > +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
> > >  {
> > >  	struct resource *res;
> > >  	void __iomem *reg;
> > >  	u32 val;
> > > +	int ret;
> > > 
> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >  	reg = devm_ioremap_resource(&pdev->dev, res);
> > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
> > > platform_device *pdev)
> > > 
> > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> > > 
> > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
> > > &sun50i_a64_ccu_desc);
> > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Gate then ungate PLL CPU after any rate changes */
> > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> > > +
> > > +	/* Reparent CPU during PLL CPU rate changes */
> > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> > > +				  &sun50i_a64_cpu_nb);
> > > +
> > > +	return 0;
> > 
> > So this is the fourth user of the exact same code, can you turn that
> > into a shared function?
> 
> I think it's not so worthful to extract the code, as:

It does, because the order is important. If you do not register the
notifiers in the right order, you have a bug, and:

> - the notifier structs contains info of the clocks

this should be passed as a parameter anyway,

> - A31 seems not to need the PLL notifier.

And you don't care about the ordering in that case, since there's just
one. If was talking about the H3, A64, R40 and A33 that all have that
code.

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 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[icenowy]

在 2017-09-28 22:20，Maxime Ripard 写道：
> On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy@aosc.io wrote:
>> > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
>> > > The A64 PLL_CPU clock has the same instability if some factor changed
>> > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
>> > > H3.
>> > >
>> > > Add the mux and pll notifiers for A64 CPU clock to workaround the
>> > > problem.
>> > >
>> > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
>> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> > > ---
>> > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
>> > > +++++++++++++++++++++++++++-
>> > >  1 file changed, 27 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > index 2bb4cabf802f..b55fa69dd0c1 100644
>> > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
>> > > sun50i_a64_ccu_desc = {
>> > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>> > >  };
>> > >
>> > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
>> > > +	.common	= &pll_cpux_clk.common,
>> > > +	/* copy from pll_cpux_clk */
>> > > +	.enable	= BIT(31),
>> > > +	.lock	= BIT(28),
>> > > +};
>> > > +
>> > > +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>> > >  {
>> > >  	struct resource *res;
>> > >  	void __iomem *reg;
>> > >  	u32 val;
>> > > +	int ret;
>> > >
>> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > >  	reg = devm_ioremap_resource(&pdev->dev, res);
>> > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
>> > > platform_device *pdev)
>> > >
>> > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>> > >
>> > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
>> > > &sun50i_a64_ccu_desc);
>> > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
>> > > +	if (ret)
>> > > +		return ret;
>> > > +
>> > > +	/* Gate then ungate PLL CPU after any rate changes */
>> > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
>> > > +
>> > > +	/* Reparent CPU during PLL CPU rate changes */
>> > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
>> > > +				  &sun50i_a64_cpu_nb);
>> > > +
>> > > +	return 0;
>> >
>> > So this is the fourth user of the exact same code, can you turn that
>> > into a shared function?
>> 
>> I think it's not so worthful to extract the code, as:
> 
> It does, because the order is important. If you do not register the
> notifiers in the right order, you have a bug, and:
> 
>> - the notifier structs contains info of the clocks
> 
> this should be passed as a parameter anyway,

So the function only does these two registers?

> 
>> - A31 seems not to need the PLL notifier.
> 
> And you don't care about the ordering in that case, since there's just
> one. If was talking about the H3, A64, R40 and A33 that all have that
> code.
> 
> Maxime

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[icenowy at aosc.io]

? 2017-09-28 22:20?Maxime Ripard ???
> On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy at aosc.io wrote:
>> > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
>> > > The A64 PLL_CPU clock has the same instability if some factor changed
>> > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
>> > > H3.
>> > >
>> > > Add the mux and pll notifiers for A64 CPU clock to workaround the
>> > > problem.
>> > >
>> > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
>> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> > > ---
>> > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
>> > > +++++++++++++++++++++++++++-
>> > >  1 file changed, 27 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > index 2bb4cabf802f..b55fa69dd0c1 100644
>> > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
>> > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
>> > > sun50i_a64_ccu_desc = {
>> > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
>> > >  };
>> > >
>> > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
>> > > +	.common	= &pll_cpux_clk.common,
>> > > +	/* copy from pll_cpux_clk */
>> > > +	.enable	= BIT(31),
>> > > +	.lock	= BIT(28),
>> > > +};
>> > > +
>> > > +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
>> > >  {
>> > >  	struct resource *res;
>> > >  	void __iomem *reg;
>> > >  	u32 val;
>> > > +	int ret;
>> > >
>> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > >  	reg = devm_ioremap_resource(&pdev->dev, res);
>> > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
>> > > platform_device *pdev)
>> > >
>> > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>> > >
>> > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
>> > > &sun50i_a64_ccu_desc);
>> > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
>> > > +	if (ret)
>> > > +		return ret;
>> > > +
>> > > +	/* Gate then ungate PLL CPU after any rate changes */
>> > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
>> > > +
>> > > +	/* Reparent CPU during PLL CPU rate changes */
>> > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
>> > > +				  &sun50i_a64_cpu_nb);
>> > > +
>> > > +	return 0;
>> >
>> > So this is the fourth user of the exact same code, can you turn that
>> > into a shared function?
>> 
>> I think it's not so worthful to extract the code, as:
> 
> It does, because the order is important. If you do not register the
> notifiers in the right order, you have a bug, and:
> 
>> - the notifier structs contains info of the clocks
> 
> this should be passed as a parameter anyway,

So the function only does these two registers?

> 
>> - A31 seems not to need the PLL notifier.
> 
> And you don't care about the ordering in that case, since there's just
> one. If was talking about the H3, A64, R40 and A33 that all have that
> code.
> 
> Maxime

Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

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

On Thu, Sep 28, 2017 at 02:24:18PM +0000, icenowy@aosc.io wrote:
> 在 2017-09-28 22:20，Maxime Ripard 写道：
> > On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy@aosc.io wrote:
> > > > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> > > > > The A64 PLL_CPU clock has the same instability if some factor changed
> > > > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > > > > H3.
> > > > >
> > > > > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > > > > problem.
> > > > >
> > > > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > > ---
> > > > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
> > > > > +++++++++++++++++++++++++++-
> > > > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > index 2bb4cabf802f..b55fa69dd0c1 100644
> > > > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
> > > > > sun50i_a64_ccu_desc = {
> > > > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
> > > > >  };
> > > > >
> > > > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> > > > > +	.common	= &pll_cpux_clk.common,
> > > > > +	/* copy from pll_cpux_clk */
> > > > > +	.enable	= BIT(31),
> > > > > +	.lock	= BIT(28),
> > > > > +};
> > > > > +
> > > > > +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
> > > > >  {
> > > > >  	struct resource *res;
> > > > >  	void __iomem *reg;
> > > > >  	u32 val;
> > > > > +	int ret;
> > > > >
> > > > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > >  	reg = devm_ioremap_resource(&pdev->dev, res);
> > > > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
> > > > > platform_device *pdev)
> > > > >
> > > > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> > > > >
> > > > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
> > > > > &sun50i_a64_ccu_desc);
> > > > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	/* Gate then ungate PLL CPU after any rate changes */
> > > > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> > > > > +
> > > > > +	/* Reparent CPU during PLL CPU rate changes */
> > > > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> > > > > +				  &sun50i_a64_cpu_nb);
> > > > > +
> > > > > +	return 0;
> > > >
> > > > So this is the fourth user of the exact same code, can you turn that
> > > > into a shared function?
> > > 
> > > I think it's not so worthful to extract the code, as:
> > 
> > It does, because the order is important. If you do not register the
> > notifiers in the right order, you have a bug, and:
> > 
> > > - the notifier structs contains info of the clocks
> > 
> > this should be passed as a parameter anyway,
> 
> So the function only does these two registers?

Yes.

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 --]

[PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

[Maxime Ripard]

On Thu, Sep 28, 2017 at 02:24:18PM +0000, icenowy at aosc.io wrote:
> ? 2017-09-28 22:20?Maxime Ripard ???
> > On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy at aosc.io wrote:
> > > > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> > > > > The A64 PLL_CPU clock has the same instability if some factor changed
> > > > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > > > > H3.
> > > > >
> > > > > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > > > > problem.
> > > > >
> > > > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > > ---
> > > > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
> > > > > +++++++++++++++++++++++++++-
> > > > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > index 2bb4cabf802f..b55fa69dd0c1 100644
> > > > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
> > > > > sun50i_a64_ccu_desc = {
> > > > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
> > > > >  };
> > > > >
> > > > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> > > > > +	.common	= &pll_cpux_clk.common,
> > > > > +	/* copy from pll_cpux_clk */
> > > > > +	.enable	= BIT(31),
> > > > > +	.lock	= BIT(28),
> > > > > +};
> > > > > +
> > > > > +static struct ccu_mux_nb sun50i_a64_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 int sun50i_a64_ccu_probe(struct platform_device *pdev)
> > > > >  {
> > > > >  	struct resource *res;
> > > > >  	void __iomem *reg;
> > > > >  	u32 val;
> > > > > +	int ret;
> > > > >
> > > > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > >  	reg = devm_ioremap_resource(&pdev->dev, res);
> > > > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
> > > > > platform_device *pdev)
> > > > >
> > > > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> > > > >
> > > > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
> > > > > &sun50i_a64_ccu_desc);
> > > > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	/* Gate then ungate PLL CPU after any rate changes */
> > > > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> > > > > +
> > > > > +	/* Reparent CPU during PLL CPU rate changes */
> > > > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> > > > > +				  &sun50i_a64_cpu_nb);
> > > > > +
> > > > > +	return 0;
> > > >
> > > > So this is the fourth user of the exact same code, can you turn that
> > > > into a shared function?
> > > 
> > > I think it's not so worthful to extract the code, as:
> > 
> > It does, because the order is important. If you do not register the
> > notifiers in the right order, you have a bug, and:
> > 
> > > - the notifier structs contains info of the clocks
> > 
> > this should be passed as a parameter anyway,
> 
> So the function only does these two registers?

Yes.

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/20170928/57c1bf49/attachment.sig>