[PATCH V4 4/5] clk: imx: support fracn gppll

[Peng Fan]

> Subject: Re: [PATCH V4 4/5] clk: imx: support fracn gppll
> 
> On Thu, Feb 24, 2022 at 04:22:50PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan at nxp.com>
> >
> > This PLL module is a Fractional-N synthesizer, supporting 30-bit
> > numerator and denominator. Numerator is a signed number. It has
> > feature to adjust fractional portion of feedback divider dynamically.
> > This fracn gppll is used in i.MX93.
> >
> > Reviewed-by: Abel Vesa <abel.vesa at nxp.com>
> > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > ---
> >  drivers/clk/imx/Makefile          |   1 +
> >  drivers/clk/imx/clk-fracn-gppll.c | 326
> ++++++++++++++++++++++++++++++
> >  drivers/clk/imx/clk.h             |  21 ++
> >  3 files changed, 348 insertions(+)
> >  create mode 100644 drivers/clk/imx/clk-fracn-gppll.c
> >
> > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index
> > 36c04922d789..60c8a4bb7574 100644
> > --- a/drivers/clk/imx/Makefile
> > +++ b/drivers/clk/imx/Makefile
> > @@ -5,6 +5,7 @@ mxc-clk-objs += clk-busy.o  mxc-clk-objs +=
> > clk-composite-7ulp.o  mxc-clk-objs += clk-composite-8m.o  mxc-clk-objs
> > += clk-composite-93.o
> > +mxc-clk-objs += clk-fracn-gppll.o
> >  mxc-clk-objs += clk-cpu.o
> >  mxc-clk-objs += clk-divider-gate.o
> >  mxc-clk-objs += clk-fixup-div.o
> > diff --git a/drivers/clk/imx/clk-fracn-gppll.c
> > b/drivers/clk/imx/clk-fracn-gppll.c
> > new file mode 100644
> > index 000000000000..b6add3a2e472
> > --- /dev/null
> > +++ b/drivers/clk/imx/clk-fracn-gppll.c
> > @@ -0,0 +1,326 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2021 NXP
> > + */
> > +
> > +#include <asm/div64.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/err.h>
> > +#include <linux/export.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/slab.h>
> > +#include <linux/jiffies.h>
> > +
> > +#include "clk.h"
> > +
> > +#define PLL_CTRL		0x0
> > +#define CLKMUX_BYPASS		BIT(2)
> > +#define CLKMUX_EN		BIT(1)
> > +#define POWERUP_MASK		BIT(0)
> > +
> > +#define PLL_ANA_PRG		0x10
> > +#define PLL_SPREAD_SPECTRUM	0x30
> > +
> > +#define PLL_NUMERATOR		0x40
> > +#define PLL_MFN_MASK		GENMASK(31, 2)
> > +#define PLL_MFN_SHIFT		2
> > +
> > +#define PLL_DENOMINATOR		0x50
> > +#define PLL_MFD_MASK		GENMASK(29, 0)
> > +
> > +#define PLL_DIV			0x60
> > +#define PLL_MFI_MASK		GENMASK(24, 16)
> 
> Is the MFI field really 9 bits wide? Could be, I don't have the reference manual.
> It just looks like it could be a one-off.

Yes, 9 bits wide. MFI: 24-16
Integer portion of loop divider
Sets the value of the divider in the PLL feedback loop, and the value specified establishes the
multiplication factor applied to the reference frequency. Divider value = MFI, where the chosen MFI does
not violate VCO frequency specifications.

> 
> > +#define PLL_MFI_SHIFT		16
> > +#define PLL_RDIV_MASK		GENMASK(15, 13)
> > +#define PLL_RDIV_SHIFT		13
> > +#define PLL_ODIV_MASK		GENMASK(7, 0)
> 
> The *_SHIFT defines can be removed.

ok

> 
> > +static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
> > +				    unsigned long prate)
> > +{
> > +	struct clk_fracn_gppll *pll = to_clk_fracn_gppll(hw);
> > +	const struct imx_fracn_gppll_rate_table *rate;
> > +	u32 tmp, pll_div, ana_mfn;
> > +	int ret;
> > +
> > +	rate = imx_get_pll_settings(pll, drate);
> > +
> > +	/* Disable output */
> > +	tmp = readl_relaxed(pll->base + PLL_CTRL);
> > +	tmp &= ~CLKMUX_EN;
> > +	writel_relaxed(tmp, pll->base + PLL_CTRL);
> > +
> > +	/* Power Down */
> > +	tmp &= ~POWERUP_MASK;
> > +	writel_relaxed(tmp, pll->base + PLL_CTRL);
> > +
> > +	/* Disable BYPASS */
> > +	tmp &= ~CLKMUX_BYPASS;
> > +	writel_relaxed(tmp, pll->base + PLL_CTRL);
> > +
> > +	pll_div = FIELD_PREP(PLL_RDIV_MASK, rate->rdiv) | rate->odiv |
> > +		FIELD_PREP(PLL_MFI_MASK, rate->mfi);
> > +	writel_relaxed(pll_div, pll->base + PLL_DIV);
> > +	writel_relaxed(rate->mfd, pll->base + PLL_DENOMINATOR);
> > +	writel_relaxed(rate->mfn << PLL_MFN_SHIFT, pll->base +
> > +PLL_NUMERATOR);
> 
> FIELD_PREP

Fix in V5.

> 
> > +
> > +	/* Wait for 5us according to fracn mode pll doc */
> > +	udelay(5);
> > +
> > +	/* Enable Powerup */
> > +	tmp |= POWERUP_MASK;
> > +	writel_relaxed(tmp, pll->base + PLL_CTRL);
> > +
> > +	/* Wait Lock */
> > +	ret = clk_fracn_gppll_wait_lock(pll);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Enable output */
> > +	tmp |= CLKMUX_EN;
> > +	writel_relaxed(tmp, pll->base + PLL_CTRL);
> > +
> > +	ana_mfn = (readl_relaxed(pll->base + PLL_STATUS) &
> PLL_MFN_MASK) >>
> > +PLL_MFN_SHIFT;
> 
> FIELD_GET

Fix in V5.

Thanks,
Peng.

> 
> Sascha
> 
> --
> Pengutronix e.K.                           |
> |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
> engutronix.de%2F&data=04%7C01%7Cpeng.fan%40nxp.com%7C4dc747
> 31b1f34e99946108d9f77c488f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C637812935432676998%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00&sdata=0MkuXWdHlLdazu63dgaTlqJdf1RJKwZcSeI2dnE0Uho%3D&am
> p;reserved=0  |
> 31137 Hildesheim, Germany                  | Phone:
> +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |