* [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2
@ 2016-12-09 13:00 Nikita Yushchenko
2016-12-13 7:38 ` Stephen Boyd
0 siblings, 1 reply; 5+ messages in thread
From: Nikita Yushchenko @ 2016-12-09 13:00 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer, Fabio Estevam, Michael Turquette,
Stephen Boyd, linux-clk
Cc: Chris Healy, linux-kernel, Andrey Smirnov, Nikita Yushchenko
On vf610, PLL1 and PLL2 have registers to configure fractional part of
frequency multiplier.
This patch adds support for these registers.
This fixes "fast system clock" issue on boards where bootloader sets
fractional multiplier for PLL1.
Suggested-by: Andrey Smirnov <andrew.smirnov@gmail.com>
CC: Chris Healy <cphealy@gmail.com>
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
drivers/clk/imx/clk-pllv3.c | 87 +++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/imx/clk-vf610.c | 4 +--
drivers/clk/imx/clk.h | 1 +
3 files changed, 90 insertions(+), 2 deletions(-)
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
@@ -21,6 +21,9 @@
#define PLL_NUM_OFFSET 0x10
#define PLL_DENOM_OFFSET 0x20
+#define PLL_VF610_NUM_OFFSET 0x20
+#define PLL_VF610_DENOM_OFFSET 0x30
+
#define BM_PLL_POWER (0x1 << 12)
#define BM_PLL_LOCK (0x1 << 31)
#define IMX7_ENET_PLL_POWER (0x1 << 5)
@@ -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;
+
+ 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;
+ 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;
+}
+
+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)
+ val &= ~pll->div_mask;
+ else
+ val |= pll->div_mask;
+ writel_relaxed(val, pll->base);
+ writel_relaxed(mfn, pll->base + PLL_VF610_NUM_OFFSET);
+ writel_relaxed(mfd, pll->base + PLL_VF610_DENOM_OFFSET);
+
+ return clk_pllv3_wait_lock(pll);
+}
+
+static const struct clk_ops clk_pllv3_vf610_ops = {
+ .prepare = clk_pllv3_prepare,
+ .unprepare = clk_pllv3_unprepare,
+ .is_prepared = clk_pllv3_is_prepared,
+ .recalc_rate = clk_pllv3_vf610_recalc_rate,
+ .round_rate = clk_pllv3_vf610_round_rate,
+ .set_rate = clk_pllv3_vf610_set_rate,
+};
+
static unsigned long clk_pllv3_enet_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
@@ -322,6 +406,9 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
case IMX_PLLV3_SYS:
ops = &clk_pllv3_sys_ops;
break;
+ case IMX_PLLV3_SYS_VF610:
+ ops = &clk_pllv3_vf610_ops;
+ break;
case IMX_PLLV3_USB_VF610:
pll->div_shift = 1;
case IMX_PLLV3_USB:
diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
index 0476353ab423..59b1863deb88 100644
--- a/drivers/clk/imx/clk-vf610.c
+++ b/drivers/clk/imx/clk-vf610.c
@@ -219,8 +219,8 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
clk[VF610_CLK_PLL6_BYPASS_SRC] = imx_clk_mux("pll6_bypass_src", PLL6_CTRL, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
clk[VF610_CLK_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", PLL7_CTRL, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
- clk[VF610_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll1", "pll1_bypass_src", PLL1_CTRL, 0x1);
- clk[VF610_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", PLL2_CTRL, 0x1);
+ clk[VF610_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS_VF610, "pll1", "pll1_bypass_src", PLL1_CTRL, 0x1);
+ clk[VF610_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_SYS_VF610, "pll2", "pll2_bypass_src", PLL2_CTRL, 0x1);
clk[VF610_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB_VF610, "pll3", "pll3_bypass_src", PLL3_CTRL, 0x2);
clk[VF610_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV, "pll4", "pll4_bypass_src", PLL4_CTRL, 0x7f);
clk[VF610_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_ENET, "pll5", "pll5_bypass_src", PLL5_CTRL, 0x3);
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 3799ff82a9b4..99a617897831 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -34,6 +34,7 @@ enum imx_pllv3_type {
IMX_PLLV3_AV,
IMX_PLLV3_ENET,
IMX_PLLV3_ENET_IMX7,
+ IMX_PLLV3_SYS_VF610,
};
struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2
2016-12-09 13:00 [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2 Nikita Yushchenko
@ 2016-12-13 7:38 ` Stephen Boyd
2016-12-13 7:49 ` Nikita Yushchenko
2016-12-13 7:51 ` Nikita Yushchenko
0 siblings, 2 replies; 5+ messages in thread
From: Stephen Boyd @ 2016-12-13 7:38 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Michael Turquette,
linux-clk, Chris Healy, linux-kernel, Andrey Smirnov
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2
2016-12-13 7:38 ` Stephen Boyd
@ 2016-12-13 7:49 ` Nikita Yushchenko
2016-12-13 23:19 ` Stephen Boyd
2016-12-13 7:51 ` Nikita Yushchenko
1 sibling, 1 reply; 5+ messages in thread
From: Nikita Yushchenko @ 2016-12-13 7:49 UTC (permalink / raw)
To: Stephen Boyd
Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Michael Turquette,
linux-clk, Chris Healy, linux-kernel, Andrey Smirnov
>> 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.
Ok
>> +
>> + 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.
These come directly from datasheet:
Frequency multipler selection (MFI).
0: Fout = Fref * 20
1: Fout = Fref * 22
These numbers (20 / 22) are common among flavours of pllv3 hardware.
In similar places in the same file (e.g. in clk_pllv3_recalc_rate(), in
clk_pllv3_set_rate() ,etc) there are no comments explaining them.
Are you sure this place is special and comment is needed here?
>> + 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?
Just for readability, can remove if it hurts.
>> +}
>> +
>> +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.
See my reply above. Same applies here.
Nikita
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2
2016-12-13 7:49 ` Nikita Yushchenko
@ 2016-12-13 23:19 ` Stephen Boyd
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2016-12-13 23:19 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Michael Turquette,
linux-clk, Chris Healy, linux-kernel, Andrey Smirnov
On 12/13, 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
>
> >> +
> >> + 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.
>
> These come directly from datasheet:
>
> Frequency multipler selection (MFI).
> 0: Fout = Fref * 20
> 1: Fout = Fref * 22
>
> These numbers (20 / 22) are common among flavours of pllv3 hardware.
> In similar places in the same file (e.g. in clk_pllv3_recalc_rate(), in
> clk_pllv3_set_rate() ,etc) there are no comments explaining them.
>
> Are you sure this place is special and comment is needed here?
Probably an oversight when merging the code before. Please add a
comment, or make some sort of function like
rate_to_mfi(rate, parent_rate)
And then add a comment above the function to describe what's
special about 22 and 20.
>
> >> + 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?
>
> Just for readability, can remove if it hurts.
Please do.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2
2016-12-13 7:38 ` Stephen Boyd
2016-12-13 7:49 ` Nikita Yushchenko
@ 2016-12-13 7:51 ` Nikita Yushchenko
1 sibling, 0 replies; 5+ messages in thread
From: Nikita Yushchenko @ 2016-12-13 7:51 UTC (permalink / raw)
To: Stephen Boyd
Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Michael Turquette,
linux-clk, Chris Healy, linux-kernel, Andrey Smirnov
>> 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;
Should be not 'div' but 'mfi' to be consistent with datasheet and with
other routines in the same patch. Will fix.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-13 23:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-09 13:00 [PATCH] clk: imx: pllv3: support fractional multiplier on vf610 PLL1/PLL2 Nikita Yushchenko
2016-12-13 7:38 ` Stephen Boyd
2016-12-13 7:49 ` Nikita Yushchenko
2016-12-13 23:19 ` Stephen Boyd
2016-12-13 7:51 ` Nikita Yushchenko
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).