[PATCH 01/11] clk: imx: clk-pllv3: fix incorrect handle of enet powerdown bit

After commit f53947456f98 ("ARM: clk: imx: update pllv3 to support imx7"),
the former used BM_PLL_POWER bit is not correct anymore for IMX7 ENET.
Instead, pll->powerdown holds the correct bit, so using powerdown bit
in clk_pllv3_{prepare | unprepare} functions.

Fixes: f53947456f98 ("ARM: clk: imx: update pllv3 to support imx7")
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-pllv3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 4826b3c9e19e..44d294a336f0 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -77,9 +77,9 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 
 	val = readl_relaxed(pll->base);
 	if (pll->powerup_set)
-		val |= BM_PLL_POWER;
+		val |= pll->powerdown;
 	else
-		val &= ~BM_PLL_POWER;
+		val &= ~pll->powerdown;
 	writel_relaxed(val, pll->base);
 
 	return clk_pllv3_wait_lock(pll);

@@ -92,9 +92,9 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 
 	val = readl_relaxed(pll->base);
 	if (pll->powerup_set)
-		val &= ~BM_PLL_POWER;
+		val &= ~pll->powerdown;
 	else
-		val |= BM_PLL_POWER;
+		val |= pll->powerdown;
 	writel_relaxed(val, pll->base);
 }
 

-- 
1.9.1

From: Anson Huang <redacted>

DRAM root clk should be either from pll dram main clk
or dram alt root clk.

Signed-off-by: Anson Huang <redacted>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx7d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 3be2e9371491..1cb866773fc0 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c

@@ -65,7 +65,7 @@ static const char *dram_phym_sel[] = { "pll_dram_main_clk",
 	"dram_phym_alt_clk", };
 
 static const char *dram_sel[] = { "pll_dram_main_clk",
-	"dram_alt_clk", };
+	"dram_alt_root_clk", };
 
 static const char *dram_phym_alt_sel[] = { "osc", "pll_dram_533m_clk",
 	"pll_sys_main_clk", "pll_enet_500m_clk",

-- 
1.9.1

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx6sx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6sx.c b/drivers/clk/imx/clk-imx6sx.c
index 97e742a8be17..b5c96de41ccf 100644
--- a/drivers/clk/imx/clk-imx6sx.c
+++ b/drivers/clk/imx/clk-imx6sx.c

@@ -174,13 +174,13 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 	clks[IMX6SX_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", base + 0x20, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
 
 	/*                                    type               name    parent_name        base         div_mask */
-	clks[IMX6SX_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,     "pll1", "pll1_bypass_src", base + 0x00, 0x7f);
-	clks[IMX6SX_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", base + 0x30, 0x1);
-	clks[IMX6SX_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll3", "pll3_bypass_src", base + 0x10, 0x3);
-	clks[IMX6SX_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "pll4_bypass_src", base + 0x70, 0x7f);
-	clks[IMX6SX_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll5", "pll5_bypass_src", base + 0xa0, 0x7f);
-	clks[IMX6SX_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll6", "pll6_bypass_src", base + 0xe0, 0x3);
-	clks[IMX6SX_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll7", "pll7_bypass_src", base + 0x20, 0x3);
+	clks[IMX6SX_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,     "pll1", "osc", base + 0x00, 0x7f);
+	clks[IMX6SX_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "osc", base + 0x30, 0x1);
+	clks[IMX6SX_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll3", "osc", base + 0x10, 0x3);
+	clks[IMX6SX_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "osc", base + 0x70, 0x7f);
+	clks[IMX6SX_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll5", "osc", base + 0xa0, 0x7f);
+	clks[IMX6SX_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll6", "osc", base + 0xe0, 0x3);
+	clks[IMX6SX_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll7", "osc", base + 0x20, 0x3);
 
 	clks[IMX6SX_PLL1_BYPASS] = imx_clk_mux_flags("pll1_bypass", base + 0x00, 16, 1, pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels), CLK_SET_RATE_PARENT);
 	clks[IMX6SX_PLL2_BYPASS] = imx_clk_mux_flags("pll2_bypass", base + 0x30, 16, 1, pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels), CLK_SET_RATE_PARENT);

-- 
1.9.1

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx6sl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6sl.c b/drivers/clk/imx/clk-imx6sl.c
index 1be6230a07af..5fd4ddac1bf1 100644
--- a/drivers/clk/imx/clk-imx6sl.c
+++ b/drivers/clk/imx/clk-imx6sl.c

@@ -218,13 +218,13 @@ static void __init imx6sl_clocks_init(struct device_node *ccm_node)
 	clks[IMX6SL_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", base + 0x20, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
 
 	/*                                    type               name    parent_name        base         div_mask */
-	clks[IMX6SL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,     "pll1", "pll1_bypass_src", base + 0x00, 0x7f);
-	clks[IMX6SL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", base + 0x30, 0x1);
-	clks[IMX6SL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll3", "pll3_bypass_src", base + 0x10, 0x3);
-	clks[IMX6SL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "pll4_bypass_src", base + 0x70, 0x7f);
-	clks[IMX6SL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll5", "pll5_bypass_src", base + 0xa0, 0x7f);
-	clks[IMX6SL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll6", "pll6_bypass_src", base + 0xe0, 0x3);
-	clks[IMX6SL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll7", "pll7_bypass_src", base + 0x20, 0x3);
+	clks[IMX6SL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,     "pll1", "osc", base + 0x00, 0x7f);
+	clks[IMX6SL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "osc", base + 0x30, 0x1);
+	clks[IMX6SL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll3", "osc", base + 0x10, 0x3);
+	clks[IMX6SL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "osc", base + 0x70, 0x7f);
+	clks[IMX6SL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll5", "osc", base + 0xa0, 0x7f);
+	clks[IMX6SL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll6", "osc", base + 0xe0, 0x3);
+	clks[IMX6SL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll7", "osc", base + 0x20, 0x3);
 
 	clks[IMX6SL_PLL1_BYPASS] = imx_clk_mux_flags("pll1_bypass", base + 0x00, 16, 1, pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels), CLK_SET_RATE_PARENT);
 	clks[IMX6SL_PLL2_BYPASS] = imx_clk_mux_flags("pll2_bypass", base + 0x30, 16, 1, pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels), CLK_SET_RATE_PARENT);

-- 
1.9.1

fix gpt2 clock names

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx6ul.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index 0f1f17a8f3ed..67ae3465dd37 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c

@@ -305,8 +305,8 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node)
 	clks[IMX6UL_CLK_CAN1_SERIAL]	= imx_clk_gate2("can1_serial",	"can_podf",	base + 0x68,	16);
 	clks[IMX6UL_CLK_CAN2_IPG]	= imx_clk_gate2("can2_ipg",	"ipg",		base + 0x68,	18);
 	clks[IMX6UL_CLK_CAN2_SERIAL]	= imx_clk_gate2("can2_serial",	"can_podf",	base + 0x68,	20);
-	clks[IMX6UL_CLK_GPT2_BUS]	= imx_clk_gate2("gpt_bus",	"perclk",	base + 0x68,	24);
-	clks[IMX6UL_CLK_GPT2_SERIAL]	= imx_clk_gate2("gpt_serial",	"perclk",	base + 0x68,	26);
+	clks[IMX6UL_CLK_GPT2_BUS]	= imx_clk_gate2("gpt_bus2",	"perclk",	base + 0x68,	24);
+	clks[IMX6UL_CLK_GPT2_SERIAL]	= imx_clk_gate2("gpt_serial2",	"perclk",	base + 0x68,	26);
 	clks[IMX6UL_CLK_UART2_IPG]	= imx_clk_gate2("uart2_ipg",	"ipg",		base + 0x68,	28);
 	clks[IMX6UL_CLK_UART2_SERIAL]	= imx_clk_gate2("uart2_serial",	"uart_podf",	base + 0x68,	28);
 	clks[IMX6UL_CLK_AIPSTZ3]	= imx_clk_gate2("aips_tz3",	"ahb",		base + 0x68,	30);

-- 
1.9.1

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx6q.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 02e18182fcb5..89bbfc1be476 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c

@@ -192,13 +192,13 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[IMX6QDL_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", base + 0x20, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
 
 	/*                                    type               name    parent_name        base         div_mask */
-	clk[IMX6QDL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,     "pll1", "pll1_bypass_src", base + 0x00, 0x7f);
-	clk[IMX6QDL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", base + 0x30, 0x1);
-	clk[IMX6QDL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll3", "pll3_bypass_src", base + 0x10, 0x3);
-	clk[IMX6QDL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "pll4_bypass_src", base + 0x70, 0x7f);
-	clk[IMX6QDL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll5", "pll5_bypass_src", base + 0xa0, 0x7f);
-	clk[IMX6QDL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll6", "pll6_bypass_src", base + 0xe0, 0x3);
-	clk[IMX6QDL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll7", "pll7_bypass_src", base + 0x20, 0x3);
+	clk[IMX6QDL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,     "pll1", "osc", base + 0x00, 0x7f);
+	clk[IMX6QDL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "osc", base + 0x30, 0x1);
+	clk[IMX6QDL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll3", "osc", base + 0x10, 0x3);
+	clk[IMX6QDL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "osc", base + 0x70, 0x7f);
+	clk[IMX6QDL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll5", "osc", base + 0xa0, 0x7f);
+	clk[IMX6QDL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll6", "osc", base + 0xe0, 0x3);
+	clk[IMX6QDL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll7", "osc", base + 0x20, 0x3);
 
 	clk[IMX6QDL_PLL1_BYPASS] = imx_clk_mux_flags("pll1_bypass", base + 0x00, 16, 1, pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels), CLK_SET_RATE_PARENT);
 	clk[IMX6QDL_PLL2_BYPASS] = imx_clk_mux_flags("pll2_bypass", base + 0x30, 16, 1, pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels), CLK_SET_RATE_PARENT);

-- 
1.9.1

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

e.g. before fix, the pll tree is:
osc                                      6            6    24000000          0 0
   pll1_bypass_src                       0            0    24000000          0 0
      pll1                               0            0   792000000          0 0
         pll1_bypass                     0            0   792000000          0 0
            pll1_sys                     0            0   792000000          0 0

After the fix, it's:
osc                                      6            6    24000000          0 0
   pll1                                  0            0   792000000          0 0
      pll1_bypass                        0            0   792000000          0 0
         pll1_sys                        0            0   792000000          0 0

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx6ul.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index 67ae3465dd37..bdf4cf807b78 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c

@@ -130,13 +130,13 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node)
 	clks[IMX6UL_PLL6_BYPASS_SRC] = imx_clk_mux("pll6_bypass_src", base + 0xe0, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
 	clks[IMX6UL_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", base + 0x20, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
 
-	clks[IMX6UL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,	 "pll1", "pll1_bypass_src", base + 0x00, 0x7f);
-	clks[IMX6UL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", base + 0x30, 0x1);
-	clks[IMX6UL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,	 "pll3", "pll3_bypass_src", base + 0x10, 0x3);
-	clks[IMX6UL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,	 "pll4", "pll4_bypass_src", base + 0x70, 0x7f);
-	clks[IMX6UL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,	 "pll5", "pll5_bypass_src", base + 0xa0, 0x7f);
-	clks[IMX6UL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,	 "pll6", "pll6_bypass_src", base + 0xe0, 0x3);
-	clks[IMX6UL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,	 "pll7", "pll7_bypass_src", base + 0x20, 0x3);
+	clks[IMX6UL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,	 "pll1", "osc", base + 0x00, 0x7f);
+	clks[IMX6UL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "osc", base + 0x30, 0x1);
+	clks[IMX6UL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,	 "pll3", "osc", base + 0x10, 0x3);
+	clks[IMX6UL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,	 "pll4", "osc", base + 0x70, 0x7f);
+	clks[IMX6UL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,	 "pll5", "osc", base + 0xa0, 0x7f);
+	clks[IMX6UL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,	 "pll6", "osc", base + 0xe0, 0x3);
+	clks[IMX6UL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,	 "pll7", "osc", base + 0x20, 0x3);
 
 	clks[IMX6UL_PLL1_BYPASS] = imx_clk_mux_flags("pll1_bypass", base + 0x00, 16, 1, pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels), CLK_SET_RATE_PARENT);
 	clks[IMX6UL_PLL2_BYPASS] = imx_clk_mux_flags("pll2_bypass", base + 0x30, 16, 1, pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels), CLK_SET_RATE_PARENT);

-- 
1.9.1

There's a powerdown bit already, so let's change the name of
powerup_set bit to power_invert to reflects the power polarity
to make it less confusing.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-pllv3.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index eea2b1b3791e..3fdfb6d2cc71 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -29,8 +29,8 @@
  * struct clk_pllv3 - IMX PLL clock version 3
  * @clk_hw:	 clock source
  * @base:	 base address of PLL registers
- * @powerup_set: set POWER bit to power up the PLL
- * @powerdown:   pll powerdown offset bit
+ * @powerdown:	 pll powerdown bit offset
+ * @power_invert: set powerdown bit to power up the PLL
  * @div_mask:	 mask of divider bits
  * @div_shift:	 shift of divider bits
  *

@@ -40,7 +40,7 @@
 struct clk_pllv3 {
 	struct clk_hw	hw;
 	void __iomem	*base;
-	bool		powerup_set;
+	bool		power_invert;
 	u32		powerdown;
 	u32		div_mask;
 	u32		div_shift;

@@ -55,7 +55,7 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
 	u32 val = readl_relaxed(pll->base) & pll->powerdown;
 
 	/* No need to wait for lock when pll is not powered up */
-	if ((pll->powerup_set && !val) || (!pll->powerup_set && val))
+	if ((pll->power_invert && !val) || (!pll->power_invert && val))
 		return 0;
 
 	/* Wait for PLL to lock */

@@ -76,7 +76,7 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 	u32 val;
 
 	val = readl_relaxed(pll->base);
-	if (pll->powerup_set)
+	if (pll->power_invert)
 		val |= pll->powerdown;
 	else
 		val &= ~pll->powerdown;

@@ -91,7 +91,7 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 	u32 val;
 
 	val = readl_relaxed(pll->base);
-	if (pll->powerup_set)
+	if (pll->power_invert)
 		val &= ~pll->powerdown;
 	else
 		val |= pll->powerdown;

@@ -326,7 +326,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		pll->div_shift = 1;
 	case IMX_PLLV3_USB:
 		ops = &clk_pllv3_ops;
-		pll->powerup_set = true;
+		pll->power_invert = true;
 		break;
 	case IMX_PLLV3_AV:
 		ops = &clk_pllv3_av_ops;

-- 
1.9.1

From: Anson Huang <redacted>

The audio/video PLL's rate calculation is as below in RM:

Fref * (DIV_SELECT + NUM / DENOM), in origin clk-pllv3's
code, below code is used:

(parent_rate * div) + ((parent_rate / mfd) * mfn

as it does NOT consider the float data using div, so below
formula should be used as a decent method:

(parent_rate * div) + ((parent_rate * mfn) / mfd)

and we also need to consider parent_rate * mfd may overflow
a 32 bit value, 64 bit value should be used.

After updating this formula, the dram PLL's rate is
1066MHz, which is correct, while the old formula gets
1056MHz.

[Aisheng: fix clk_pllv3_av_round_rate too]

Signed-off-by: Anson Huang <redacted>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-pllv3.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 44d294a336f0..eea2b1b3791e 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -218,8 +218,12 @@ static unsigned long clk_pllv3_av_recalc_rate(struct clk_hw *hw,
 	u32 mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
 	u32 mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
 	u32 div = readl_relaxed(pll->base) & pll->div_mask;
+	u64 temp64 = (u64)parent_rate;
 
-	return (parent_rate * div) + ((parent_rate / mfd) * mfn);
+	temp64 *= mfn;
+	do_div(temp64, mfd);
+
+	return (parent_rate * div) + (u32)temp64;
 }
 
 static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,

@@ -243,7 +247,7 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
 	do_div(temp64, parent_rate);
 	mfn = temp64;
 
-	return parent_rate * div + parent_rate / mfd * mfn;
+	return parent_rate * div + parent_rate * mfn / mfd;
 }
 
 static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,

-- 
1.9.1

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx7d.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index dc8c3355a66d..738a5289b378 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c

@@ -394,12 +394,12 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_PLL_AUDIO_MAIN_SRC] = imx_clk_mux("pll_audio_main_src", base + 0xf0, 14, 2, pll_bypass_src_sel, ARRAY_SIZE(pll_bypass_src_sel));
 	clks[IMX7D_PLL_VIDEO_MAIN_SRC] = imx_clk_mux("pll_video_main_src", base + 0x130, 14, 2, pll_bypass_src_sel, ARRAY_SIZE(pll_bypass_src_sel));
 
-	clks[IMX7D_PLL_ARM_MAIN]  = imx_clk_pllv3(IMX_PLLV3_SYS, "pll_arm_main", "pll_arm_main_src", base + 0x60, 0x7f);
-	clks[IMX7D_PLL_DRAM_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_dram_main", "pll_dram_main_src", base + 0x70, 0x7f);
-	clks[IMX7D_PLL_SYS_MAIN]  = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll_sys_main", "pll_sys_main_src", base + 0xb0, 0x1);
-	clks[IMX7D_PLL_ENET_MAIN] = imx_clk_pllv3(IMX_PLLV3_ENET_IMX7, "pll_enet_main", "pll_enet_main_src", base + 0xe0, 0x0);
-	clks[IMX7D_PLL_AUDIO_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_audio_main", "pll_audio_main_src", base + 0xf0, 0x7f);
-	clks[IMX7D_PLL_VIDEO_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_video_main", "pll_video_main_src", base + 0x130, 0x7f);
+	clks[IMX7D_PLL_ARM_MAIN]  = imx_clk_pllv3(IMX_PLLV3_SYS, "pll_arm_main", "osc", base + 0x60, 0x7f);
+	clks[IMX7D_PLL_DRAM_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_dram_main", "osc", base + 0x70, 0x7f);
+	clks[IMX7D_PLL_SYS_MAIN]  = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll_sys_main", "osc", base + 0xb0, 0x1);
+	clks[IMX7D_PLL_ENET_MAIN] = imx_clk_pllv3(IMX_PLLV3_ENET_IMX7, "pll_enet_main", "osc", base + 0xe0, 0x0);
+	clks[IMX7D_PLL_AUDIO_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_audio_main", "osc", base + 0xf0, 0x7f);
+	clks[IMX7D_PLL_VIDEO_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_video_main", "osc", base + 0x130, 0x7f);
 
 	clks[IMX7D_PLL_ARM_MAIN_BYPASS]  = imx_clk_mux_flags("pll_arm_main_bypass", base + 0x60, 16, 1, pll_arm_bypass_sel, ARRAY_SIZE(pll_arm_bypass_sel), CLK_SET_RATE_PARENT);
 	clks[IMX7D_PLL_DRAM_MAIN_BYPASS] = imx_clk_mux_flags("pll_dram_main_bypass", base + 0x70, 16, 1, pll_dram_bypass_sel, ARRAY_SIZE(pll_dram_bypass_sel), CLK_SET_RATE_PARENT);

-- 
1.9.1

From: Anson Huang <redacted>

DRAM PLL is a audio/video type PLL, need to correct
it to get correct ops of PLL.

Signed-off-by: Anson Huang <redacted>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx7d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 1cb866773fc0..dc8c3355a66d 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c

@@ -395,7 +395,7 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_PLL_VIDEO_MAIN_SRC] = imx_clk_mux("pll_video_main_src", base + 0x130, 14, 2, pll_bypass_src_sel, ARRAY_SIZE(pll_bypass_src_sel));
 
 	clks[IMX7D_PLL_ARM_MAIN]  = imx_clk_pllv3(IMX_PLLV3_SYS, "pll_arm_main", "pll_arm_main_src", base + 0x60, 0x7f);
-	clks[IMX7D_PLL_DRAM_MAIN] = imx_clk_pllv3(IMX_PLLV3_SYS, "pll_dram_main", "pll_dram_main_src", base + 0x70, 0x7f);
+	clks[IMX7D_PLL_DRAM_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_dram_main", "pll_dram_main_src", base + 0x70, 0x7f);
 	clks[IMX7D_PLL_SYS_MAIN]  = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll_sys_main", "pll_sys_main_src", base + 0xb0, 0x1);
 	clks[IMX7D_PLL_ENET_MAIN] = imx_clk_pllv3(IMX_PLLV3_ENET_IMX7, "pll_enet_main", "pll_enet_main_src", base + 0xe0, 0x0);
 	clks[IMX7D_PLL_AUDIO_MAIN] = imx_clk_pllv3(IMX_PLLV3_AV, "pll_audio_main", "pll_audio_main_src", base + 0xf0, 0x7f);

-- 
1.9.1

Hi,

On Wed, 8 Jun 2016 22:33:34 +0800 Dong Aisheng wrote:

s/set/clear/ ?



Lothar Wa?mann

On Wed, Jun 08, 2016 at 10:33:31PM +0800, Dong Aisheng wrote:

From: Anson Huang <redacted>

The audio/video PLL's rate calculation is as below in RM:

Fref * (DIV_SELECT + NUM / DENOM), in origin clk-pllv3's
code, below code is used:

(parent_rate * div) + ((parent_rate / mfd) * mfn

as it does NOT consider the float data using div, so below
formula should be used as a decent method:

(parent_rate * div) + ((parent_rate * mfn) / mfd)

and we also need to consider parent_rate * mfd may overflow
a 32 bit value, 64 bit value should be used.

After updating this formula, the dram PLL's rate is
1066MHz, which is correct, while the old formula gets
1056MHz.

[Aisheng: fix clk_pllv3_av_round_rate too]

Signed-off-by: Anson Huang <redacted>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

Applied, thanks.

On Wed, Jun 08, 2016 at 10:33:32PM +0800, Dong Aisheng wrote:

From: Anson Huang <redacted>

DRAM root clk should be either from pll dram main clk
or dram alt root clk.

Signed-off-by: Anson Huang <redacted>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

Applied, thanks.

On Wed, Jun 08, 2016 at 10:33:33PM +0800, Dong Aisheng wrote:

From: Anson Huang <redacted>

DRAM PLL is a audio/video type PLL, need to correct
it to get correct ops of PLL.

Signed-off-by: Anson Huang <redacted>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

'clk: imx7d: ' for patch prefix would be more clear.  Updated it and
applied the patch.

Shawn

On Wed, Jun 08, 2016 at 10:33:34PM +0800, Dong Aisheng wrote:

I think 'powerdown' is more confusing here.  I prefer to rename it to
something like 'power_mask' and keep 'powerdown' as it is.

Shawn

On Wed, Jun 08, 2016 at 10:33:35PM +0800, Dong Aisheng wrote:

gpt2_bus and gpt2_serial, please.

Shawn

 	clks[IMX6UL_CLK_UART2_IPG]	= imx_clk_gate2("uart2_ipg",	"ipg",		base + 0x68,	28);
 	clks[IMX6UL_CLK_UART2_SERIAL]	= imx_clk_gate2("uart2_serial",	"uart_podf",	base + 0x68,	28);
 	clks[IMX6UL_CLK_AIPSTZ3]	= imx_clk_gate2("aips_tz3",	"ahb",		base + 0x68,	30);
-- 
1.9.1


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

On Wed, Jun 08, 2016 at 10:33:36PM +0800, Dong Aisheng wrote:

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

e.g. before fix, the pll tree is:
osc                                      6            6    24000000          0 0
   pll1_bypass_src                       0            0    24000000          0 0
      pll1                               0            0   792000000          0 0
         pll1_bypass                     0            0   792000000          0 0
            pll1_sys                     0            0   792000000          0 0

After the fix, it's:
osc                                      6            6    24000000          0 0
   pll1                                  0            0   792000000          0 0
      pll1_bypass                        0            0   792000000          0 0
         pll1_sys                        0            0   792000000          0 0

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

I squashed 7 ~ 11 into one patch and applied it, thanks.

Shawn

Hi Shawn,

On Sun, Jun 12, 2016 at 07:36:27PM +0800, Shawn Guo wrote:

On Wed, Jun 08, 2016 at 10:33:34PM +0800, Dong Aisheng wrote:

quoted

There's a powerdown bit already, so let's change the name of
powerup_set bit to power_invert to reflects the power polarity
to make it less confusing.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-pllv3.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index eea2b1b3791e..3fdfb6d2cc71 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -29,8 +29,8 @@
  * struct clk_pllv3 - IMX PLL clock version 3
  * @clk_hw:	 clock source
  * @base:	 base address of PLL registers
- * @powerup_set: set POWER bit to power up the PLL
- * @powerdown:   pll powerdown offset bit
+ * @powerdown:	 pll powerdown bit offset

I think 'powerdown' is more confusing here.  I prefer to rename it to
something like 'power_mask' and keep 'powerdown' as it is.

A bit confused, you mean rename which one?
powerdown bit is defined by the spec so i just keep it.

Since *invert is wildly used in gpio subsystem, so i
change the powerup_set to power_invert to indicate the invert using
of powerdown bit.

Regards
Dong Aisheng

Shawn

quoted

+ * @power_invert: set powerdown bit to power up the PLL
  * @div_mask:	 mask of divider bits
  * @div_shift:	 shift of divider bits
  *

@@ -40,7 +40,7 @@
 struct clk_pllv3 {
 	struct clk_hw	hw;
 	void __iomem	*base;
-	bool		powerup_set;
+	bool		power_invert;
 	u32		powerdown;
 	u32		div_mask;
 	u32		div_shift;

@@ -55,7 +55,7 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
 	u32 val = readl_relaxed(pll->base) & pll->powerdown;
 
 	/* No need to wait for lock when pll is not powered up */
-	if ((pll->powerup_set && !val) || (!pll->powerup_set && val))
+	if ((pll->power_invert && !val) || (!pll->power_invert && val))
 		return 0;
 
 	/* Wait for PLL to lock */

@@ -76,7 +76,7 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 	u32 val;
 
 	val = readl_relaxed(pll->base);
-	if (pll->powerup_set)
+	if (pll->power_invert)
 		val |= pll->powerdown;
 	else
 		val &= ~pll->powerdown;

@@ -91,7 +91,7 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 	u32 val;
 
 	val = readl_relaxed(pll->base);
-	if (pll->powerup_set)
+	if (pll->power_invert)
 		val &= ~pll->powerdown;
 	else
 		val |= pll->powerdown;

@@ -326,7 +326,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		pll->div_shift = 1;
 	case IMX_PLLV3_USB:
 		ops = &clk_pllv3_ops;
-		pll->powerup_set = true;
+		pll->power_invert = true;
 		break;
 	case IMX_PLLV3_AV:
 		ops = &clk_pllv3_av_ops;

-- 
1.9.1


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

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Jun 12, 2016 at 07:43:53PM +0800, Shawn Guo wrote:

On Wed, Jun 08, 2016 at 10:33:36PM +0800, Dong Aisheng wrote:

quoted

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

e.g. before fix, the pll tree is:
osc                                      6            6    24000000          0 0
   pll1_bypass_src                       0            0    24000000          0 0
      pll1                               0            0   792000000          0 0
         pll1_bypass                     0            0   792000000          0 0
            pll1_sys                     0            0   792000000          0 0

After the fix, it's:
osc                                      6            6    24000000          0 0
   pll1                                  0            0   792000000          0 0
      pll1_bypass                        0            0   792000000          0 0
         pll1_sys                        0            0   792000000          0 0

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

I squashed 7 ~ 11 into one patch and applied it, thanks.

I'm fine.
Thanks

Regards
Dong Aisheng

Shawn
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Jun 12, 2016 at 07:41:11PM +0800, Shawn Guo wrote:

On Wed, Jun 08, 2016 at 10:33:35PM +0800, Dong Aisheng wrote:

quoted

fix gpt2 clock names

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx6ul.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index 0f1f17a8f3ed..67ae3465dd37 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c

@@ -305,8 +305,8 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node)
 	clks[IMX6UL_CLK_CAN1_SERIAL]	= imx_clk_gate2("can1_serial",	"can_podf",	base + 0x68,	16);
 	clks[IMX6UL_CLK_CAN2_IPG]	= imx_clk_gate2("can2_ipg",	"ipg",		base + 0x68,	18);
 	clks[IMX6UL_CLK_CAN2_SERIAL]	= imx_clk_gate2("can2_serial",	"can_podf",	base + 0x68,	20);
-	clks[IMX6UL_CLK_GPT2_BUS]	= imx_clk_gate2("gpt_bus",	"perclk",	base + 0x68,	24);
-	clks[IMX6UL_CLK_GPT2_SERIAL]	= imx_clk_gate2("gpt_serial",	"perclk",	base + 0x68,	26);
+	clks[IMX6UL_CLK_GPT2_BUS]	= imx_clk_gate2("gpt_bus2",	"perclk",	base + 0x68,	24);
+	clks[IMX6UL_CLK_GPT2_SERIAL]	= imx_clk_gate2("gpt_serial2",	"perclk",	base + 0x68,	26);

gpt2_bus and gpt2_serial, please.

Yes, sorry the careless.
Will resend it.

Regards
Dong Aisheng

Shawn

quoted

 	clks[IMX6UL_CLK_UART2_IPG]	= imx_clk_gate2("uart2_ipg",	"ipg",		base + 0x68,	28);
 	clks[IMX6UL_CLK_UART2_SERIAL]	= imx_clk_gate2("uart2_serial",	"uart_podf",	base + 0x68,	28);
 	clks[IMX6UL_CLK_AIPSTZ3]	= imx_clk_gate2("aips_tz3",	"ahb",		base + 0x68,	30);
-- 
1.9.1


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

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Jun 09, 2016 at 09:43:28AM +0200, Lothar Wa??mann wrote:

Hi,

On Wed, 8 Jun 2016 22:33:34 +0800 Dong Aisheng wrote:

quoted

There's a powerdown bit already, so let's change the name of
powerup_set bit to power_invert to reflects the power polarity
to make it less confusing.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-pllv3.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index eea2b1b3791e..3fdfb6d2cc71 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -29,8 +29,8 @@
  * struct clk_pllv3 - IMX PLL clock version 3
  * @clk_hw:	 clock source
  * @base:	 base address of PLL registers
- * @powerup_set: set POWER bit to power up the PLL
- * @powerdown:   pll powerdown offset bit
+ * @powerdown:	 pll powerdown bit offset
+ * @power_invert: set powerdown bit to power up the PLL

s/set/clear/ ?

It is set.
By default set the powerdown bit will powerdown the PLL according
to spec.
However, for IMX_PLLV3_USB, it's actually power up the PLL,
so the power_invert reflect such using.

Regards
Dong Aisheng

Lothar Wa??mann

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

On Sun, Jun 12, 2016 at 07:36:27PM +0800, Shawn Guo wrote:

On Wed, Jun 08, 2016 at 10:33:34PM +0800, Dong Aisheng wrote:

quoted

There's a powerdown bit already, so let's change the name of
powerup_set bit to power_invert to reflects the power polarity
to make it less confusing.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-pllv3.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index eea2b1b3791e..3fdfb6d2cc71 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -29,8 +29,8 @@
  * struct clk_pllv3 - IMX PLL clock version 3
  * @clk_hw:	 clock source
  * @base:	 base address of PLL registers
- * @powerup_set: set POWER bit to power up the PLL
- * @powerdown:   pll powerdown offset bit
+ * @powerdown:	 pll powerdown bit offset

I think 'powerdown' is more confusing here.  I prefer to rename it to
something like 'power_mask' and keep 'powerdown' as it is.

I understand your point.
How about using power_bit and powerup_set?
* @power_bit:   pll power bit offset
* @powerup_set: set power_bit to power up the PLL

Regards
Dong Aisheng

Shawn

quoted

+ * @power_invert: set powerdown bit to power up the PLL
  * @div_mask:	 mask of divider bits
  * @div_shift:	 shift of divider bits
  *

@@ -40,7 +40,7 @@
 struct clk_pllv3 {
 	struct clk_hw	hw;
 	void __iomem	*base;
-	bool		powerup_set;
+	bool		power_invert;
 	u32		powerdown;
 	u32		div_mask;
 	u32		div_shift;

@@ -55,7 +55,7 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
 	u32 val = readl_relaxed(pll->base) & pll->powerdown;
 
 	/* No need to wait for lock when pll is not powered up */
-	if ((pll->powerup_set && !val) || (!pll->powerup_set && val))
+	if ((pll->power_invert && !val) || (!pll->power_invert && val))
 		return 0;
 
 	/* Wait for PLL to lock */

@@ -76,7 +76,7 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 	u32 val;
 
 	val = readl_relaxed(pll->base);
-	if (pll->powerup_set)
+	if (pll->power_invert)
 		val |= pll->powerdown;
 	else
 		val &= ~pll->powerdown;

@@ -91,7 +91,7 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 	u32 val;
 
 	val = readl_relaxed(pll->base);
-	if (pll->powerup_set)
+	if (pll->power_invert)
 		val &= ~pll->powerdown;
 	else
 		val |= pll->powerdown;

@@ -326,7 +326,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		pll->div_shift = 1;
 	case IMX_PLLV3_USB:
 		ops = &clk_pllv3_ops;
-		pll->powerup_set = true;
+		pll->power_invert = true;
 		break;
 	case IMX_PLLV3_AV:
 		ops = &clk_pllv3_av_ops;

-- 
1.9.1


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

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Jun 12, 2016 at 07:52:04PM +0800, Dong Aisheng wrote:

On Sun, Jun 12, 2016 at 07:43:53PM +0800, Shawn Guo wrote:

quoted

On Wed, Jun 08, 2016 at 10:33:36PM +0800, Dong Aisheng wrote:

quoted

pllx_bypass_src mux shouldn't be the parent of pllx clock
since it's only valid when when pllx BYPASS bit is set.
Thus it is actually one parent of pllx_bypass only.

Instead, pllx parent should be fixed to osc according to
reference manual.
Other plls have the same issue.

e.g. before fix, the pll tree is:
osc                                      6            6    24000000          0 0
   pll1_bypass_src                       0            0    24000000          0 0
      pll1                               0            0   792000000          0 0
         pll1_bypass                     0            0   792000000          0 0
            pll1_sys                     0            0   792000000          0 0

After the fix, it's:
osc                                      6            6    24000000          0 0
   pll1                                  0            0   792000000          0 0
      pll1_bypass                        0            0   792000000          0 0
         pll1_sys                        0            0   792000000          0 0

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

I squashed 7 ~ 11 into one patch and applied it, thanks.

I'm fine.
Thanks

You probably may need to change the patch title after merge.
clk: imx: fix pll clock parents

Regards
Dong Aisheng

Regards
Dong Aisheng

quoted

Shawn
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Jun 12, 2016 at 08:19:47PM +0800, Dong Aisheng wrote:

You probably may need to change the patch title after merge.
clk: imx: fix pll clock parents

Right, thanks for the reminding.

Shawn

On Sun, Jun 12, 2016 at 08:13:03PM +0800, Dong Aisheng wrote:

I understand your point.
How about using power_bit and powerup_set?
* @power_bit:   pll power bit offset

I'm fine with the name, but the comment should be fixed, since we are
actually using it as a bit mask instead of offset.

Shawn

* @powerup_set: set power_bit to power up the PLL

On Sun, Jun 12, 2016 at 9:29 PM, Shawn Guo [off-list ref] wrote:

On Sun, Jun 12, 2016 at 08:13:03PM +0800, Dong Aisheng wrote:

quoted

I understand your point.
How about using power_bit and powerup_set?
* @power_bit:   pll power bit offset

I'm fine with the name, but the comment should be fixed, since we are
actually using it as a bit mask instead of offset.

Yes, i can change to:
* @power_bit:   pll power bit mask
* @powerup_set: set power_bit to power up the PLL

Regards
Dong Aisheng

Shawn

quoted

* @powerup_set: set power_bit to power up the PLL

Hi Shawn,

On Wed, Jun 8, 2016 at 10:33 PM, Dong Aisheng [off-list ref] wrote:

After commit f53947456f98 ("ARM: clk: imx: update pllv3 to support imx7"),
the former used BM_PLL_POWER bit is not correct anymore for IMX7 ENET.
Instead, pll->powerdown holds the correct bit, so using powerdown bit
in clk_pllv3_{prepare | unprepare} functions.

Fixes: f53947456f98 ("ARM: clk: imx: update pllv3 to support imx7")
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

Any comments about this one?

Regards
Dong Aisheng

On Sun, Jun 12, 2016 at 10:56:38PM +0800, Dong Aisheng wrote:

Hi Shawn,

On Wed, Jun 8, 2016 at 10:33 PM, Dong Aisheng [off-list ref] wrote:

quoted

After commit f53947456f98 ("ARM: clk: imx: update pllv3 to support imx7"),
the former used BM_PLL_POWER bit is not correct anymore for IMX7 ENET.
Instead, pll->powerdown holds the correct bit, so using powerdown bit
in clk_pllv3_{prepare | unprepare} functions.

Fixes: f53947456f98 ("ARM: clk: imx: update pllv3 to support imx7")
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

Any comments about this one?

Sorry.  I thought I had applied it.  Applied it now, thanks.

Shawn

fix gpt2 clock names

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx6ul.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index 38da5a522d58..d1d7787ce211 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c

@@ -305,8 +305,8 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node)
 	clks[IMX6UL_CLK_CAN1_SERIAL]	= imx_clk_gate2("can1_serial",	"can_podf",	base + 0x68,	16);
 	clks[IMX6UL_CLK_CAN2_IPG]	= imx_clk_gate2("can2_ipg",	"ipg",		base + 0x68,	18);
 	clks[IMX6UL_CLK_CAN2_SERIAL]	= imx_clk_gate2("can2_serial",	"can_podf",	base + 0x68,	20);
-	clks[IMX6UL_CLK_GPT2_BUS]	= imx_clk_gate2("gpt_bus",	"perclk",	base + 0x68,	24);
-	clks[IMX6UL_CLK_GPT2_SERIAL]	= imx_clk_gate2("gpt_serial",	"perclk",	base + 0x68,	26);
+	clks[IMX6UL_CLK_GPT2_BUS]	= imx_clk_gate2("gpt2_bus",	"perclk",	base + 0x68,	24);
+	clks[IMX6UL_CLK_GPT2_SERIAL]	= imx_clk_gate2("gpt2_serial",	"perclk",	base + 0x68,	26);
 	clks[IMX6UL_CLK_UART2_IPG]	= imx_clk_gate2("uart2_ipg",	"ipg",		base + 0x68,	28);
 	clks[IMX6UL_CLK_UART2_SERIAL]	= imx_clk_gate2("uart2_serial",	"uart_podf",	base + 0x68,	28);
 	clks[IMX6UL_CLK_AIPSTZ3]	= imx_clk_gate2("aips_tz3",	"ahb",		base + 0x68,	30);

-- 
1.9.1

The powerdown bit is a bit confused, let's change it to power_bit
to relfect both powerdown and powerup case according to different
plls.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-pllv3.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index eea2b1b3791e..379ec133fab2 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -29,8 +29,8 @@
  * struct clk_pllv3 - IMX PLL clock version 3
  * @clk_hw:	 clock source
  * @base:	 base address of PLL registers
- * @powerup_set: set POWER bit to power up the PLL
- * @powerdown:   pll powerdown offset bit
+ * @power_bit:	 pll power bit mask
+ * @powerup_set: set power_bit to power up the PLL
  * @div_mask:	 mask of divider bits
  * @div_shift:	 shift of divider bits
  *

@@ -40,8 +40,8 @@
 struct clk_pllv3 {
 	struct clk_hw	hw;
 	void __iomem	*base;
+	u32		power_bit;
 	bool		powerup_set;
-	u32		powerdown;
 	u32		div_mask;
 	u32		div_shift;
 	unsigned long	ref_clock;

@@ -52,7 +52,7 @@ struct clk_pllv3 {
 static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(10);
-	u32 val = readl_relaxed(pll->base) & pll->powerdown;
+	u32 val = readl_relaxed(pll->base) & pll->power_bit;
 
 	/* No need to wait for lock when pll is not powered up */
 	if ((pll->powerup_set && !val) || (!pll->powerup_set && val))

@@ -77,7 +77,7 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 
 	val = readl_relaxed(pll->base);
 	if (pll->powerup_set)
-		val |= pll->powerdown;
+		val |= pll->power_bit;
 	else
 		val &= ~pll->powerdown;
 	writel_relaxed(val, pll->base);

@@ -92,7 +92,7 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 
 	val = readl_relaxed(pll->base);
 	if (pll->powerup_set)
-		val &= ~pll->powerdown;
+		val &= ~pll->power_bit;
 	else
 		val |= pll->powerdown;
 	writel_relaxed(val, pll->base);

@@ -316,7 +316,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 	if (!pll)
 		return ERR_PTR(-ENOMEM);
 
-	pll->powerdown = BM_PLL_POWER;
+	pll->power_bit = BM_PLL_POWER;
 
 	switch (type) {
 	case IMX_PLLV3_SYS:

@@ -332,7 +332,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		ops = &clk_pllv3_av_ops;
 		break;
 	case IMX_PLLV3_ENET_IMX7:
-		pll->powerdown = IMX7_ENET_PLL_POWER;
+		pll->power_bit = IMX7_ENET_PLL_POWER;
 		pll->ref_clock = 1000000000;
 		ops = &clk_pllv3_enet_ops;
 		break;

-- 
1.9.1

Hi,

[auto build test ERROR on shawnguo/for-next]
[cannot apply to v4.7-rc3 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dong-Aisheng/clk-imx-refine-the-powerdown-bit-of-clk-pllv3/20160613-160108
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-imx_v4_v5_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/clk/imx/clk-pllv3.c: In function 'clk_pllv3_prepare':

quoted

drivers/clk/imx/clk-pllv3.c:82:14: error: 'struct clk_pllv3' has no member named 'powerdown'

      val &= ~pll->powerdown;
                 ^
   drivers/clk/imx/clk-pllv3.c: In function 'clk_pllv3_unprepare':
   drivers/clk/imx/clk-pllv3.c:97:13: error: 'struct clk_pllv3' has no member named 'powerdown'
      val |= pll->powerdown;
                ^

vim +82 drivers/clk/imx/clk-pllv3.c

bc3b84da arch/arm/mach-imx/clk-pllv3.c Shawn Guo     2013-10-30  76  	u32 val;
bc3b84da arch/arm/mach-imx/clk-pllv3.c Shawn Guo     2013-10-30  77  
bc3b84da arch/arm/mach-imx/clk-pllv3.c Shawn Guo     2013-10-30  78  	val = readl_relaxed(pll->base);
bc3b84da arch/arm/mach-imx/clk-pllv3.c Shawn Guo     2013-10-30  79  	if (pll->powerup_set)
b3e3e45d drivers/clk/imx/clk-pllv3.c   Dong Aisheng  2016-06-13  80  		val |= pll->power_bit;
0a036388 arch/arm/mach-imx/clk-pllv3.c Peter Chen    2013-07-16  81  	else
b3e76bdc drivers/clk/imx/clk-pllv3.c   Dong Aisheng  2016-06-08 @82  		val &= ~pll->powerdown;
bc3b84da arch/arm/mach-imx/clk-pllv3.c Shawn Guo     2013-10-30  83  	writel_relaxed(val, pll->base);
bc3b84da arch/arm/mach-imx/clk-pllv3.c Shawn Guo     2013-10-30  84  
c400f7a2 arch/arm/mach-imx/clk-pllv3.c Dmitry Voytik 2014-11-06  85  	return clk_pllv3_wait_lock(pll);

:::::: The code at line 82 was first introduced by commit
:::::: b3e76bdc0b2190e67427d31cd740debd01c03631 clk: imx: clk-pllv3: fix incorrect handle of enet powerdown bit

:::::: TO: Dong Aisheng [off-list ref]
:::::: CC: Shawn Guo [off-list ref]

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 22549 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160613/d18ff405/attachment-0001.obj>

The powerdown bit is a bit confused, let's change it to power_bit
to relfect both powerdown and powerup case according to different
plls.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
Changes from v2:
Sorry for sending the patch too quickly.
There's a rebase error in V2, fixed it.
---
 drivers/clk/imx/clk-pllv3.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index eea2b1b3791e..19f9b622981a 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c

@@ -29,8 +29,8 @@
  * struct clk_pllv3 - IMX PLL clock version 3
  * @clk_hw:	 clock source
  * @base:	 base address of PLL registers
- * @powerup_set: set POWER bit to power up the PLL
- * @powerdown:   pll powerdown offset bit
+ * @power_bit:	 pll power bit mask
+ * @powerup_set: set power_bit to power up the PLL
  * @div_mask:	 mask of divider bits
  * @div_shift:	 shift of divider bits
  *

@@ -40,8 +40,8 @@
 struct clk_pllv3 {
 	struct clk_hw	hw;
 	void __iomem	*base;
+	u32		power_bit;
 	bool		powerup_set;
-	u32		powerdown;
 	u32		div_mask;
 	u32		div_shift;
 	unsigned long	ref_clock;

@@ -52,7 +52,7 @@ struct clk_pllv3 {
 static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(10);
-	u32 val = readl_relaxed(pll->base) & pll->powerdown;
+	u32 val = readl_relaxed(pll->base) & pll->power_bit;
 
 	/* No need to wait for lock when pll is not powered up */
 	if ((pll->powerup_set && !val) || (!pll->powerup_set && val))

@@ -77,9 +77,9 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 
 	val = readl_relaxed(pll->base);
 	if (pll->powerup_set)
-		val |= pll->powerdown;
+		val |= pll->power_bit;
 	else
-		val &= ~pll->powerdown;
+		val &= ~pll->power_bit;
 	writel_relaxed(val, pll->base);
 
 	return clk_pllv3_wait_lock(pll);

@@ -92,9 +92,9 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 
 	val = readl_relaxed(pll->base);
 	if (pll->powerup_set)
-		val &= ~pll->powerdown;
+		val &= ~pll->power_bit;
 	else
-		val |= pll->powerdown;
+		val |= pll->power_bit;
 	writel_relaxed(val, pll->base);
 }
 

@@ -316,7 +316,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 	if (!pll)
 		return ERR_PTR(-ENOMEM);
 
-	pll->powerdown = BM_PLL_POWER;
+	pll->power_bit = BM_PLL_POWER;
 
 	switch (type) {
 	case IMX_PLLV3_SYS:

@@ -332,7 +332,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		ops = &clk_pllv3_av_ops;
 		break;
 	case IMX_PLLV3_ENET_IMX7:
-		pll->powerdown = IMX7_ENET_PLL_POWER;
+		pll->power_bit = IMX7_ENET_PLL_POWER;
 		pll->ref_clock = 1000000000;
 		ops = &clk_pllv3_enet_ops;
 		break;

-- 
1.9.1

On Mon, Jun 13, 2016 at 08:24:52PM +0800, Dong Aisheng wrote:

The powerdown bit is a bit confused, let's change it to power_bit
to relfect both powerdown and powerup case according to different
plls.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

Applied, thanks.

On Mon, Jun 13, 2016 at 03:38:30PM +0800, Dong Aisheng wrote:

fix gpt2 clock names

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

Applied, thanks.