Re: [PATCH v3 2/9] clk: imx: Update the pllv4 to support imx8ulp

[Abel Vesa <abel.vesa@nxp.com>]

On 21-09-14 14:52:01, Jacky Bai wrote:
> The PLLs used on i.MX8ULP is mostly the same as on i.MX7ULP,
> except the PLL register offset is changed. Change the PLLv4
> driver for code reuse on i.MX7ULP and i.MX8ULP.
> 
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>

I'm OK with this one. But maybe later on we will have two separate
wrappers in clk.h called imx_clk_hw_pllv4_7ulp and
imx_clk_hw_pllv4_8ulp. We'll see. 

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>   v3 changes: no
> ---
>  drivers/clk/imx/clk-imx7ulp.c |  4 ++--
>  drivers/clk/imx/clk-pllv4.c   | 34 +++++++++++++++++++++++++---------
>  drivers/clk/imx/clk.h         |  9 +++++++--
>  3 files changed, 34 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx7ulp.c b/drivers/clk/imx/clk-imx7ulp.c
> index 779e09105da7..ba50d6db8097 100644
> --- a/drivers/clk/imx/clk-imx7ulp.c
> +++ b/drivers/clk/imx/clk-imx7ulp.c
> @@ -78,8 +78,8 @@ static void __init imx7ulp_clk_scg1_init(struct device_node *np)
>  	hws[IMX7ULP_CLK_SPLL_PRE_DIV]	= imx_clk_hw_divider_flags("spll_pre_div", "spll_pre_sel", base + 0x608,	8,	3,	CLK_SET_RATE_GATE);
>  
>  	/*						name	 parent_name	 base */
> -	hws[IMX7ULP_CLK_APLL]		= imx_clk_hw_pllv4("apll",  "apll_pre_div", base + 0x500);
> -	hws[IMX7ULP_CLK_SPLL]		= imx_clk_hw_pllv4("spll",  "spll_pre_div", base + 0x600);
> +	hws[IMX7ULP_CLK_APLL]		= imx_clk_hw_pllv4(IMX_PLLV4_IMX7ULP, "apll",  "apll_pre_div", base + 0x500);
> +	hws[IMX7ULP_CLK_SPLL]		= imx_clk_hw_pllv4(IMX_PLLV4_IMX7ULP, "spll",  "spll_pre_div", base + 0x600);
>  
>  	/* APLL PFDs */
>  	hws[IMX7ULP_CLK_APLL_PFD0]	= imx_clk_hw_pfdv2("apll_pfd0", "apll", base + 0x50c, 0);
> diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
> index 8ec703f27417..3c750ccbee25 100644
> --- a/drivers/clk/imx/clk-pllv4.c
> +++ b/drivers/clk/imx/clk-pllv4.c
> @@ -23,14 +23,17 @@
>  
>  /* PLL Configuration Register (xPLLCFG) */
>  #define PLL_CFG_OFFSET		0x08
> +#define IMX8ULP_PLL_CFG_OFFSET	0x10
>  #define BP_PLL_MULT		16
>  #define BM_PLL_MULT		(0x7f << 16)
>  
>  /* PLL Numerator Register (xPLLNUM) */
>  #define PLL_NUM_OFFSET		0x10
> +#define IMX8ULP_PLL_NUM_OFFSET	0x1c
>  
>  /* PLL Denominator Register (xPLLDENOM) */
>  #define PLL_DENOM_OFFSET	0x14
> +#define IMX8ULP_PLL_DENOM_OFFSET	0x18
>  
>  #define MAX_MFD			0x3fffffff
>  #define DEFAULT_MFD		1000000
> @@ -38,6 +41,9 @@
>  struct clk_pllv4 {
>  	struct clk_hw	hw;
>  	void __iomem	*base;
> +	u32		cfg_offset;
> +	u32		num_offset;
> +	u32		denom_offset;
>  };
>  
>  /* Valid PLL MULT Table */
> @@ -72,12 +78,12 @@ static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
>  	u32 mult, mfn, mfd;
>  	u64 temp64;
>  
> -	mult = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> +	mult = readl_relaxed(pll->base + pll->cfg_offset);
>  	mult &= BM_PLL_MULT;
>  	mult >>= BP_PLL_MULT;
>  
> -	mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
> -	mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
> +	mfn = readl_relaxed(pll->base + pll->num_offset);
> +	mfd = readl_relaxed(pll->base + pll->denom_offset);
>  	temp64 = parent_rate;
>  	temp64 *= mfn;
>  	do_div(temp64, mfd);
> @@ -165,13 +171,13 @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,
>  	do_div(temp64, parent_rate);
>  	mfn = temp64;
>  
> -	val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> +	val = readl_relaxed(pll->base + pll->cfg_offset);
>  	val &= ~BM_PLL_MULT;
>  	val |= mult << BP_PLL_MULT;
> -	writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
> +	writel_relaxed(val, pll->base + pll->cfg_offset);
>  
> -	writel_relaxed(mfn, pll->base + PLL_NUM_OFFSET);
> -	writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
> +	writel_relaxed(mfn, pll->base + pll->num_offset);
> +	writel_relaxed(mfd, pll->base + pll->denom_offset);
>  
>  	return 0;
>  }
> @@ -207,8 +213,8 @@ static const struct clk_ops clk_pllv4_ops = {
>  	.is_prepared	= clk_pllv4_is_prepared,
>  };
>  
> -struct clk_hw *imx_clk_hw_pllv4(const char *name, const char *parent_name,
> -			  void __iomem *base)
> +struct clk_hw *imx_clk_hw_pllv4(enum imx_pllv4_type type, const char *name,
> +		 const char *parent_name, void __iomem *base)
>  {
>  	struct clk_pllv4 *pll;
>  	struct clk_hw *hw;
> @@ -221,6 +227,16 @@ struct clk_hw *imx_clk_hw_pllv4(const char *name, const char *parent_name,
>  
>  	pll->base = base;
>  
> +	if (type == IMX_PLLV4_IMX8ULP) {
> +		pll->cfg_offset = IMX8ULP_PLL_CFG_OFFSET;
> +		pll->num_offset = IMX8ULP_PLL_NUM_OFFSET;
> +		pll->denom_offset = IMX8ULP_PLL_DENOM_OFFSET;
> +	} else {
> +		pll->cfg_offset = PLL_CFG_OFFSET;
> +		pll->num_offset = PLL_NUM_OFFSET;
> +		pll->denom_offset = PLL_DENOM_OFFSET;
> +	}
> +
>  	init.name = name;
>  	init.ops = &clk_pllv4_ops;
>  	init.parent_names = &parent_name;
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> index e144f983fd8c..3f518559b8f9 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -42,6 +42,11 @@ enum imx_pll14xx_type {
>  	PLL_1443X,
>  };
>  
> +enum imx_pllv4_type {
> +	IMX_PLLV4_IMX7ULP,
> +	IMX_PLLV4_IMX8ULP,
> +};
> +
>  /* NOTE: Rate table should be kept sorted in descending order. */
>  struct imx_pll14xx_rate_table {
>  	unsigned int rate;
> @@ -191,8 +196,8 @@ struct clk_hw *imx_clk_hw_pllv3(enum imx_pllv3_type type, const char *name,
>  		.kdiv	=	(_k),			\
>  	}
>  
> -struct clk_hw *imx_clk_hw_pllv4(const char *name, const char *parent_name,
> -			     void __iomem *base);
> +struct clk_hw *imx_clk_hw_pllv4(enum imx_pllv4_type type, const char *name,
> +		const char *parent_name, void __iomem *base);
>  
>  struct clk_hw *clk_hw_register_gate2(struct device *dev, const char *name,
>  		const char *parent_name, unsigned long flags,
> -- 
> 2.26.2
>