Re: [PATCH v3 3/4] clk: imx8m: reduce rate table duplication

2022-03-14 Thread Marek Vasut

On 3/14/22 20:15, Angus Ainslie wrote:

On 2022-03-14 11:57, Marek Vasut wrote:

On 3/14/22 19:22, Angus Ainslie wrote:

Re-factor the imx8m[nmpq] rate tables into the common pll1416x clock
driver.

43cdaa1567ad3 ("clk: imx8mm: Move 1443X/1416X PLL clock structure to 
common place")


Signed-off-by: Angus Ainslie 


Thanks

[...]


 imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel",
-   base + 0x114, _sys_pll));
+   base + 0x114, _1416x_pll));


imx8m_pll1416x , since those samsung PLL14xxx are new on MX8M I think.



For the kernel driver it uses the generic ones ?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/imx/clk-imx8mm.c#n346 


You could try sending a patch to kernel too, since I don't think I saw 
these samsung plls in iMX before iMX8M.


But then, if you want to keep the naming consistent between linux and 
u-boot, that's fine by me.


Re: [PATCH v3 3/4] clk: imx8m: reduce rate table duplication

2022-03-14 Thread Angus Ainslie

On 2022-03-14 11:57, Marek Vasut wrote:

On 3/14/22 19:22, Angus Ainslie wrote:

Re-factor the imx8m[nmpq] rate tables into the common pll1416x clock
driver.

43cdaa1567ad3 ("clk: imx8mm: Move 1443X/1416X PLL clock structure to 
common place")


Signed-off-by: Angus Ainslie 


Thanks

[...]


   imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel",
-  base + 0x114, _sys_pll));
+  base + 0x114, _1416x_pll));


imx8m_pll1416x , since those samsung PLL14xxx are new on MX8M I think.



For the kernel driver it uses the generic ones ?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/imx/clk-imx8mm.c#n346


[...]

diff --git a/drivers/clk/imx/clk-pll14xx.c 
b/drivers/clk/imx/clk-pll14xx.c

index b0ccb6c8eda..a60cf9bdcb0 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -52,6 +52,50 @@ struct clk_pll14xx {
#define to_clk_pll14xx(_clk) container_of(_clk, struct 
clk_pll14xx, clk)

  +static const struct imx_pll14xx_rate_table imx_pll1416x_tbl[] = {
+   PLL_1416X_RATE(18U, 225, 3, 0),
+   PLL_1416X_RATE(16U, 200, 3, 0),
+   PLL_1416X_RATE(15U, 375, 3, 1),
+   PLL_1416X_RATE(14U, 350, 3, 1),
+   PLL_1416X_RATE(12U, 300, 3, 1),
+   PLL_1416X_RATE(10U, 250, 3, 1),
+   PLL_1416X_RATE(8U,  200, 3, 1),
+   PLL_1416X_RATE(75000U,  250, 2, 2),
+   PLL_1416X_RATE(7U,  350, 3, 2),
+   PLL_1416X_RATE(6U,  300, 3, 2),
+};
+
+const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
+   PLL_1443X_RATE(103950U, 173, 2, 1, 16384),
+   PLL_1443X_RATE(65000U, 325, 3, 2, 0),
+   PLL_1443X_RATE(59400U, 198, 2, 2, 0),
+   PLL_1443X_RATE(51975U, 173, 2, 2, 16384),
+   PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
+   PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
+};
+
+struct imx_pll14xx_clk imx_1443x_pll __initdata = {
+   .type = PLL_1443X,
+   .rate_table = imx_pll1443x_tbl,
+   .rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
+};
+EXPORT_SYMBOL_GPL(imx_1443x_pll);
+
+struct imx_pll14xx_clk imx_1443x_dram_pll __initdata = {
+   .type = PLL_1443X,
+   .rate_table = imx_pll1443x_tbl,
+   .rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
+   .flags = CLK_GET_RATE_NOCACHE,
+};
+EXPORT_SYMBOL_GPL(imx_1443x_dram_pll);
+
+struct imx_pll14xx_clk imx_1416x_pll __initdata = {
+   .type = PLL_1416X,
+   .rate_table = imx_pll1416x_tbl,
+   .rate_count = ARRAY_SIZE(imx_pll1416x_tbl),
+};
+EXPORT_SYMBOL_GPL(imx_1416x_pll);
+
  static const struct imx_pll14xx_rate_table *imx_get_pll_settings(
struct clk_pll14xx *pll, unsigned long rate)
  {
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 60f287046b9..ac7e50fbe97 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -41,6 +41,27 @@ struct imx_pll14xx_clk {
int flags;
  };
  +extern struct imx_pll14xx_clk imx_1416x_pll;
+extern struct imx_pll14xx_clk imx_1443x_pll;
+extern struct imx_pll14xx_clk imx_1443x_dram_pll;
+
+#define PLL_1416X_RATE(_rate, _m, _p, _s)  \
+   {   \
+   .rate   =   (_rate),\
+   .mdiv   =   (_m),   \
+   .pdiv   =   (_p),   \
+   .sdiv   =   (_s),   \
+   }
+
+#define PLL_1443X_RATE(_rate, _m, _p, _s, _k)  \
+   {   \
+   .rate   =   (_rate),\
+   .mdiv   =   (_m),   \
+   .pdiv   =   (_p),   \
+   .sdiv   =   (_s),   \
+   .kdiv   =   (_k),   \
+   }
+


Should these macros be in drivers/clk/imx/clk-pll14xx.c ?


Yeah I guess they aren't used anywhere else.


Re: [PATCH v3 3/4] clk: imx8m: reduce rate table duplication

2022-03-14 Thread Marek Vasut

On 3/14/22 19:22, Angus Ainslie wrote:

Re-factor the imx8m[nmpq] rate tables into the common pll1416x clock
driver.

43cdaa1567ad3 ("clk: imx8mm: Move 1443X/1416X PLL clock structure to common 
place")

Signed-off-by: Angus Ainslie 


Thanks

[...]


   imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel",
-  base + 0x114, _sys_pll));
+  base + 0x114, _1416x_pll));


imx8m_pll1416x , since those samsung PLL14xxx are new on MX8M I think.

[...]


diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index b0ccb6c8eda..a60cf9bdcb0 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -52,6 +52,50 @@ struct clk_pll14xx {
  
  #define to_clk_pll14xx(_clk) container_of(_clk, struct clk_pll14xx, clk)
  
+static const struct imx_pll14xx_rate_table imx_pll1416x_tbl[] = {

+   PLL_1416X_RATE(18U, 225, 3, 0),
+   PLL_1416X_RATE(16U, 200, 3, 0),
+   PLL_1416X_RATE(15U, 375, 3, 1),
+   PLL_1416X_RATE(14U, 350, 3, 1),
+   PLL_1416X_RATE(12U, 300, 3, 1),
+   PLL_1416X_RATE(10U, 250, 3, 1),
+   PLL_1416X_RATE(8U,  200, 3, 1),
+   PLL_1416X_RATE(75000U,  250, 2, 2),
+   PLL_1416X_RATE(7U,  350, 3, 2),
+   PLL_1416X_RATE(6U,  300, 3, 2),
+};
+
+const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
+   PLL_1443X_RATE(103950U, 173, 2, 1, 16384),
+   PLL_1443X_RATE(65000U, 325, 3, 2, 0),
+   PLL_1443X_RATE(59400U, 198, 2, 2, 0),
+   PLL_1443X_RATE(51975U, 173, 2, 2, 16384),
+   PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
+   PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
+};
+
+struct imx_pll14xx_clk imx_1443x_pll __initdata = {
+   .type = PLL_1443X,
+   .rate_table = imx_pll1443x_tbl,
+   .rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
+};
+EXPORT_SYMBOL_GPL(imx_1443x_pll);
+
+struct imx_pll14xx_clk imx_1443x_dram_pll __initdata = {
+   .type = PLL_1443X,
+   .rate_table = imx_pll1443x_tbl,
+   .rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
+   .flags = CLK_GET_RATE_NOCACHE,
+};
+EXPORT_SYMBOL_GPL(imx_1443x_dram_pll);
+
+struct imx_pll14xx_clk imx_1416x_pll __initdata = {
+   .type = PLL_1416X,
+   .rate_table = imx_pll1416x_tbl,
+   .rate_count = ARRAY_SIZE(imx_pll1416x_tbl),
+};
+EXPORT_SYMBOL_GPL(imx_1416x_pll);
+
  static const struct imx_pll14xx_rate_table *imx_get_pll_settings(
struct clk_pll14xx *pll, unsigned long rate)
  {
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 60f287046b9..ac7e50fbe97 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -41,6 +41,27 @@ struct imx_pll14xx_clk {
int flags;
  };
  
+extern struct imx_pll14xx_clk imx_1416x_pll;

+extern struct imx_pll14xx_clk imx_1443x_pll;
+extern struct imx_pll14xx_clk imx_1443x_dram_pll;
+
+#define PLL_1416X_RATE(_rate, _m, _p, _s)  \
+   {   \
+   .rate   =   (_rate),\
+   .mdiv   =   (_m),   \
+   .pdiv   =   (_p),   \
+   .sdiv   =   (_s),   \
+   }
+
+#define PLL_1443X_RATE(_rate, _m, _p, _s, _k)  \
+   {   \
+   .rate   =   (_rate),\
+   .mdiv   =   (_m),   \
+   .pdiv   =   (_p),   \
+   .sdiv   =   (_s),   \
+   .kdiv   =   (_k),   \
+   }
+


Should these macros be in drivers/clk/imx/clk-pll14xx.c ?