Re: [PATCH v6 6/6] clk: X1000: Add FIXDIV for SSI clock of X1000.

Zhou Yanjie <zhouyanjie@xxxxxxxxxxxxxx> · Sun, 22 Mar 2020 20:43:16 +0800

Hi Paul,

On 2020年03月22日 20:12, Paul Cercueil wrote:
Hi Zhou,

Le dim. 22 mars 2020 à 16:21, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@xxxxxxxxxxxxxx> a écrit :
1.The SSI clock of X1000 not like JZ4770 and JZ4780, they are not
  directly derived from the output of SSIPLL, but from the clock
  obtained by dividing the frequency by 2. "X1000_CLK_SSIPLL_DIV2"
  is added for this purpose, and ensure that it initialized before
  "X1000_CLK_SSIMUX" when initializing the clocks.
2.Clocks of LCD, OTG, EMC, EFUSE, OST, and gates of CPU, PCLK
  are also added.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx>
---

Notes:
    v5:
    New patch.

    V5->v6:
    Add missing part of X1000's CGU.

 drivers/clk/ingenic/x1000-cgu.c | 56 
++++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/ingenic/x1000-cgu.c 
b/drivers/clk/ingenic/x1000-cgu.c
index 6f0ec9d..7e3ef0d 100644
--- a/drivers/clk/ingenic/x1000-cgu.c
+++ b/drivers/clk/ingenic/x1000-cgu.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * X1000 SoC CGU driver
- * Copyright (c) 2019 Zhou Yanjie <zhouyanjie@xxxxxxxx>
+ * Copyright (c) 2019 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx>
  */

 #include <linux/clk-provider.h>
@@ -18,6 +18,7 @@
 #define CGU_REG_CLKGR        0x20
 #define CGU_REG_OPCR        0x24
 #define CGU_REG_DDRCDR        0x2c
+#define CGU_REG_USBCDR        0x50
 #define CGU_REG_MACCDR        0x54
 #define CGU_REG_I2SCDR        0x60
 #define CGU_REG_LPCDR        0x64
@@ -114,9 +115,10 @@ static const struct ingenic_cgu_clk_info 
x1000_cgu_clocks[] = {
     },

     [X1000_CLK_CPU] = {
-        "cpu", CGU_CLK_DIV,
+        "cpu", CGU_CLK_DIV | CGU_CLK_GATE,
         .parents = { X1000_CLK_CPUMUX, -1, -1, -1 },
         .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 },
+        .gate = { CGU_REG_CLKGR, 30 },
     },

     [X1000_CLK_L2CACHE] = {
@@ -145,9 +147,10 @@ static const struct ingenic_cgu_clk_info 
x1000_cgu_clocks[] = {
     },

     [X1000_CLK_PCLK] = {
-        "pclk", CGU_CLK_DIV,
+        "pclk", CGU_CLK_DIV | CGU_CLK_GATE,
         .parents = { X1000_CLK_AHB2PMUX, -1, -1, -1 },
         .div = { CGU_REG_CPCCR, 16, 1, 4, 20, -1, -1 },
+        .gate = { CGU_REG_CLKGR, 28 },
     },

     [X1000_CLK_DDR] = {
@@ -160,12 +163,20 @@ static const struct ingenic_cgu_clk_info 
x1000_cgu_clocks[] = {

     [X1000_CLK_MAC] = {
         "mac", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
-        .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL},
+        .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL },
         .mux = { CGU_REG_MACCDR, 31, 1 },
         .div = { CGU_REG_MACCDR, 0, 1, 8, 29, 28, 27 },
         .gate = { CGU_REG_CLKGR, 25 },
     },

+    [X1000_CLK_LCD] = {
+        "lcd", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
+        .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL },
+        .mux = { CGU_REG_LPCDR, 31, 1 },
+        .div = { CGU_REG_LPCDR, 0, 1, 8, 28, 27, 26 },
+        .gate = { CGU_REG_CLKGR, 23 },
+    },
+
     [X1000_CLK_MSCMUX] = {
         "msc_mux", CGU_CLK_MUX,
         .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL},
@@ -186,6 +197,15 @@ static const struct ingenic_cgu_clk_info 
x1000_cgu_clocks[] = {
         .gate = { CGU_REG_CLKGR, 5 },
     },

+    [X1000_CLK_OTG] = {
+        "otg", CGU_CLK_DIV | CGU_CLK_GATE | CGU_CLK_MUX,
+        .parents = { X1000_CLK_EXCLK, -1,
+                     X1000_CLK_APLL, X1000_CLK_MPLL },
+        .mux = { CGU_REG_USBCDR, 30, 2 },
+        .div = { CGU_REG_USBCDR, 0, 1, 8, 29, 28, 27 },
+        .gate = { CGU_REG_CLKGR, 3 },
+    },
+
     [X1000_CLK_SSIPLL] = {
         "ssi_pll", CGU_CLK_MUX | CGU_CLK_DIV,
         .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL, -1, -1 },
@@ -193,14 +213,32 @@ static const struct ingenic_cgu_clk_info 
x1000_cgu_clocks[] = {
         .div = { CGU_REG_SSICDR, 0, 1, 8, 29, 28, 27 },
     },

+    [X1000_CLK_SSIPLL_DIV2] = {
+        "ssi_pll_div2", CGU_CLK_FIXDIV,
+        .parents = { X1000_CLK_SSIPLL },
+        .fixdiv = { 2 },
+    },
+
     [X1000_CLK_SSIMUX] = {
         "ssi_mux", CGU_CLK_MUX,
-        .parents = { X1000_CLK_EXCLK, X1000_CLK_SSIPLL, -1, -1 },
+        .parents = { X1000_CLK_EXCLK, X1000_CLK_SSIPLL_DIV2, -1, -1 },
         .mux = { CGU_REG_SSICDR, 30, 1 },
     },

     /* Gate-only clocks */

+    [X1000_CLK_EMC] = {
+        "emc", CGU_CLK_GATE,
+        .parents = { X1000_CLK_AHB2, -1, -1, -1 },
+        .gate = { CGU_REG_CLKGR, 0 },
+    },
+
+    [X1000_CLK_EFUSE] = {
+        "efuse", CGU_CLK_GATE,
+        .parents = { X1000_CLK_AHB2, -1, -1, -1 },
+        .gate = { CGU_REG_CLKGR, 1 },
+    },
+
     [X1000_CLK_SFC] = {
         "sfc", CGU_CLK_GATE,
         .parents = { X1000_CLK_SSIPLL, -1, -1, -1 },
@@ -249,6 +287,12 @@ static const struct ingenic_cgu_clk_info 
x1000_cgu_clocks[] = {
         .gate = { CGU_REG_CLKGR, 19 },
     },

+    [X1000_CLK_OST] = {
+        "ost", CGU_CLK_GATE,
+        .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
+        .gate = { CGU_REG_CLKGR, 20 },
+    },
+
     [X1000_CLK_PDMA] = {
         "pdma", CGU_CLK_GATE,
         .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
@@ -275,4 +319,4 @@ static void __init x1000_cgu_init(struct 
device_node *np)

     ingenic_cgu_register_syscore_ops(cgu);
 }
-CLK_OF_DECLARE(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init);
+CLK_OF_DECLARE_DRIVER(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init);

This change is not listed in the commit message, so it's not obvious 
why it's here. Please explain why it's changed in the commit message.


This should be that when x1000-cgu.c was first submitted the old way was 
used. In earlier cgu drivers CLK_OF_DECLARE was used here, but other cgu 
drivers have now been changed to CLK_OF_DECLARE_DRIVER., and this one 
should also be changed. I will mention this in the commit message in the 
next version.

Thanks and best regards!

Cheers,
-Paul

--
2.7.4









