From: Aisheng Dong <aisheng.dong@nxp.com>
To: Anson Huang <anson.huang@nxp.com>,
"mturquette@baylibre.com" <mturquette@baylibre.com>,
"sboyd@kernel.org" <sboyd@kernel.org>,
"shawnguo@kernel.org" <shawnguo@kernel.org>,
"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
"kernel@pengutronix.de" <kernel@pengutronix.de>,
"festevam@gmail.com" <festevam@gmail.com>,
"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: dl-linux-imx <linux-imx@nxp.com>
Subject: RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
Date: Mon, 29 Apr 2019 11:28:21 +0000 [thread overview]
Message-ID: <AM0PR04MB4211D3F29BCA3727E799005F80390@AM0PR04MB4211.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <1556507637-22847-1-git-send-email-Anson.Huang@nxp.com>
> From: Anson Huang
> Sent: Monday, April 29, 2019 11:19 AM
>
clk: imx: pllv4: add fractional-N pll support
> The pllv4 supports fractional-N function, the formula is:
>
> PLL output freq = input * (mult + num/denom),
>
> This patch adds fractional-N function support, including clock round rate,
> calculate rate and set rate, with this patch, the clock rate of APLL in clock tree
> is more accurate than before:
>
> Without fraction:
> apll_pre_sel 1 1 1 24000000
> 0 0 50000
> apll_pre_div 1 1 2 24000000
> 0 0 50000
> apll 1 1 2 528000000
> 0 0 50000
> apll_pfd3 0 0 0 792000000
> 0 0 50000
> apll_pfd2 0 0 0 339428571
> 0 0 50000
> apll_pfd1 0 0 0 352000000
> 0 0 50000
> usdhc0 0 0 0 352000000
> 0 0 50000
> apll_pfd0 1 1 1 352000000
> 0 0 50000
>
> With fraction:
> apll_pre_sel 1 1 1 24000000
> 0 0 50000
> apll_pre_div 1 1 2 24000000
> 0 0 50000
> apll 1 1 2 529200000
> 0 0 50000
> apll_pfd3 0 0 0 793800000
> 0 0 50000
> apll_pfd2 0 0 0 340200000
> 0 0 50000
> apll_pfd1 0 0 0 352800000
> 0 0 50000
> usdhc0 0 0 0 352800000
> 0 0 50000
> apll_pfd0 1 1 1 352800000
> 0 0 50000
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> drivers/clk/imx/clk-pllv4.c | 68
> +++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c index
> d38bc9f..4ced5ca 100644
> --- a/drivers/clk/imx/clk-pllv4.c
> +++ b/drivers/clk/imx/clk-pllv4.c
> @@ -64,13 +64,18 @@ static unsigned long clk_pllv4_recalc_rate(struct
> clk_hw *hw,
> unsigned long parent_rate)
> {
> struct clk_pllv4 *pll = to_clk_pllv4(hw);
> - u32 div;
> + u32 mult = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> + u32 mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
> + u32 mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
Nitpick:
We usually don't write code like this.
How about separate the assignment from declaration?
> + u64 temp64 = parent_rate;
>
> - div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> - div &= BM_PLL_MULT;
> - div >>= BP_PLL_MULT;
> + mult &= BM_PLL_MULT;
> + mult >>= BP_PLL_MULT;
>
> - return parent_rate * div;
> + temp64 *= mfn;
> + do_div(temp64, mfd);
> +
> + return (parent_rate * mult) + (u32)temp64;
> }
>
> static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate, @@
> -78,14 +83,47 @@ static long clk_pllv4_round_rate(struct clk_hw *hw,
> unsigned long rate, {
> unsigned long parent_rate = *prate;
> unsigned long round_rate, i;
> + bool found = false;
> + u32 mfn, mfd = 1000000;
> + u32 max_mfd = 0x3FFFFFFF;
Please keep sort from long to short.
And the multi Max_mfd definitions could be move out the function and
Defined use macro.
> + u64 temp64;
>
> for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> round_rate = parent_rate * pllv4_mult_table[i];
> - if (rate >= round_rate)
> - return round_rate;
> + if (rate >= round_rate) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found) {
> + pr_warn("%s: unable to round rate %lu, parent rate %lu\n",
> + clk_hw_get_name(hw), rate, parent_rate);
> + return 0;
> }
>
> - return round_rate;
> + if (parent_rate <= max_mfd)
> + mfd = parent_rate;
> +
> + temp64 = (u64)(rate - round_rate);
> + temp64 *= mfd;
> + do_div(temp64, parent_rate);
> + mfn = temp64;
> +
> + /*
> + * NOTE: The value of numerator must always be configured to be
> + * less than the value of the denominator. If we can't get a proper
> + * pair of mfn/mfd, we simply return the round_rate without using
> + * the frac part.
> + */
> + if (mfn >= mfd)
> + return round_rate;
> +
> + temp64 = (u64)parent_rate;
> + temp64 *= mfn;
> + do_div(temp64, mfd);
> +
> + return round_rate + (u32)temp64;
> }
>
> static bool clk_pllv4_is_valid_mult(unsigned int mult) @@ -106,17 +144,31
> @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate, {
> struct clk_pllv4 *pll = to_clk_pllv4(hw);
> u32 val, mult;
> + u32 mfn, mfd = 1000000;
> + u32 max_mfd = 0x3FFFFFFF;
Ditto
Otherwise:
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Dong Aisheng
> + u64 temp64;
>
> mult = rate / parent_rate;
>
> if (!clk_pllv4_is_valid_mult(mult))
> return -EINVAL;
>
> + if (parent_rate <= max_mfd)
> + mfd = parent_rate;
> +
> + temp64 = (u64)(rate - mult * parent_rate);
> + temp64 *= mfd;
> + do_div(temp64, parent_rate);
> + mfn = temp64;
> +
> 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(mfn, pll->base + PLL_NUM_OFFSET);
> + writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
> +
> return 0;
> }
>
> --
> 2.7.4
next prev parent reply other threads:[~2019-04-29 11:30 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-29 3:18 [PATCH] clk: imx: add fractional-N pll support to pllv4 Anson Huang
2019-04-29 11:28 ` Aisheng Dong [this message]
2019-04-29 11:35 ` Aisheng Dong
2019-04-29 12:33 ` Anson Huang
2019-04-29 12:34 ` Anson Huang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AM0PR04MB4211D3F29BCA3727E799005F80390@AM0PR04MB4211.eurprd04.prod.outlook.com \
--to=aisheng.dong@nxp.com \
--cc=anson.huang@nxp.com \
--cc=festevam@gmail.com \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=s.hauer@pengutronix.de \
--cc=sboyd@kernel.org \
--cc=shawnguo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).