[PATCH 0/3] Fix "i.MX: vf610: Ramp CPU clock to maximum frequency"

[PATCH 0/3] Fix "i.MX: vf610: Ramp CPU clock to maximum frequency"

[Andrey Smirnov]

Everyone,

It looks like my test setup was somehow contaminated -- possibly by
using JTAG to initalize SoC and upload Barebox binary -- so
21921f7f419dfaabcd385595ada24f0352310f1a ("i.MX: vf610: Ramp CPU clock
to maximum frequency") didn't actually work as I thought it was and
instead it made Barebox not boot on Vybrid.

This patch series contains all of the changes I had to make in order
to make CPU clock switching work/"unbreak" Barebox on Vybrid.

Let me know if anything needs changing.

Thanks,
Andrey Smirnov


Andrey Smirnov (3):
  clk-vf610: Mark all of CCSR muxes with CLK_OPS_PARENT_ENABLE
  i.MX: clk-pllv3: Do not touch PLL_BYPASS bit
  i.MX: clk: Remove imx_clk_pllv3_locked()

 drivers/clk/imx/clk-pllv3.c | 26 --------------------------
 drivers/clk/imx/clk-vf610.c | 28 ++++++++++++++++++++--------
 drivers/clk/imx/clk.h       |  4 ----
 3 files changed, 20 insertions(+), 38 deletions(-)

-- 
2.9.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] clk-vf610: Mark all of CCSR muxes with CLK_OPS_PARENT_ENABLE

[Andrey Smirnov]

As per VYBRIDRM.pdf, p. 673:

"... When switching clock sources on GL MUX, both active and target
clock sources must be active..."

So mark all of the clock muxes controlled by CCM_CSSR with
CLK_OPS_PARENT_ENABLE to satisfy that requirement. Experiment shows
that failing to do so would result in failure (in some cases CPU
hang).

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/clk/imx/clk-vf610.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
index 41fa3e9..5469840 100644
--- a/drivers/clk/imx/clk-vf610.c
+++ b/drivers/clk/imx/clk-vf610.c
@@ -181,8 +181,8 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
 		CCSR_PLL1_PFDn_EN(4);
 	writel(ccsr, CCM_CCSR);
 
-	clk[VF610_CLK_SLOW_CLK_SEL] = imx_clk_mux("slow_clk_sel", CCM_CCSR, 4, 1, slow_sels, ARRAY_SIZE(slow_sels));
-	clk[VF610_CLK_FASK_CLK_SEL] = imx_clk_mux("fast_clk_sel", CCM_CCSR, 5, 1, fast_sels, ARRAY_SIZE(fast_sels));
+	clk[VF610_CLK_SLOW_CLK_SEL] = imx_clk_mux2("slow_clk_sel", CCM_CCSR, 4, 1, slow_sels, ARRAY_SIZE(slow_sels));
+	clk[VF610_CLK_FASK_CLK_SEL] = imx_clk_mux2("fast_clk_sel", CCM_CCSR, 5, 1, fast_sels, ARRAY_SIZE(fast_sels));
 
 	clk[VF610_CLK_PLL1_BYPASS_SRC] = imx_clk_mux("pll1_bypass_src", PLL1_CTRL, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
 	clk[VF610_CLK_PLL2_BYPASS_SRC] = imx_clk_mux("pll2_bypass_src", PLL2_CTRL, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
@@ -243,10 +243,10 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
 	clk[VF610_CLK_PLL3_PFD3] = imx_clk_pfd("pll3_pfd3", "pll3_usb_otg", PFD_PLL3_BASE, 2);
 	clk[VF610_CLK_PLL3_PFD4] = imx_clk_pfd("pll3_pfd4", "pll3_usb_otg", PFD_PLL3_BASE, 3);
 
-	clk[VF610_CLK_PLL1_PFD_SEL] = imx_clk_mux("pll1_pfd_sel", CCM_CCSR, 16, 3, pll1_sels, 5);
-	clk[VF610_CLK_PLL2_PFD_SEL] = imx_clk_mux("pll2_pfd_sel", CCM_CCSR, 19, 3, pll2_sels, 5);
-	clk[VF610_CLK_SYS_SEL] = imx_clk_mux("sys_sel", CCM_CCSR, 0, 3, sys_sels, ARRAY_SIZE(sys_sels));
-	clk[VF610_CLK_DDR_SEL] = imx_clk_mux("ddr_sel", CCM_CCSR, 6, 1, ddr_sels, ARRAY_SIZE(ddr_sels));
+	clk[VF610_CLK_PLL1_PFD_SEL] = imx_clk_mux2("pll1_pfd_sel", CCM_CCSR, 16, 3, pll1_sels, 5);
+	clk[VF610_CLK_PLL2_PFD_SEL] = imx_clk_mux2("pll2_pfd_sel", CCM_CCSR, 19, 3, pll2_sels, 5);
+	clk[VF610_CLK_SYS_SEL] = imx_clk_mux2("sys_sel", CCM_CCSR, 0, 3, sys_sels, ARRAY_SIZE(sys_sels));
+	clk[VF610_CLK_DDR_SEL] = imx_clk_mux2("ddr_sel", CCM_CCSR, 6, 1, ddr_sels, ARRAY_SIZE(ddr_sels));
 	clk[VF610_CLK_SYS_BUS] = imx_clk_divider("sys_bus", "sys_sel", CCM_CACRR, 0, 3);
 	clk[VF610_CLK_PLATFORM_BUS] = imx_clk_divider("platform_bus", "sys_bus", CCM_CACRR, 3, 3);
 	clk[VF610_CLK_IPG_BUS] = imx_clk_divider("ipg_bus", "platform_bus", CCM_CACRR, 11, 2);
-- 
2.9.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] i.MX: clk-pllv3: Do not touch PLL_BYPASS bit

[Andrey Smirnov]

Do not touch PLL_BYPASS bit as a part of clk_pll3_enable/disable
execution. For a number of platforms (e.g. Vybrid, i.MX6SL) PLL_BYPASS
is specified as a bit controlling a clock MUX represented by a
dedicated 'struct clk'. Altering that bit as a part of
clk_pll3_enable/disable is equivalent to calling clk_set_parent() and
it makes in the following code:

clk_enable(clk_disable(<some pll>))

change clock chain instead of being a no-op.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/clk/imx/clk-pllv3.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 6d4399b..a14d36e 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -56,7 +56,6 @@ static int clk_pllv3_enable(struct clk *clk)
 	int timeout = 10000;
 
 	val = readl(pll->base);
-	val &= ~BM_PLL_BYPASS;
 	if (pll->powerup_set)
 		val |= pll->power_bit;
 	else
@@ -88,7 +87,6 @@ static void clk_pllv3_disable(struct clk *clk)
 	val &= ~BM_PLL_ENABLE;
 	writel(val, pll->base);
 
-	val |= BM_PLL_BYPASS;
 	if (pll->powerup_set)
 		val &= ~pll->power_bit;
 	else
-- 
2.9.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] i.MX: clk: Remove imx_clk_pllv3_locked()

[Andrey Smirnov]

Remove imx_clk_pllv3_locked() which was introduced for the sake of
Vybrid platform. The same effect (waiting on 'locked' bit) can be
achived with vanilla clk_pllv3_enable/disable, which make said
function unnecessary.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/clk/imx/clk-pllv3.c | 24 ------------------------
 drivers/clk/imx/clk-vf610.c | 16 ++++++++++++++--
 drivers/clk/imx/clk.h       |  4 ----
 3 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index a14d36e..0e55a63 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -41,8 +41,6 @@ struct clk_pllv3 {
 	u32		div_mask;
 	u32		div_shift;
 	const char	*parent;
-	void __iomem	*lock_reg;
-	u32		lock_mask;
 	u32		ref_clock;
 	u32		power_bit;
 };
@@ -354,9 +352,6 @@ static int clk_pllv3_sys_vf610_set_rate(struct clk *clk, unsigned long rate,
 	writel(mfn, pll->base + SYS_VF610_PLL_OFFSET + PLL_NUM_OFFSET);
 	writel(mfd, pll->base + SYS_VF610_PLL_OFFSET + PLL_DENOM_OFFSET);
 
-	while (!(readl(pll->lock_reg) & pll->lock_mask))
-		;
-
 	return 0;
 }
 
@@ -427,22 +422,3 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 
 	return &pll->clk;
 }
-
-struct clk *imx_clk_pllv3_locked(enum imx_pllv3_type type, const char *name,
-				 const char *parent, void __iomem *base,
-				 u32 div_mask, void __iomem *lock_reg, u32 lock_mask)
-{
-	struct clk *clk;
-	struct clk_pllv3 *pll;
-
-	clk = imx_clk_pllv3(type, name, parent, base, div_mask);
-	if (IS_ERR(clk))
-		return clk;
-
-	pll = to_clk_pllv3(clk);
-
-	pll->lock_reg  = lock_reg;
-	pll->lock_mask = lock_mask;
-
-	return clk;
-}
diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
index 5469840..49d66fb 100644
--- a/drivers/clk/imx/clk-vf610.c
+++ b/drivers/clk/imx/clk-vf610.c
@@ -192,8 +192,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_locked(IMX_PLLV3_SYS_VF610, "pll1", "pll1_bypass_src", PLL1_CTRL, 0x1, PLL_LOCK, BIT(6));
-	clk[VF610_CLK_PLL2] = imx_clk_pllv3_locked(IMX_PLLV3_SYS_VF610, "pll2", "pll2_bypass_src", PLL2_CTRL, 0x1, PLL_LOCK, BIT(5));
+	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);
@@ -478,6 +478,18 @@ static int vf610_switch_cpu_clock_to_500mhz(void)
 		return -EINVAL;
 	}
 
+	/*
+	 * Code below alters the frequency of PLL1, and doing so would
+	 * require us to wait for PLL1 lock before proceeding to
+	 * select it as a clock source again.
+	 *
+	 * We achive this by relying on PLL1 being disabled implicitly
+	 * by selecting different source for "sys_sel", and then
+	 * consecutively enabled (which would result in busy waiting
+	 * on 'lock' bit) as a part of setting "pll1_pfd_sel" as a
+	 * source for "sys_sel".
+	 *
+	 */
 	ret = clk_set_parent(clk[VF610_CLK_SYS_SEL], clk[VF610_CLK_PLL2_BUS]);
 	if (ret < 0) {
 		pr_crit("Unable to re-parent '%s'\n",
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 8da8064..c46c261 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -126,10 +126,6 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 			  const char *parent, void __iomem *base,
 			  u32 div_mask);
 
-struct clk *imx_clk_pllv3_locked(enum imx_pllv3_type type, const char *name,
-				 const char *parent, void __iomem *base,
-				 u32 div_mask, void __iomem *lock_reg, u32 lock_mask);
-
 struct clk *imx_clk_pfd(const char *name, const char *parent,
 			void __iomem *reg, u8 idx);
 
-- 
2.9.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] Fix "i.MX: vf610: Ramp CPU clock to maximum frequency"

[Sascha Hauer]

On Mon, Jun 19, 2017 at 07:40:36AM -0700, Andrey Smirnov wrote:
> Everyone,
> 
> It looks like my test setup was somehow contaminated -- possibly by
> using JTAG to initalize SoC and upload Barebox binary -- so
> 21921f7f419dfaabcd385595ada24f0352310f1a ("i.MX: vf610: Ramp CPU clock
> to maximum frequency") didn't actually work as I thought it was and
> instead it made Barebox not boot on Vybrid.
> 
> This patch series contains all of the changes I had to make in order
> to make CPU clock switching work/"unbreak" Barebox on Vybrid.

You mean you want to have this series on master, right?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] Fix "i.MX: vf610: Ramp CPU clock to maximum frequency"

[Andrey Smirnov]

On Tue, Jun 20, 2017 at 12:35 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Jun 19, 2017 at 07:40:36AM -0700, Andrey Smirnov wrote:
>> Everyone,
>>
>> It looks like my test setup was somehow contaminated -- possibly by
>> using JTAG to initalize SoC and upload Barebox binary -- so
>> 21921f7f419dfaabcd385595ada24f0352310f1a ("i.MX: vf610: Ramp CPU clock
>> to maximum frequency") didn't actually work as I thought it was and
>> instead it made Barebox not boot on Vybrid.
>>
>> This patch series contains all of the changes I had to make in order
>> to make CPU clock switching work/"unbreak" Barebox on Vybrid.
>
> You mean you want to have this series on master, right?
>

I didn't really think about it, but yeah It probably would make sense
to apply it to master.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] Fix "i.MX: vf610: Ramp CPU clock to maximum frequency"

[Sascha Hauer]

On Tue, Jun 20, 2017 at 08:41:59PM -0700, Andrey Smirnov wrote:
> On Tue, Jun 20, 2017 at 12:35 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Mon, Jun 19, 2017 at 07:40:36AM -0700, Andrey Smirnov wrote:
> >> Everyone,
> >>
> >> It looks like my test setup was somehow contaminated -- possibly by
> >> using JTAG to initalize SoC and upload Barebox binary -- so
> >> 21921f7f419dfaabcd385595ada24f0352310f1a ("i.MX: vf610: Ramp CPU clock
> >> to maximum frequency") didn't actually work as I thought it was and
> >> instead it made Barebox not boot on Vybrid.
> >>
> >> This patch series contains all of the changes I had to make in order
> >> to make CPU clock switching work/"unbreak" Barebox on Vybrid.
> >
> > You mean you want to have this series on master, right?
> >
> 
> I didn't really think about it, but yeah It probably would make sense
> to apply it to master.

Ok, just did that.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox