Re: [PATCH v2 3/4] clk: imx: pll14xx: support spread spectrum clock generation

Dario Binacchi
2025-02-05T11:19:50Z

Hi Peng,

On Wed, Feb 5, 2025 at 10:51 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> Add support for spread spectrum clock (SSC) generation to the pll14xxx
> driver.
>
> Co-developed-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

It doesn’t seem right to me.
You can’t take 90% of my patch, where the SSC management was actually
implemented,
add 10%, and consider yourself the author of the patch.
Please correct it in version 3.

Thanks and regards,
Dario

> ---
> drivers/clk/imx/clk-pll14xx.c | 66 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index f290981ea13bdba3602af7aa44aaadfe0b78dcf9..3bdce762a9d651a6fb048dcbf58db396af9d3aaf 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -20,6 +20,8 @@
> #define GNRL_CTL 0x0
> #define DIV_CTL0 0x4
> #define DIV_CTL1 0x8
> +#define SSCG_CTRL 0xc
> +
> #define LOCK_STATUS BIT(31)
> #define LOCK_SEL_MASK BIT(29)
> #define CLKE_MASK BIT(11)
> @@ -31,15 +33,26 @@
> #define KDIV_MASK GENMASK(15, 0)
> #define KDIV_MIN SHRT_MIN
> #define KDIV_MAX SHRT_MAX
> +#define SSCG_ENABLE BIT(31)
> +#define MFREQ_CTL_MASK GENMASK(19, 12)
> +#define MRAT_CTL_MASK GENMASK(9, 4)
> +#define SEL_PF_MASK GENMASK(1, 0)
>
> #define LOCK_TIMEOUT_US 10000
>
> +enum imx_pll14xx_ssc_mod_type {
> + IMX_PLL14XX_SSC_DOWN_SPREAD,
> + IMX_PLL14XX_SSC_UP_SPREAD,
> + IMX_PLL14XX_SSC_CENTER_SPREAD,
> +};
> +
> struct clk_pll14xx {
> struct clk_hw hw;
> void __iomem *base;
> enum imx_pll14xx_type type;
> const struct imx_pll14xx_rate_table *rate_table;
> int rate_count;
> + struct clk_spread_spectrum ssc_conf;
> };
>
> #define to_clk_pll14xx(_hw) container_of(_hw, struct clk_pll14xx, hw)
> @@ -349,6 +362,42 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
> return 0;
> }
>
> +static void clk_pll1443x_enable_ssc(struct clk_hw *hw, unsigned long parent_rate,
> + unsigned int pdiv, unsigned int mdiv)
> +{
> + struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> + struct clk_spread_spectrum *conf = &pll->ssc_conf;
> + u32 sscg_ctrl, mfr, mrr, mod_type;
> +
> + sscg_ctrl = readl_relaxed(pll->base + SSCG_CTRL);
> + sscg_ctrl &=
> + ~(SSCG_ENABLE | MFREQ_CTL_MASK | MRAT_CTL_MASK | SEL_PF_MASK);
> +
> + mfr = parent_rate / (conf->modfreq * pdiv * (1 << 5));
> + mrr = ((conf->spreaddepth / 100) * mdiv * (1 << 6)) / (100 * mfr);
> +
> + switch (conf->method) {
> + case CLK_SSC_CENTER_SPREAD:
> + mod_type = IMX_PLL14XX_SSC_CENTER_SPREAD;
> + break;
> + case CLK_SSC_UP_SPREAD:
> + mod_type = IMX_PLL14XX_SSC_UP_SPREAD;
> + break;
> + case CLK_SSC_DOWN_SPREAD:
> + mod_type = IMX_PLL14XX_SSC_DOWN_SPREAD;
> + break;
> + default:
> + mod_type = IMX_PLL14XX_SSC_DOWN_SPREAD;
> + break;
> + }
> +
> + sscg_ctrl |= SSCG_ENABLE | FIELD_PREP(MFREQ_CTL_MASK, mfr) |
> + FIELD_PREP(MRAT_CTL_MASK, mrr) |
> + FIELD_PREP(SEL_PF_MASK, mod_type);
> +
> + writel_relaxed(sscg_ctrl, pll->base + SSCG_CTRL);
> +}
> +
> static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> unsigned long prate)
> {
> @@ -370,6 +419,9 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv),
> pll->base + DIV_CTL1);
>
> + if (pll->ssc_conf.enable)
> + clk_pll1443x_enable_ssc(hw, prate, rate.pdiv, rate.mdiv);
> +
> return 0;
> }
>
> @@ -410,6 +462,9 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> gnrl_ctl &= ~BYPASS_MASK;
> writel_relaxed(gnrl_ctl, pll->base + GNRL_CTL);
>
> + if (pll->ssc_conf.enable)
> + clk_pll1443x_enable_ssc(hw, prate, rate.pdiv, rate.mdiv);
> +
> return 0;
> }
>
> @@ -465,6 +520,16 @@ static void clk_pll14xx_unprepare(struct clk_hw *hw)
> writel_relaxed(val, pll->base + GNRL_CTL);
> }
>
> +static int clk_pll1443x_set_spread_spectrum(struct clk_hw *hw,
> + struct clk_spread_spectrum *clk_ss)
> +{
> + struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> +
> + memcpy(&pll->ssc_conf, clk_ss, sizeof(pll->ssc_conf));
> +
> + return 0;
> +}
> +
> static const struct clk_ops clk_pll1416x_ops = {
> .prepare = clk_pll14xx_prepare,
> .unprepare = clk_pll14xx_unprepare,
> @@ -485,6 +550,7 @@ static const struct clk_ops clk_pll1443x_ops = {
> .recalc_rate = clk_pll14xx_recalc_rate,
> .round_rate = clk_pll1443x_round_rate,
> .set_rate = clk_pll1443x_set_rate,
> + .set_spread_spectrum = clk_pll1443x_set_spread_spectrum,
> };
>
> struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
>
> --
> 2.37.1
>

--

Dario Binacchi

Senior Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________

Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com