RE: [PATCH 1/2] clk: imx8mm: Move 1443X/1416X PLL clock structure to common place

[Anson Huang <anson.huang@nxp.com>]

Hi, Leonard

> On 05.09.2019 12:59, Anson Huang wrote:
> > Many i.MX8M SoCs use same 1443X/1416X PLL, such as i.MX8MM,
> i.MX8MN
> > and later i.MX8M SoCs, moving these PLL definitions to common place
> > can save a lot of duplicated code on each platform.
> 
> There are lots of similarities between imx8m clocks, do you plan to do
> combine them further?

I will consider it later, maybe we can create a new clock file named clk-imx8m.c
as common clock for i.MX8M SoCs which are similar.

> 
> > Meanwhile, no need to define PLL clock structure for every module
> > which uses same type of PLL, e.g., audio/video/dram use 1443X PLL,
> > arm/gpu/vpu/sys use 1416X PLL, define 2 PLL clock structure for each
> > group is enough.
> 
> > diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
> 
> > +const struct imx_pll14xx_rate_table imx_pll1416x_tbl[] = {
> > +	PLL_1416X_RATE(1800000000U, 225, 3, 0),
> > +	PLL_1416X_RATE(1600000000U, 200, 3, 0),
> > +	PLL_1416X_RATE(1200000000U, 300, 3, 1),
> > +	PLL_1416X_RATE(1000000000U, 250, 3, 1),
> > +	PLL_1416X_RATE(800000000U,  200, 3, 1),
> > +	PLL_1416X_RATE(750000000U,  250, 2, 2),
> > +	PLL_1416X_RATE(700000000U,  350, 3, 2),
> > +	PLL_1416X_RATE(600000000U,  300, 3, 2), };
> > +
> > +const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
> > +	PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> > +	PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
> > +	PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
> > +	PLL_1443X_RATE(361267200U, 361, 3, 3, 17511), };
> > +
> > +struct imx_pll14xx_clk imx_1443x_pll = {
> > +	.type = PLL_1443X,
> > +	.rate_table = imx_pll1443x_tbl,
> > +	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl), };
> > +
> > +struct imx_pll14xx_clk imx_1416x_pll = {
> > +	.type = PLL_1416X,
> > +	.rate_table = imx_pll1416x_tbl,
> > +	.rate_count = ARRAY_SIZE(imx_pll1416x_tbl), };
> 
> Perhaps these consts should be in clk-pll14xx.c? That way they won't be
> compiled for imx6 as well.

Make sense, I will do it in V2.

Thanks,
Anson