| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AM0PR04MB4211D3F29BCA3727E799005F80390@AM0PR04MB4211.eurprd04.prod.outlook.com>
Date: Mon, 29 Apr 2019 11:28:21 +0000
From: Aisheng Dong <aisheng.dong@....com>
To: Anson Huang <anson.huang@....com>,
"mturquette@...libre.com" <mturquette@...libre.com>,
"sboyd@...nel.org" <sboyd@...nel.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
> 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@....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@....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
Powered by blists - more mailing lists