linux-kernel - Re: [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161213073858.GO5423@codeaurora.org>
Date:   Mon, 12 Dec 2016 23:38:58 -0800
From:   Stephen Boyd <sboyd@...eaurora.org>
To:     Nikita Yushchenko <nikita.yoush@...entembedded.com>
Cc:     Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        Michael Turquette <mturquette@...libre.com>,
        linux-clk@...r.kernel.org, Chris Healy <cphealy@...il.com>,
        linux-kernel@...r.kernel.org,
        Andrey Smirnov <andrew.smirnov@...il.com>
Subject: Re: [PATCH] clk: imx: pllv3: support fractional multiplier on vf610
 PLL1/PLL2

On 12/09, Nikita Yushchenko wrote:
> diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
> index 19f9b622981a..24a9e914e0d5 100644
> --- a/drivers/clk/imx/clk-pllv3.c
> +++ b/drivers/clk/imx/clk-pllv3.c
> @@ -288,6 +291,87 @@ static const struct clk_ops clk_pllv3_av_ops = {
>  	.set_rate	= clk_pllv3_av_set_rate,
>  };
>  
> +static unsigned long clk_pllv3_vf610_recalc_rate(struct clk_hw *hw,
> +					      unsigned long parent_rate)
> +{
> +	struct clk_pllv3 *pll = to_clk_pllv3(hw);
> +	u32 mfn = readl_relaxed(pll->base + PLL_VF610_NUM_OFFSET);
> +	u32 mfd = readl_relaxed(pll->base + PLL_VF610_DENOM_OFFSET);
> +	u32 div = (readl_relaxed(pll->base) & pll->div_mask) ? 22 : 20;
> +	u64 temp64 = (u64)parent_rate;

Useless cast, please remove.

> +
> +	temp64 *= mfn;
> +	do_div(temp64, mfd);
> +
> +	return (parent_rate * div) + (u32)temp64;
> +}
> +
> +static long clk_pllv3_vf610_round_rate(struct clk_hw *hw, unsigned long rate,
> +				    unsigned long *prate)
> +{
> +	unsigned long parent_rate = *prate;
> +	unsigned int mfi = (rate >= 22 * parent_rate) ? 22 : 20;

What is the importance of 22 and 20? Hint, at the least it needs
a comment.

> +	u32 mfn, mfd = 0x3fffffff;
> +	u64 temp64;
> +
> +	if (rate <= parent_rate * mfi)
> +		mfn = 0;
> +	else if (rate >= parent_rate * (mfi + 1))
> +		mfn = mfd - 1;
> +	else {
> +		/* rate = parent_rate * (mfi + mfn/mfd) */
> +		temp64 = rate - parent_rate * mfi;
> +		temp64 *= mfd;
> +		do_div(temp64, parent_rate);
> +		mfn = temp64;
> +	}
> +
> +	temp64 = ((u64)mfd * mfi + mfn) * parent_rate;
> +	do_div(temp64, mfd);
> +	return (u32)temp64;

Do we need the cast here for some reason?

> +}
> +
> +static int clk_pllv3_vf610_set_rate(struct clk_hw *hw, unsigned long rate,
> +		unsigned long parent_rate)
> +{
> +	struct clk_pllv3 *pll = to_clk_pllv3(hw);
> +	unsigned int mfi = (rate >= 22 * parent_rate) ? 22 : 20;
> +	u32 val, mfn, mfd = 0x3fffffff;
> +	u64 temp64;
> +
> +	if (rate <= parent_rate * mfi)
> +		mfn = 0;
> +	else if (rate >= parent_rate * (mfi + 1))
> +		mfn = mfd - 1;
> +	else {
> +		/* rate = parent_rate * (mfi + mfn/mfd) */
> +		temp64 = rate - parent_rate * mfi;
> +		temp64 *= mfd;
> +		do_div(temp64, parent_rate);
> +		mfn = temp64;
> +	}
> +
> +	val = readl_relaxed(pll->base);
> +	if (mfi == 20)

Presumably this is another place 20 and 22 are special.

> +		val &= ~pll->div_mask;
> +	else
> +		val |= pll->div_mask;
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project