ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by Ranjani Vaidyanathan <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 121 insertions(+), 4 deletions(-)

arch/arm/mach-imx/clk-imx6q.c +121 -4

index 20ad0d1..3ee45f4 100644
@@ -140,6 +140,123 @@ static struct clk_div_table video_div_table[] = {
 	{ /* sentinel */ }
 };
 
+static void init_ldb_clks(enum mx6q_clks new_parent)
+{
+	struct device_node *np;
+	static void __iomem *ccm_base;
+	unsigned int reg;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ccm");
+	ccm_base = of_iomap(np, 0);
+	WARN_ON(!ccm_base);
+
+	/*
+	 * Need to follow a strict procedure when changing the LDB
+	 * clock, else we can introduce a glitch. Things to keep in
+	 * mind:
+	 * 1. The current and new parent clocks must be disabled.
+	 * 2. The default clock for ldb_dio_clk is mmdc_ch1 which has
+	 * no CG bit.
+	 * 3. In the RTL implementation of the LDB_DI_CLK_SEL mux
+	 * the top four options are in one mux and the PLL3 option along
+	 * with another option is in the second mux. There is third mux
+	 * used to decide between the first and second mux.
+	 * The code below switches the parent to the bottom mux first
+	 * and then manipulates the top mux. This ensures that no glitch
+	 * will enter the divider.
+	 *
+	 * Need to disable MMDC_CH1 clock manually as there is no CG bit
+	 * for this clock. The only way to disable this clock is to move
+	 * it topll3_sw_clk and then to disable pll3_sw_clk
+	 * Make sure periph2_clk2_sel is set to pll3_sw_clk
+	 */
+	reg = readl_relaxed(ccm_base + 0x18);
+	reg &= ~(1 << 20);
+	writel_relaxed(reg, ccm_base + 0x18);
+
+	/* Set MMDC_CH1 mask bit */
+	reg = readl_relaxed(ccm_base + 0x4);
+	reg |= 1 << 16;
+	writel_relaxed(reg, ccm_base + 0x4);
+
+	/*
+	 * Set the periph2_clk_sel to the top mux so that
+	 * mmdc_ch1 is from pll3_sw_clk.
+	 */
+	reg = readl_relaxed(ccm_base + 0x14);
+	reg |= 1 << 26;
+	writel_relaxed(reg, ccm_base + 0x14);
+
+	/* Wait for the clock switch */
+	while (readl_relaxed(ccm_base + 0x48))
+		;
+
+	/* Disable pll3_sw_clk by selecting the bypass clock source */
+	reg = readl_relaxed(ccm_base + 0xc);
+	reg |= 1 << 0;
+	writel_relaxed(reg, ccm_base + 0xc);
+
+	/* Set the ldb_di0_clk and ldb_di1_clk to 111b */
+	reg = readl_relaxed(ccm_base + 0x2c);
+	reg |= ((7 << 9) | (7 << 12));
+	writel_relaxed(reg, ccm_base + 0x2c);
+
+	/* Set the ldb_di0_clk and ldb_di1_clk to 100b */
+	reg = readl_relaxed(ccm_base + 0x2c);
+	reg &= ~((7 << 9) | (7 << 12));
+	reg |= ((4 << 9) | (4 << 12));
+	writel_relaxed(reg, ccm_base + 0x2c);
+
+	/* Perform the LDB parent clock switch */
+	clk_set_parent(clk[ldb_di0_sel], clk[new_parent]);
+	clk_set_parent(clk[ldb_di1_sel], clk[new_parent]);
+
+	/* Unbypass pll3_sw_clk */
+	reg = readl_relaxed(ccm_base + 0xc);
+	reg &= ~(1 << 0);
+	writel_relaxed(reg, ccm_base + 0xc);
+
+	/*
+	 * Set the periph2_clk_sel back to the bottom mux so that
+	 * mmdc_ch1 is from its original parent.
+	 */
+	reg = readl_relaxed(ccm_base + 0x14);
+	reg &= ~(1 << 26);
+	writel_relaxed(reg, ccm_base + 0x14);
+
+	/* Wait for the clock switch */
+	while (readl_relaxed(ccm_base + 0x48))
+		;
+
+	/* Clear MMDC_CH1 mask bit */
+	reg = readl_relaxed(ccm_base + 0x4);
+	reg &= ~(1 << 16);
+	writel_relaxed(reg, ccm_base + 0x4);
+}
+
+static void disable_anatop_clocks(void __iomem *anatop_base)
+{
+	unsigned int reg;
+
+	/* Make sure PFDs are disabled at boot. */
+	reg = readl_relaxed(anatop_base + 0x100);
+	/* Cannot disable pll2_pfd2_396M, as it is the MMDC clock in iMX6DL */
+	if (cpu_is_imx6dl())
+		reg |= 0x80008080;
+	else
+		reg |= 0x80808080;
+	writel_relaxed(reg, anatop_base + 0x100);
+
+	reg = readl_relaxed(anatop_base + 0xf0);
+	reg |= 0x80808080;
+	writel_relaxed(reg, anatop_base + 0xf0);
+
+	/* Make sure PLLs is disabled */
+	reg = readl_relaxed(anatop_base + 0xa0);
+	reg &= ~(1 << 13);
+	writel_relaxed(reg, anatop_base + 0xa0);
+}
+
 static void __init imx6q_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;

@@ -232,6 +349,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[pll5_post_div] = clk_register_divider_table(NULL, "pll5_post_div", "pll5_video", CLK_SET_RATE_PARENT, base + 0xa0, 19, 2, 0, post_div_table, &imx_ccm_lock);
 	clk[pll5_video_div] = clk_register_divider_table(NULL, "pll5_video_div", "pll5_post_div", CLK_SET_RATE_PARENT, base + 0x170, 30, 2, 0, video_div_table, &imx_ccm_lock);
 
+	disable_anatop_clocks(base);
+
 	np = ccm_node;
 	base = of_iomap(np, 0);
 	WARN_ON(!base);

@@ -440,10 +559,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk_register_clkdev(clk[enet_ref], "enet_ref", NULL);
 
 	if ((imx_get_soc_revision() != IMX_CHIP_REVISION_1_0) ||
-	    cpu_is_imx6dl()) {
-		clk_set_parent(clk[ldb_di0_sel], clk[pll5_video_div]);
-		clk_set_parent(clk[ldb_di1_sel], clk[pll5_video_div]);
-	}
+	    cpu_is_imx6dl())
+		init_ldb_clks(pll5_video_div);
 
 	/*
 	 * The gpmi needs 100MHz frequency in the EDO/Sync mode,

Signature · 1 line

1.8.3.2

I'm not sure this is a full/correct fix to the problem. You assume that the re-parenting only happens during boot for once? What about the

re-parenting triggered by the clk_set_parent() call?

Quoted 2 lines · Shawn Guo

I'm not sure this is a full/correct fix to the problem. You assume that the re-parenting only happens during boot for once? What about the

Yes, correct. As far as I can see LDB parent is only set once in clk-imx6q.

Quoted 1 line

re-parenting triggered by the clk_set_parent() call?

Where do you see the LDB clock parent to change via clk_set_parent() call?

In imx_ldb.c we use clk_set_parent() to set the di parent, not ldb parent.

As long as you call clk_register_mux() to register a multiplexer, you have to ensure that clk_set_parent() call always works properly on it, no matter whether there is one actually calling into it right now.

Furthermore, some re-parenting happens in a way you may not be aware of. See commit e366fdd (clk: clk-mux: implement remuxing on set_rate), for example.

Hi Shawn,

Quoted 3 lines · Shawn Guo

As long as you call clk_register_mux() to register a multiplexer, you have to ensure that clk_set_parent() call always works properly on it, no matter whether there is one actually calling into it right now.

clk_register_mux() is not called for the ldb_di_sels multiplexer:

**** Clk name is lvds1_sel
********* Calling clk_register_mux
**** Clk name is lvds2_sel
********* Calling clk_register_mux
**** Clk name is step
********* Calling clk_register_mux
**** Clk name is pll1_sw
********* Calling clk_register_mux
**** Clk name is periph_pre
********* Calling clk_register_mux
**** Clk name is periph2_pre
********* Calling clk_register_mux
**** Clk name is periph_clk2_sel
********* Calling clk_register_mux
**** Clk name is periph2_clk2_sel
********* Calling clk_register_mux
**** Clk name is axi_sel
********* Calling clk_register_mux
**** Clk name is esai_sel
********* Calling clk_register_mux
**** Clk name is asrc_sel
********* Calling clk_register_mux
**** Clk name is spdif_sel
********* Calling clk_register_mux
**** Clk name is gpu2d_axi
********* Calling clk_register_mux
**** Clk name is gpu3d_axi
********* Calling clk_register_mux
**** Clk name is gpu2d_core_sel
********* Calling clk_register_mux
**** Clk name is gpu3d_core_sel
********* Calling clk_register_mux

**** Clk name is gpu3d_shader_sel
********* Calling clk_register_mux
**** Clk name is ipu1_sel
********* Calling clk_register_mux
**** Clk name is ipu2_sel
********* Calling clk_register_mux
********* Calling clk_register_mux
********* Calling clk_register_mux

**** Clk name is ipu1_di0_pre_sel
********* Calling clk_register_mux
**** Clk name is ipu1_di1_pre_sel
********* Calling clk_register_mux
**** Clk name is ipu2_di0_pre_sel
********* Calling clk_register_mux
**** Clk name is ipu2_di1_pre_sel
********* Calling clk_register_mux
**** Clk name is ipu1_di0_sel
********* Calling clk_register_mux
**** Clk name is ipu1_di1_sel
********* Calling clk_register_mux
**** Clk name is ipu2_di0_sel
********* Calling clk_register_mux
**** Clk name is ipu2_di1_sel
********* Calling clk_register_mux
**** Clk name is hsi_tx_sel
********* Calling clk_register_mux
**** Clk name is pcie_axi_sel
********* Calling clk_register_mux
**** Clk name is enfc_sel
********* Calling clk_register_mux
**** Clk name is vdo_axi_sel
********* Calling clk_register_mux
**** Clk name is vpu_axi_sel

********* Calling clk_register_mux
**** Clk name is cko1_sel
********* Calling clk_register_mux
**** Clk name is cko2_sel
********* Calling clk_register_mux
**** Clk name is cko
********* Calling clk_register_mux
********* doing ldb clock switch

Quoted 5 lines

Furthermore, some re-parenting happens in a way you may not be aware of. See commit e366fdd (clk: clk-mux: implement remuxing on set_rate), for example.

This commit does not affect us as we pass the CLK_SET_RATE_NO_REPARENT flag.

For the record, here is my printk gives.

*** clk_register_mux: ldb_di0_sel
*** clk_register_mux: ldb_di1_sel

Quoted 6 lines

Quoted 5 lines

Furthermore, some re-parenting happens in a way you may not be aware of. See commit e366fdd (clk: clk-mux: implement remuxing on set_rate), for example.

Quoted 1 line

This commit does not affect us as we pass the CLK_SET_RATE_NO_REPARENT flag.

You did not get my point. This is just an example, and we happen to set this flag for now. My point is that as long as you register a clk to clock framework, you do not have a way to stop one from calling clk API on the clock then. This is how clk framework and API work, simple as it is.

Quoted 1 line · Shawn Guo

For the record, here is my printk gives.

...

Quoted 2 lines

*** clk_register_mux: ldb_di0_sel
*** clk_register_mux: ldb_di1_sel

Ok, I ran it again and yes, I can see it now. Sorry for the previous wrong printk's.

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Furthermore, some re-parenting happens in a way you may not be aware of. See commit e366fdd (clk: clk-mux: implement remuxing on set_rate), for example.

Quoted 7 lines

Quoted 1 line

This commit does not affect us as we pass the CLK_SET_RATE_NO_REPARENT flag.

You did not get my point. This is just an example, and we happen to set this flag for now. My point is that as long as you register a clk to clock framework, you do not have a way to stop one from calling clk API on the clock then. This is how clk framework and API work, simple as it is.

The issue that this patch wants to solve is that we need to perform this protection clock switching mechanism prior to doing the clk_set_parent for the ldb clocks.

Putting a printk in clk_set_parent like this:

drivers/clk/clk.c +2 -0

@@ -1719,6 +1719,8 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
        if (!clk->ops)
                return -EINVAL;
 
+       pr_info(" *** Calling clk_set_parent for %s clock\n", clk->name);
+
        /* verify ops for for multi-parent clks */
        if ((clk->num_parents > 1) && (!clk->ops->set_parent))
                return -ENOSYS;

,results in:

*** Calling clk_set_parent for ldb_di0_sel clock *** Calling clk_set_parent for ldb_di1_sel clock *** Calling clk_set_parent for enfc_sel clock *** Calling clk_set_parent for cko2_sel clock *** Calling clk_set_parent for cko clock *** Calling clk_set_parent for spdif_sel clock *** Calling clk_set_parent for lvds1_sel clock *** Calling clk_set_parent for ipu1_di0_sel clock

Which shows that there is only one clk_set_parent being called for the ldb_di clocks and this one is in clk-mx6q.c.

So it is safe to perform this workaround in clk-imx6q.c.

Don't you agree?

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by Ranjani Vaidyanathan <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 121 insertions(+), 4 deletions(-)

arch/arm/mach-imx/clk-imx6q.c +121 -5

index 20ad0d1..3ee45f4 100644
@@ -140,6 +140,123 @@ static struct clk_div_table video_div_table[] = {
        { /* sentinel */ }
 };
 
+static void init_ldb_clks(enum mx6q_clks new_parent)
+{
+       struct device_node *np;
+       static void __iomem *ccm_base;
+       unsigned int reg;
+
+       np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ccm");
+       ccm_base = of_iomap(np, 0);
+       WARN_ON(!ccm_base);
+
+       /*
+        * Need to follow a strict procedure when changing the LDB
+        * clock, else we can introduce a glitch. Things to keep in
+        * mind:
+        * 1. The current and new parent clocks must be disabled.
+        * 2. The default clock for ldb_dio_clk is mmdc_ch1 which has
+        * no CG bit.
+        * 3. In the RTL implementation of the LDB_DI_CLK_SEL mux
+        * the top four options are in one mux and the PLL3 option along
+        * with another option is in the second mux. There is third mux
+        * used to decide between the first and second mux.
+        * The code below switches the parent to the bottom mux first
+        * and then manipulates the top mux. This ensures that no glitch
+        * will enter the divider.
+        *
+        * Need to disable MMDC_CH1 clock manually as there is no CG bit
+        * for this clock. The only way to disable this clock is to move
+        * it topll3_sw_clk and then to disable pll3_sw_clk
+        * Make sure periph2_clk2_sel is set to pll3_sw_clk
+        */
+       reg = readl_relaxed(ccm_base + 0x18);
+       reg &= ~(1 << 20);
+       writel_relaxed(reg, ccm_base + 0x18);
+
+       /* Set MMDC_CH1 mask bit */
+       reg = readl_relaxed(ccm_base + 0x4);
+       reg |= 1 << 16;
+       writel_relaxed(reg, ccm_base + 0x4);
+
+       /*
+        * Set the periph2_clk_sel to the top mux so that
+        * mmdc_ch1 is from pll3_sw_clk.
+        */
+       reg = readl_relaxed(ccm_base + 0x14);
+       reg |= 1 << 26;
+       writel_relaxed(reg, ccm_base + 0x14);
+
+       /* Wait for the clock switch */
+       while (readl_relaxed(ccm_base + 0x48))
+               ;
+
+       /* Disable pll3_sw_clk by selecting the bypass clock source */
+       reg = readl_relaxed(ccm_base + 0xc);
+       reg |= 1 << 0;
+       writel_relaxed(reg, ccm_base + 0xc);
+
+       /* Set the ldb_di0_clk and ldb_di1_clk to 111b */
+       reg = readl_relaxed(ccm_base + 0x2c);
+       reg |= ((7 << 9) | (7 << 12));
+       writel_relaxed(reg, ccm_base + 0x2c);
+
+       /* Set the ldb_di0_clk and ldb_di1_clk to 100b */
+       reg = readl_relaxed(ccm_base + 0x2c);
+       reg &= ~((7 << 9) | (7 << 12));
+       reg |= ((4 << 9) | (4 << 12));
+       writel_relaxed(reg, ccm_base + 0x2c);
+
+       /* Perform the LDB parent clock switch */
+       clk_set_parent(clk[ldb_di0_sel], clk[new_parent]);
+       clk_set_parent(clk[ldb_di1_sel], clk[new_parent]);
+
+       /* Unbypass pll3_sw_clk */
+       reg = readl_relaxed(ccm_base + 0xc);
+       reg &= ~(1 << 0);
+       writel_relaxed(reg, ccm_base + 0xc);
+
+       /*
+        * Set the periph2_clk_sel back to the bottom mux so that
+        * mmdc_ch1 is from its original parent.
+        */
+       reg = readl_relaxed(ccm_base + 0x14);
+       reg &= ~(1 << 26);
+       writel_relaxed(reg, ccm_base + 0x14);
+
+       /* Wait for the clock switch */
+       while (readl_relaxed(ccm_base + 0x48))
+               ;
+
+       /* Clear MMDC_CH1 mask bit */
+       reg = readl_relaxed(ccm_base + 0x4);
+       reg &= ~(1 << 16);
+       writel_relaxed(reg, ccm_base + 0x4);
+}
+
+static void disable_anatop_clocks(void __iomem *anatop_base)
+{
+       unsigned int reg;
+
+       /* Make sure PFDs are disabled at boot. */
+       reg = readl_relaxed(anatop_base + 0x100);
+       /* Cannot disable pll2_pfd2_396M, as it is the MMDC clock in iMX6DL */
+       if (cpu_is_imx6dl())
+               reg |= 0x80008080;
+       else
+               reg |= 0x80808080;
+       writel_relaxed(reg, anatop_base + 0x100);
+
+       reg = readl_relaxed(anatop_base + 0xf0);
+       reg |= 0x80808080;
+       writel_relaxed(reg, anatop_base + 0xf0);
+
+       /* Make sure PLLs is disabled */
+       reg = readl_relaxed(anatop_base + 0xa0);
+       reg &= ~(1 << 13);
+       writel_relaxed(reg, anatop_base + 0xa0);
+}
+
 static void __init imx6q_clocks_init(struct device_node *ccm_node)
 {
        struct device_node *np;

@@ -232,6 +349,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
        clk[pll5_post_div] = clk_register_divider_table(NULL, "pll5_post_div", "pll5_video", CLK_SET_RATE_PARENT, base + 0xa0, 19, 2, 0, post_div_table, &imx_ccm_lock);
        clk[pll5_video_div] = clk_register_divider_table(NULL, "pll5_video_div", "pll5_post_div", CLK_SET_RATE_PARENT, base + 0x170, 30, 2, 0, video_div_table, &imx_ccm_lock);
 
+       disable_anatop_clocks(base);
+
        np = ccm_node;
        base = of_iomap(np, 0);
        WARN_ON(!base);

@@ -440,10 +559,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
        clk_register_clkdev(clk[enet_ref], "enet_ref", NULL);
 
        if ((imx_get_soc_revision() != IMX_CHIP_REVISION_1_0) ||
-           cpu_is_imx6dl()) {
-               clk_set_parent(clk[ldb_di0_sel], clk[pll5_video_div]);
-               clk_set_parent(clk[ldb_di1_sel], clk[pll5_video_div]);
-       }
+           cpu_is_imx6dl())
+               init_ldb_clks(pll5_video_div);
 
        /*
         * The gpmi needs 100MHz frequency in the EDO/Sync mode,
--

1.8.3.2

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by Ranjani Vaidyanathan <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
  arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 121 insertions(+), 4 deletions(-)

arch/arm/mach-imx/clk-imx6q.c +28 -0

index 20ad0d1..3ee45f4 100644
@@ -140,6 +140,123 @@ static struct clk_div_table video_div_table[] = {
  	{ /* sentinel */ }
  };
 
+static void init_ldb_clks(enum mx6q_clks new_parent)
+{
+	struct device_node *np;
+	static void __iomem *ccm_base;
+	unsigned int reg;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ccm");
+	ccm_base = of_iomap(np, 0);
+	WARN_ON(!ccm_base);
+
+	/*
+	 * Need to follow a strict procedure when changing the LDB
+	 * clock, else we can introduce a glitch. Things to keep in
+	 * mind:
+	 * 1. The current and new parent clocks must be disabled.
+	 * 2. The default clock for ldb_dio_clk is mmdc_ch1 which has
+	 * no CG bit.
+	 * 3. In the RTL implementation of the LDB_DI_CLK_SEL mux
+	 * the top four options are in one mux and the PLL3 option along
+	 * with another option is in the second mux. There is third mux
+	 * used to decide between the first and second mux.
+	 * The code below switches the parent to the bottom mux first
+	 * and then manipulates the top mux. This ensures that no glitch
+	 * will enter the divider.
+	 *
+	 * Need to disable MMDC_CH1 clock manually as there is no CG bit
+	 * for this clock. The only way to disable this clock is to move
+	 * it topll3_sw_clk and then to disable pll3_sw_clk

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by: Ranjani Vaidyanathan
<[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
  arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 121 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-imx/clk-imx6q.c index 20ad0d1..3ee45f4 100644

arch/arm/mach-imx/clk-imx6q.c +27 -0

@@ -140,6 +140,123 @@ static struct clk_div_table video_div_table[] = {
  	{ /* sentinel */ }
  };
 
+static void init_ldb_clks(enum mx6q_clks new_parent) {
+	struct device_node *np;
+	static void __iomem *ccm_base;
+	unsigned int reg;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ccm");
+	ccm_base = of_iomap(np, 0);
+	WARN_ON(!ccm_base);
+
+	/*
+	 * Need to follow a strict procedure when changing the LDB
+	 * clock, else we can introduce a glitch. Things to keep in
+	 * mind:
+	 * 1. The current and new parent clocks must be disabled.
+	 * 2. The default clock for ldb_dio_clk is mmdc_ch1 which has
+	 * no CG bit.
+	 * 3. In the RTL implementation of the LDB_DI_CLK_SEL mux
+	 * the top four options are in one mux and the PLL3 option along
+	 * with another option is in the second mux. There is third mux
+	 * used to decide between the first and second mux.
+	 * The code below switches the parent to the bottom mux first
+	 * and then manipulates the top mux. This ensures that no glitch
+	 * will enter the divider.
+	 *
+	 * Need to disable MMDC_CH1 clock manually as there is no CG bit
+	 * for this clock. The only way to disable this clock is to move
+	 * it topll3_sw_clk and then to disable pll3_sw_clk

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by Ranjani Vaidyanathan <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 121 insertions(+), 4 deletions(-)

arch/arm/mach-imx/clk-imx6q.c +29 -0

index 20ad0d1..3ee45f4 100644
@@ -140,6 +140,123 @@ static struct clk_div_table video_div_table[] = {
 	{ /* sentinel */ }
 };
 
+static void init_ldb_clks(enum mx6q_clks new_parent)
+{
+	struct device_node *np;
+	static void __iomem *ccm_base;
+	unsigned int reg;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ccm");
+	ccm_base = of_iomap(np, 0);
+	WARN_ON(!ccm_base);
+
+	/*
+	 * Need to follow a strict procedure when changing the LDB
+	 * clock, else we can introduce a glitch. Things to keep in
+	 * mind:
+	 * 1. The current and new parent clocks must be disabled.
+	 * 2. The default clock for ldb_dio_clk is mmdc_ch1 which has
+	 * no CG bit.
+	 * 3. In the RTL implementation of the LDB_DI_CLK_SEL mux
+	 * the top four options are in one mux and the PLL3 option along
+	 * with another option is in the second mux. There is third mux
+	 * used to decide between the first and second mux.
+	 * The code below switches the parent to the bottom mux first
+	 * and then manipulates the top mux. This ensures that no glitch
+	 * will enter the divider.
+	 *
+	 * Need to disable MMDC_CH1 clock manually as there is no CG bit
+	 * for this clock. The only way to disable this clock is to move
+	 * it topll3_sw_clk and then to disable pll3_sw_clk
+	 * Make sure periph2_clk2_sel is set to pll3_sw_clk

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by Ranjani Vaidyanathan <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
  arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 121 insertions(+), 4 deletions(-)

arch/arm/mach-imx/clk-imx6q.c +0 -0

index 20ad0d1..3ee45f4 100644

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by: Ranjani Vaidyanathan
<[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
  arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 121 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-imx/clk-imx6q.c index 20ad0d1..3ee45f4 100644

arch/arm/mach-imx/clk-imx6q.c +0 -0

Hi Dirk,

Responses below.

Thanks,
Ranjani

-----Original Message-----
From: Dirk Behme [mailto:dirk.behme at gmail.com]
Sent: Wednesday, June 04, 2014 11:37 AM
To: Vaidyanathan Ranjani-RA5478; Estevam Fabio-R49496
Cc: Fabio Estevam; Guo Shawn-R65073; christian.gmeiner at gmail.com; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK

Quoted 23 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by: Ranjani Vaidyanathan
<[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
   arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
   1 file changed, 121 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-imx/clk-imx6q.c index 20ad0d1..3ee45f4 100644

arch/arm/mach-imx/clk-imx6q.c +0 -0

....

arch/arm/mach-imx/clk-imx6q.c static struct clk_div_table video_div_table[] = { · line 247

Quoted 10 lines

arch/arm/mach-imx/clk-imx6q.c +10 -0

+static void disable_anatop_clocks(void __iomem *anatop_base) {
+	unsigned int reg;
+
+	/* Make sure PFDs are disabled at boot. */
+	reg = readl_relaxed(anatop_base + 0x100);
+	/* Cannot disable pll2_pfd2_396M, as it is the MMDC clock in iMX6DL */
+	if (cpu_is_imx6dl())
+		reg |= 0x80008080;
+	else
+		reg |= 0x80808080;

There are two issues with this code section:

1) You want to know here if pll2_pfd2_396M is used as MMDC clock. This is independent of IMX6DL. Even a Quad/Dual could use pll2_pfd2_396M as MMDC clock. You better should check what you are really looking for:

if (MMDC clock == pll2_pfd2_396M)
...
[RV] Yes, you are perfectly correct. We need to check MMDC parent clock.

Hi Dirk,

Responses below.

Thanks,
Ranjani

-----Original Message-----
From: Dirk Behme [mailto:dirk.behme at gmail.com]
Sent: Wednesday, June 04, 2014 11:37 AM
To: Vaidyanathan Ranjani-RA5478; Estevam Fabio-R49496
Cc: Fabio Estevam; Guo Shawn-R65073; christian.gmeiner at gmail.com; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK

Quoted 23 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by: Ranjani Vaidyanathan
<[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
   arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
   1 file changed, 121 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-imx/clk-imx6q.c index 20ad0d1..3ee45f4 100644

arch/arm/mach-imx/clk-imx6q.c +0 -0

....

arch/arm/mach-imx/clk-imx6q.c static struct clk_div_table video_div_table[] = { · line 247

Quoted 10 lines

arch/arm/mach-imx/clk-imx6q.c +10 -0

+static void disable_anatop_clocks(void __iomem *anatop_base) {
+	unsigned int reg;
+
+	/* Make sure PFDs are disabled at boot. */
+	reg = readl_relaxed(anatop_base + 0x100);
+	/* Cannot disable pll2_pfd2_396M, as it is the MMDC clock in iMX6DL */
+	if (cpu_is_imx6dl())
+		reg |= 0x80008080;
+	else
+		reg |= 0x80808080;

There are two issues with this code section:

1) You want to know here if pll2_pfd2_396M is used as MMDC clock. This is independent of IMX6DL. Even a Quad/Dual could use pll2_pfd2_396M as MMDC clock. You better should check what you are really looking for:

if (MMDC clock == pll2_pfd2_396M)
...
[RV] Yes, you are perfectly correct. We need to check MMDC parent clock.

Hi Dirk,

Responses below.

Thanks,
Ranjani

-----Original Message-----
From: Dirk Behme [mailto:dirk.behme at gmail.com]
Sent: Wednesday, June 04, 2014 11:37 AM
To: Vaidyanathan Ranjani-RA5478; Estevam Fabio-R49496
Cc: Fabio Estevam; Guo Shawn-R65073; christian.gmeiner at gmail.com; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK

Quoted 23 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.

To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot.

Signed-off-by: Ranjani Vaidyanathan
<[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
   arch/arm/mach-imx/clk-imx6q.c | 125 ++++++++++++++++++++++++++++++++++++++++--
   1 file changed, 121 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-imx/clk-imx6q.c index 20ad0d1..3ee45f4 100644

arch/arm/mach-imx/clk-imx6q.c +0 -0

....

arch/arm/mach-imx/clk-imx6q.c static struct clk_div_table video_div_table[] = { · line 247

Quoted 10 lines

arch/arm/mach-imx/clk-imx6q.c +10 -0

+static void disable_anatop_clocks(void __iomem *anatop_base) {
+	unsigned int reg;
+
+	/* Make sure PFDs are disabled at boot. */
+	reg = readl_relaxed(anatop_base + 0x100);
+	/* Cannot disable pll2_pfd2_396M, as it is the MMDC clock in iMX6DL */
+	if (cpu_is_imx6dl())
+		reg |= 0x80008080;
+	else
+		reg |= 0x80808080;

There are two issues with this code section:

1) You want to know here if pll2_pfd2_396M is used as MMDC clock. This is independent of IMX6DL. Even a Quad/Dual could use pll2_pfd2_396M as MMDC clock. You better should check what you are really looking for:

if (MMDC clock == pll2_pfd2_396M)
...
[RV] Yes, you are perfectly correct. We need to check MMDC parent clock.

2) Why is bit 31, i.e. the uppermost '8' in 0x80808080, set here? It looks like the TRM marks bit 31 - 24 as reserved? [RV] Yes, this should also be fixed.