[PATCH v5 1/3] clk: imx6: Mask mmdc_ch1 handshake for periph2_sel and mmdc_ch1_axi_podf

[PATCH v5 1/3] clk: imx6: Mask mmdc_ch1 handshake for periph2_sel and mmdc_ch1_axi_podf

[Philipp Zabel]

MMDC CH1 is not used on i.MX6Q, so the handshake needed to change the
parent of periph2_sel or the divider of mmdc_ch1_axi_podf will never
succeed.
Disable the handshake mechanism to allow changing the frequency of
mmdc_ch1_axi, allowing to use it as a possible source for the LDB DI
clock.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/clk/imx/clk-imx6q.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index ba1c1ae..dd33ebc 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -156,6 +156,19 @@ static struct clk ** const uart_clks[] __initconst = {
 	NULL
 };
 
+#define CCM_CCDR		0x04
+
+#define CCDR_MMDC_CH1_MASK	BIT(16)
+
+static void __init imx6q_mmdc_ch1_mask_handshake(void __iomem *ccm_base)
+{
+	unsigned int reg;
+
+	reg = readl_relaxed(ccm_base + CCM_CCDR);
+	reg |= CCDR_MMDC_CH1_MASK;
+	writel_relaxed(reg, ccm_base + CCM_CCDR);
+}
+
 static void __init imx6q_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;
@@ -297,6 +310,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	base = of_iomap(np, 0);
 	WARN_ON(!base);
 
+	imx6q_mmdc_ch1_mask_handshake(base);
+
 	/*                                              name                reg       shift width parent_names     num_parents */
 	clk[IMX6QDL_CLK_STEP]             = imx_clk_mux("step",	            base + 0xc,  8,  1, step_sels,	   ARRAY_SIZE(step_sels));
 	clk[IMX6QDL_CLK_PLL1_SW]          = imx_clk_mux("pll1_sw",	    base + 0xc,  2,  1, pll1_sw_sels,      ARRAY_SIZE(pll1_sw_sels));
-- 
2.8.1

[PATCH v5 2/3] clk: imx6: Make the LDB_DI0 and LDB_DI1 clocks read-only.

[Philipp Zabel]

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. As this can not be guaranteed by
the clock framework during runtime, make the ldb_di[x]_sel muxes read-only.
A workaround to set the muxes once during boot could be added to the
kernel or bootloader.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/clk/imx/clk-imx6q.c | 10 ++--------
 drivers/clk/imx/clk.h       |  8 ++++++++
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index dd33ebc..9bbc994 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -340,8 +340,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[IMX6QDL_CLK_GPU3D_SHADER_SEL] = imx_clk_mux("gpu3d_shader_sel", base + 0x18, 8,  2, gpu3d_shader_sels, ARRAY_SIZE(gpu3d_shader_sels));
 	clk[IMX6QDL_CLK_IPU1_SEL]         = imx_clk_mux("ipu1_sel",         base + 0x3c, 9,  2, ipu_sels,          ARRAY_SIZE(ipu_sels));
 	clk[IMX6QDL_CLK_IPU2_SEL]         = imx_clk_mux("ipu2_sel",         base + 0x3c, 14, 2, ipu_sels,          ARRAY_SIZE(ipu_sels));
-	clk[IMX6QDL_CLK_LDB_DI0_SEL]      = imx_clk_mux_flags("ldb_di0_sel", base + 0x2c, 9,  3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT);
-	clk[IMX6QDL_CLK_LDB_DI1_SEL]      = imx_clk_mux_flags("ldb_di1_sel", base + 0x2c, 12, 3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT);
+	clk[IMX6QDL_CLK_LDB_DI0_SEL]      = imx_clk_mux_ldb("ldb_di0_sel", base + 0x2c, 9,  3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels));
+	clk[IMX6QDL_CLK_LDB_DI1_SEL]      = imx_clk_mux_ldb("ldb_di1_sel", base + 0x2c, 12, 3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels));
 	clk[IMX6QDL_CLK_IPU1_DI0_PRE_SEL] = imx_clk_mux_flags("ipu1_di0_pre_sel", base + 0x34, 6,  3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels), CLK_SET_RATE_PARENT);
 	clk[IMX6QDL_CLK_IPU1_DI1_PRE_SEL] = imx_clk_mux_flags("ipu1_di1_pre_sel", base + 0x34, 15, 3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels), CLK_SET_RATE_PARENT);
 	clk[IMX6QDL_CLK_IPU2_DI0_PRE_SEL] = imx_clk_mux_flags("ipu2_di0_pre_sel", base + 0x38, 6,  3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels), CLK_SET_RATE_PARENT);
@@ -593,12 +593,6 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 
 	clk_register_clkdev(clk[IMX6QDL_CLK_ENET_REF], "enet_ref", NULL);
 
-	if ((imx_get_soc_revision() != IMX_CHIP_REVISION_1_0) ||
-	    clk_on_imx6dl()) {
-		clk_set_parent(clk[IMX6QDL_CLK_LDB_DI0_SEL], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV]);
-		clk_set_parent(clk[IMX6QDL_CLK_LDB_DI1_SEL], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV]);
-	}
-
 	clk_set_rate(clk[IMX6QDL_CLK_PLL3_PFD1_540M], 540000000);
 	if (clk_on_imx6dl())
 		clk_set_parent(clk[IMX6QDL_CLK_IPU1_SEL], clk[IMX6QDL_CLK_PLL3_PFD1_540M]);
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 508d0fa..1d92065 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -143,6 +143,14 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
 			&imx_ccm_lock);
 }
 
+static inline struct clk *imx_clk_mux_ldb(const char *name, void __iomem *reg,
+		u8 shift, u8 width, const char **parents, int num_parents)
+{
+	return clk_register_mux(NULL, name, parents, num_parents,
+			CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT, reg,
+			shift, width, CLK_MUX_READ_ONLY, &imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_fixed_factor(const char *name,
 		const char *parent, unsigned int mult, unsigned int div)
 {
-- 
2.8.1

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Philipp Zabel]

From: Fabio Estevam <fabio.estevam@freescale.com>

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. The glitchy muxes are then registered as read-only. The clock
parent can be selected using the assigned-clocks and
assigned-clock-parents properties of the ccm device tree node:

	&clks {
		assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
				  <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
		assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>,
					 <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>;
	};

The issue is explained in detail in EB821 ("LDB Clock Switch Procedure &
i.MX6 Asynchronous Clock Switching Guidelines") [1].

[1] http://www.nxp.com/files/32bit/doc/eng_bulletin/EB821.pdf

Signed-off-by: Ranjani Vaidyanathan <Ranjani.Vaidyanathan@freescale.com>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v4:
 - Allow to switch from PLL3 to any other parent clock
---
 drivers/clk/imx/clk-imx6q.c | 264 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 259 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 9bbc994..fc3ed87 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -156,9 +156,90 @@ static struct clk ** const uart_clks[] __initconst = {
 	NULL
 };
 
+static int ldb_di_sel_by_clock_id(int clock_id)
+{
+	switch (clock_id) {
+	case IMX6QDL_CLK_PLL5_VIDEO_DIV:
+		if (clk_on_imx6q() &&
+		    imx_get_soc_revision() == IMX_CHIP_REVISION_1_0)
+			return -ENOENT;
+		return 0;
+	case IMX6QDL_CLK_PLL2_PFD0_352M:
+		return 1;
+	case IMX6QDL_CLK_PLL2_PFD2_396M:
+		return 2;
+	case IMX6QDL_CLK_MMDC_CH1_AXI:
+		return 3;
+	case IMX6QDL_CLK_PLL3_USB_OTG:
+		return 4;
+	default:
+		return -ENOENT;
+	}
+}
+
+static void of_assigned_ldb_sels(struct device_node *node,
+				 unsigned int *ldb_di0_sel,
+				 unsigned int *ldb_di1_sel)
+{
+	struct of_phandle_args clkspec;
+	int index, rc, num_parents;
+	int parent, child, sel;
+
+	num_parents = of_count_phandle_with_args(node, "assigned-clock-parents",
+						 "#clock-cells");
+	for (index = 0; index < num_parents; index++) {
+		rc = of_parse_phandle_with_args(node, "assigned-clock-parents",
+					"#clock-cells", index, &clkspec);
+		if (rc < 0) {
+			/* skip empty (null) phandles */
+			if (rc == -ENOENT)
+				continue;
+			else
+				return;
+		}
+		if (clkspec.np != node || clkspec.args[0] >= IMX6QDL_CLK_END) {
+			pr_err("ccm: parent clock %d not in ccm\n", index);
+			return;
+		}
+		parent = clkspec.args[0];
+
+		rc = of_parse_phandle_with_args(node, "assigned-clocks",
+				"#clock-cells", index, &clkspec);
+		if (rc < 0)
+			return;
+		if (clkspec.np != node || clkspec.args[0] >= IMX6QDL_CLK_END) {
+			pr_err("ccm: child clock %d not in ccm\n", index);
+			return;
+		}
+		child = clkspec.args[0];
+
+		if (child != IMX6QDL_CLK_LDB_DI0_SEL &&
+		    child != IMX6QDL_CLK_LDB_DI1_SEL)
+			continue;
+
+		sel = ldb_di_sel_by_clock_id(parent);
+		if (sel < 0) {
+			pr_err("ccm: invalid ldb_di%d parent clock: %d\n",
+			       child == IMX6QDL_CLK_LDB_DI1_SEL, parent);
+			continue;
+		}
+
+		if (child == IMX6QDL_CLK_LDB_DI0_SEL)
+			*ldb_di0_sel = sel;
+		if (child == IMX6QDL_CLK_LDB_DI1_SEL)
+			*ldb_di1_sel = sel;
+	}
+}
+
 #define CCM_CCDR		0x04
+#define CCM_CCSR		0x0c
+#define CCM_CS2CDR		0x2c
+
+#define CCDR_MMDC_CH1_MASK		BIT(16)
+#define CCSR_PLL3_SW_CLK_SEL		BIT(0)
 
-#define CCDR_MMDC_CH1_MASK	BIT(16)
+#define CS2CDR_LDB_DI0_CLK_SEL_SHIFT	9
+#define CS2CDR_LDB_DI1_CLK_SEL_SHIFT	12
 
 static void __init imx6q_mmdc_ch1_mask_handshake(void __iomem *ccm_base)
 {
@@ -169,10 +250,173 @@ static void __init imx6q_mmdc_ch1_mask_handshake(void __iomem *ccm_base)
 	writel_relaxed(reg, ccm_base + CCM_CCDR);
 }
 
+/*
+ * The only way to disable the MMDC_CH1 clock is to move it to pll3_sw_clk
+ * via periph2_clk2_sel and then to disable pll3_sw_clk by selecting the
+ * bypass clock source, since there is no CG bit for mmdc_ch1.
+ */
+static void mmdc_ch1_disable(void __iomem *ccm_base)
+{
+	unsigned int reg;
+
+	clk_set_parent(clk[IMX6QDL_CLK_PERIPH2_CLK2_SEL],
+		       clk[IMX6QDL_CLK_PLL3_USB_OTG]);
+
+	/*
+	 * Handshake with mmdc_ch1 module must be masked when changing
+	 * periph2_clk_sel.
+	 */
+	clk_set_parent(clk[IMX6QDL_CLK_PERIPH2], clk[IMX6QDL_CLK_PERIPH2_CLK2]);
+
+	/* Disable pll3_sw_clk by selecting the bypass clock source */
+	reg = readl_relaxed(ccm_base + CCM_CCSR);
+	reg |= CCSR_PLL3_SW_CLK_SEL;
+	writel_relaxed(reg, ccm_base + CCM_CCSR);
+}
+
+static void mmdc_ch1_reenable(void __iomem *ccm_base)
+{
+	unsigned int reg;
+
+	/* Enable pll3_sw_clk by disabling the bypass */
+	reg = readl_relaxed(ccm_base + CCM_CCSR);
+	reg &= ~CCSR_PLL3_SW_CLK_SEL;
+	writel_relaxed(reg, ccm_base + CCM_CCSR);
+
+	clk_set_parent(clk[IMX6QDL_CLK_PERIPH2], clk[IMX6QDL_CLK_PERIPH2_PRE]);
+}
+
+/*
+ * We have to follow a strict procedure when changing the LDB clock source,
+ * otherwise we risk introducing a glitch that can lock up the LDB divider.
+ * Things to keep in mind:
+ *
+ * 1. The current and new parent clock inputs to the mux must be disabled.
+ * 2. The default clock input for ldb_di0/1_clk_sel is mmdc_ch1_axi, which
+ *    has no CG bit.
+ * 3. pll2_pfd2_396m can not be gated if it is used as memory clock.
+ * 4. In the RTL implementation of the LDB_DI_CLK_SEL muxes the top four
+ *    options are in one mux and the PLL3 option along with three unused
+ *    inputs is in a second mux. There is a third mux with two inputs used
+ *    to decide between the first and second 4-port mux:
+ *
+ *    pll5_video_div 0 --|\
+ *    pll2_pfd0_352m 1 --| |_
+ *    pll2_pfd2_396m 2 --| | `-|\
+ *    mmdc_ch1_axi   3 --|/    | |
+ *                             | |--
+ *    pll3_usb_otg   4 --|\    | |
+ *                   5 --| |_,-|/
+ *                   6 --| |
+ *                   7 --|/
+ *
+ * The ldb_di0/1_clk_sel[1:0] bits control both 4-port muxes@the same time.
+ * The ldb_di0/1_clk_sel[2] bit controls the 2-port mux. The code below
+ * switches the parent to the bottom mux first and then manipulates the top
+ * mux to ensure that no glitch will enter the divider.
+ */
+static void init_ldb_clks(struct device_node *np, void __iomem *ccm_base)
+{
+	unsigned int reg;
+	unsigned int sel[2][4];
+	int i;
+
+	reg = readl_relaxed(ccm_base + CCM_CS2CDR);
+	sel[0][0] = (reg >> CS2CDR_LDB_DI0_CLK_SEL_SHIFT) & 7;
+	sel[1][0] = (reg >> CS2CDR_LDB_DI1_CLK_SEL_SHIFT) & 7;
+
+	sel[0][3] = sel[0][2] = sel[0][1] = sel[0][0];
+	sel[1][3] = sel[1][2] = sel[1][1] = sel[1][0];
+
+	of_assigned_ldb_sels(np, &sel[0][3], &sel[1][3]);
+
+	for (i = 0; i < 2; i++) {
+		/* Warn if a glitch might have been introduced already */
+		if (sel[i][0] != 3) {
+			pr_warn("ccm: ldb_di%d_sel already changed from reset value: %d\n",
+				i, sel[i][0]);
+		}
+
+		if (sel[i][0] == sel[i][3])
+			continue;
+
+		/* Only switch to or from pll2_pfd2_396m if it is disabled */
+		if ((sel[i][0] == 2 || sel[i][3] == 2) &&
+		    (clk_get_parent(clk[IMX6QDL_CLK_PERIPH_PRE]) ==
+		     clk[IMX6QDL_CLK_PLL2_PFD2_396M])) {
+			pr_err("ccm: ldb_di%d_sel: couldn't disable pll2_pfd2_396m\n",
+			       i);
+			sel[i][3] = sel[i][2] = sel[i][1] = sel[i][0];
+			continue;
+		}
+
+		/* First switch to the bottom mux */
+		sel[i][1] = sel[i][0] | 4;
+
+		/* Then configure the top mux before switching back to it */
+		sel[i][2] = sel[i][3] | 4;
+
+		pr_debug("ccm: switching ldb_di%d_sel: %d->%d->%d->%d\n", i,
+			 sel[i][0], sel[i][1], sel[i][2], sel[i][3]);
+	}
+
+	if (sel[0][0] == sel[0][3] && sel[1][0] == sel[1][3])
+		return;
+
+	mmdc_ch1_disable(ccm_base);
+
+	for (i = 1; i < 4; i++) {
+		reg = readl_relaxed(ccm_base + CCM_CS2CDR);
+		reg &= ~((7 << CS2CDR_LDB_DI0_CLK_SEL_SHIFT) |
+			 (7 << CS2CDR_LDB_DI1_CLK_SEL_SHIFT));
+		reg |= ((sel[0][i] << CS2CDR_LDB_DI0_CLK_SEL_SHIFT) |
+			(sel[1][i] << CS2CDR_LDB_DI1_CLK_SEL_SHIFT));
+		writel_relaxed(reg, ccm_base + CCM_CS2CDR);
+	}
+
+	mmdc_ch1_reenable(ccm_base);
+}
+
+#define CCM_ANALOG_PLL_VIDEO	0xa0
+#define CCM_ANALOG_PFD_480	0xf0
+#define CCM_ANALOG_PFD_528	0x100
+
+#define PLL_ENABLE		BIT(13)
+
+#define PFD0_CLKGATE		BIT(7)
+#define PFD1_CLKGATE		BIT(15)
+#define PFD2_CLKGATE		BIT(23)
+#define PFD3_CLKGATE		BIT(31)
+
+static void disable_anatop_clocks(void __iomem *anatop_base)
+{
+	unsigned int reg;
+
+	/* Make sure PLL2 PFDs 0-2 are gated */
+	reg = readl_relaxed(anatop_base + CCM_ANALOG_PFD_528);
+	/* Cannot gate PFD2 if pll2_pfd2_396m is the parent of MMDC clock */
+	if (clk_get_parent(clk[IMX6QDL_CLK_PERIPH_PRE]) ==
+	    clk[IMX6QDL_CLK_PLL2_PFD2_396M])
+		reg |= PFD0_CLKGATE | PFD1_CLKGATE;
+	else
+		reg |= PFD0_CLKGATE | PFD1_CLKGATE | PFD2_CLKGATE;
+	writel_relaxed(reg, anatop_base + CCM_ANALOG_PFD_528);
+
+	/* Make sure PLL3 PFDs 0-3 are gated */
+	reg = readl_relaxed(anatop_base + CCM_ANALOG_PFD_480);
+	reg |= PFD0_CLKGATE | PFD1_CLKGATE | PFD2_CLKGATE | PFD3_CLKGATE;
+	writel_relaxed(reg, anatop_base + CCM_ANALOG_PFD_480);
+
+	/* Make sure PLL5 is disabled */
+	reg = readl_relaxed(anatop_base + CCM_ANALOG_PLL_VIDEO);
+	reg &= ~PLL_ENABLE;
+	writel_relaxed(reg, anatop_base + CCM_ANALOG_PLL_VIDEO);
+}
+
 static void __init imx6q_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;
-	void __iomem *base;
+	void __iomem *anatop_base, *base;
 	int i;
 	int ret;
 
@@ -185,7 +429,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[IMX6QDL_CLK_ANACLK2] = imx_obtain_fixed_clock("anaclk2", 0);
 
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-anatop");
-	base = of_iomap(np, 0);
+	anatop_base = base = of_iomap(np, 0);
 	WARN_ON(!base);
 
 	/* Audio/video PLL post dividers do not work on i.MX6q revision 1.0 */
@@ -310,8 +554,6 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	base = of_iomap(np, 0);
 	WARN_ON(!base);
 
-	imx6q_mmdc_ch1_mask_handshake(base);
-
 	/*                                              name                reg       shift width parent_names     num_parents */
 	clk[IMX6QDL_CLK_STEP]             = imx_clk_mux("step",	            base + 0xc,  8,  1, step_sels,	   ARRAY_SIZE(step_sels));
 	clk[IMX6QDL_CLK_PLL1_SW]          = imx_clk_mux("pll1_sw",	    base + 0xc,  2,  1, pll1_sw_sels,      ARRAY_SIZE(pll1_sw_sels));
@@ -340,6 +582,18 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[IMX6QDL_CLK_GPU3D_SHADER_SEL] = imx_clk_mux("gpu3d_shader_sel", base + 0x18, 8,  2, gpu3d_shader_sels, ARRAY_SIZE(gpu3d_shader_sels));
 	clk[IMX6QDL_CLK_IPU1_SEL]         = imx_clk_mux("ipu1_sel",         base + 0x3c, 9,  2, ipu_sels,          ARRAY_SIZE(ipu_sels));
 	clk[IMX6QDL_CLK_IPU2_SEL]         = imx_clk_mux("ipu2_sel",         base + 0x3c, 14, 2, ipu_sels,          ARRAY_SIZE(ipu_sels));
+
+	disable_anatop_clocks(anatop_base);
+
+	imx6q_mmdc_ch1_mask_handshake(base);
+
+	/*
+	 * The LDB_DI0/1_SEL muxes are registered read-only due to a hardware
+	 * bug. Set the muxes to the requested values before registering the
+	 * ldb_di_sel clocks.
+	 */
+	init_ldb_clks(np, base);
+
 	clk[IMX6QDL_CLK_LDB_DI0_SEL]      = imx_clk_mux_ldb("ldb_di0_sel", base + 0x2c, 9,  3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels));
 	clk[IMX6QDL_CLK_LDB_DI1_SEL]      = imx_clk_mux_ldb("ldb_di1_sel", base + 0x2c, 12, 3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels));
 	clk[IMX6QDL_CLK_IPU1_DI0_PRE_SEL] = imx_clk_mux_flags("ipu1_di0_pre_sel", base + 0x34, 6,  3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels), CLK_SET_RATE_PARENT);
-- 
2.8.1

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Akshay Bhat]

On 07/11/2016 07:12 AM, Philipp Zabel wrote:
> From: Fabio Estevam<fabio.estevam@freescale.com>
>
> 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. The glitchy muxes are then registered as read-only. The clock
> parent can be selected using the assigned-clocks and
> assigned-clock-parents properties of the ccm device tree node:
>
> 	&clks {
> 		assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
> 				  <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
> 		assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>,
> 					 <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>;
> 	};
>
> The issue is explained in detail in EB821 ("LDB Clock Switch Procedure &
> i.MX6 Asynchronous Clock Switching Guidelines") [1].
>
> [1]http://www.nxp.com/files/32bit/doc/eng_bulletin/EB821.pdf
>
> Signed-off-by: Ranjani Vaidyanathan<Ranjani.Vaidyanathan@freescale.com>
> Signed-off-by: Fabio Estevam<fabio.estevam@freescale.com>
> Signed-off-by: Philipp Zabel<p.zabel@pengutronix.de>
> ---

Reviewed-by: Akshay Bhat <akshay.bhat@timesys.com>

Thanks.

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Fabio Estevam]

Hi Sean,

On Mon, Jul 11, 2016 at 8:12 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> 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. The glitchy muxes are then registered as read-only. The clock
> parent can be selected using the assigned-clocks and
> assigned-clock-parents properties of the ccm device tree node:
>
>         &clks {
>                 assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
>                                   <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
>                 assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>,
>                                          <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>;
>         };
>
> The issue is explained in detail in EB821 ("LDB Clock Switch Procedure &
> i.MX6 Asynchronous Clock Switching Guidelines") [1].
>
> [1] http://www.nxp.com/files/32bit/doc/eng_bulletin/EB821.pdf
>
> Signed-off-by: Ranjani Vaidyanathan <Ranjani.Vaidyanathan@freescale.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

On a previous version of the patch you reported issues on Novena:
http://www.spinics.net/lists/arm-kernel/msg476324.html

Does v5 work fine on your board?

Christian,

I remember you had an automated way of testing this LDB glitch
problem. Would it be possible to run v5 through your test setup?

Thanks,

Fabio Estevam

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Joshua Clayton]

Hi Philipp,
My team is testing this morning on a board that exhibits
intermittent lvds failures that seem to match this problem.
(black screen and a "dc stop timeout" message in the kernel log)
With this patch set, the screen failed to come up on the second boot :(
But...
On 07/11/2016 04:12 AM, Philipp Zabel wrote:
> +static void init_ldb_clks(struct device_node *np, void __iomem *ccm_base)
> +{
> +	unsigned int reg;
> +	unsigned int sel[2][4];
> +	int i;
> +
to feel confident the new code was running, one of our engineers added the line
        pr_warn("ccm: In init_ldb_clks\n");
right here.
> +	reg = readl_relaxed(ccm_base + CCM_CS2CDR);
> +	sel[0][0] = (reg >> CS2CDR_LDB_DI0_CLK_SEL_SHIFT) & 7;
> +	sel[1][0] = (reg >> CS2CDR_LDB_DI1_CLK_SEL_SHIFT) & 7;
> +
> +	sel[0][3] = sel[0][2] = sel[0][1] = sel[0][0];
> +	sel[1][3] = sel[1][2] = sel[1][1] = sel[1][0];
> +
> +	of_assigned_ldb_sels(np, &sel[0][3], &sel[1][3]);
>
...

We have now booted that same board successfully about 30 times
with your patches plus the extra printf .?!??

Not sure what that means in terms of delay and synchronization, so
even though we are not finished testing I thought I should post that result.

Thanks,
Joshua Clayton

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Joshua Clayton]

Hi again,

On Thursday, July 14, 2016 10:01:12 AM Joshua Clayton wrote:
> Hi Philipp,
> My team is testing this morning on a board that exhibits
> intermittent lvds failures that seem to match this problem.
> (black screen and a "dc stop timeout" message in the kernel log)
> With this patch set, the screen failed to come up on the second boot :(
> But...
> On 07/11/2016 04:12 AM, Philipp Zabel wrote:
> > +static void init_ldb_clks(struct device_node *np, void __iomem *ccm_base)
> > +{
> > +	unsigned int reg;
> > +	unsigned int sel[2][4];
> > +	int i;
> > +
> to feel confident the new code was running, one of our engineers added the line
>         pr_warn("ccm: In init_ldb_clks\n");
> right here.
Took the printk out and the patch set still works as expected.
...so the previous email was a false alarm. Mea culpa
Problem solved I think. Thanks Philipp!

for my hardware (imx6q-evi) using xorg and  lvds

Tested-by Joshua Clayton <stillcompiling@gmail.com>
> > +	reg = readl_relaxed(ccm_base + CCM_CS2CDR);
> > +	sel[0][0] = (reg >> CS2CDR_LDB_DI0_CLK_SEL_SHIFT) & 7;
> > +	sel[1][0] = (reg >> CS2CDR_LDB_DI1_CLK_SEL_SHIFT) & 7;
> > +
> > +	sel[0][3] = sel[0][2] = sel[0][1] = sel[0][0];
> > +	sel[1][3] = sel[1][2] = sel[1][1] = sel[1][0];
> > +
> > +	of_assigned_ldb_sels(np, &sel[0][3], &sel[1][3]);
> >
> ...
> 
> We have now booted that same board successfully about 30 times
> with your patches plus the extra printf .?!??
> 
> Not sure what that means in terms of delay and synchronization, so
> even though we are not finished testing I thought I should post that result.
> 
> Thanks,
> Joshua Clayton

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Fabio Estevam]

Hi Joshua,

On Sat, Jul 16, 2016 at 5:55 PM, Joshua Clayton
<stillcompiling@gmail.com> wrote:

> Took the printk out and the patch set still works as expected.
> ...so the previous email was a false alarm. Mea culpa
> Problem solved I think. Thanks Philipp!
>
> for my hardware (imx6q-evi) using xorg and  lvds
>
> Tested-by Joshua Clayton <stillcompiling@gmail.com>

Great news! It is good to know you are able to confirm this series
fixes the LVDS issue on your board.

Still interested to know if Novena LVDS still work with this series applied.

Sean,Marek,

Any chances of testing this series on Novena's board?

Thanks

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Akshay Bhat]

On Tue, Jul 19, 2016 at 7:13 AM, Fabio Estevam <festevam@gmail.com> wrote:
>
> Great news! It is good to know you are able to confirm this series
> fixes the LVDS issue on your board.
>
> Still interested to know if Novena LVDS still work with this series applied.
>
> Sean,Marek,
>
> Any chances of testing this series on Novena's board?
>
> Thanks

Hi Fabio, Shawn,

Advantech tested the v5 patch series on imx6q-b450v3 board (with 4.7
kernel) and confirmed the patch fixes the lvds blank display issue
being seen on some imx6q-b450v3 boards.

Details below:
With the patch applied:
Board # 1: 100 times power on/off, no LVDS display failure
Board # 2: 100 times power on/off, no LVDS display failure

Without the patch:
Board # 1: LVDS display failure (blank display output) at 4th time power on/off.
Board # 2: LVDS display failure (blank display output) at 3rd time power on/off.

Tested-by: Charles Kang <Charles.Kang@advantech.com.tw>

Since the patch has been reviewed and confirmed to fix display
failures on 2 different boards (imx6q-evi and imx6q-b450v3), can it be
applied?

Thanks,
Akshay

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Fabio Estevam]

Hi Akshay and Charles,

On Mon, Sep 12, 2016 at 12:09 PM, Akshay Bhat <akshay.bhat@timesys.com> wrote:

> Hi Fabio, Shawn,
>
> Advantech tested the v5 patch series on imx6q-b450v3 board (with 4.7
> kernel) and confirmed the patch fixes the lvds blank display issue
> being seen on some imx6q-b450v3 boards.
>
> Details below:
> With the patch applied:
> Board # 1: 100 times power on/off, no LVDS display failure
> Board # 2: 100 times power on/off, no LVDS display failure
>
> Without the patch:
> Board # 1: LVDS display failure (blank display output) at 4th time power on/off.
> Board # 2: LVDS display failure (blank display output) at 3rd time power on/off.
>
> Tested-by: Charles Kang <Charles.Kang@advantech.com.tw>

Thanks for testing it.

> Since the patch has been reviewed and confirmed to fix display
> failures on 2 different boards (imx6q-evi and imx6q-b450v3), can it be
> applied?

Yes, I think this series is in a good shape now.

Shawn,

Are you happy with it?

Thanks

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Shawn Guo]

On Mon, Sep 12, 2016 at 12:15:40PM -0300, Fabio Estevam wrote:
> Hi Akshay and Charles,
> 
> On Mon, Sep 12, 2016 at 12:09 PM, Akshay Bhat <akshay.bhat@timesys.com> wrote:
> 
> > Hi Fabio, Shawn,
> >
> > Advantech tested the v5 patch series on imx6q-b450v3 board (with 4.7
> > kernel) and confirmed the patch fixes the lvds blank display issue
> > being seen on some imx6q-b450v3 boards.
> >
> > Details below:
> > With the patch applied:
> > Board # 1: 100 times power on/off, no LVDS display failure
> > Board # 2: 100 times power on/off, no LVDS display failure
> >
> > Without the patch:
> > Board # 1: LVDS display failure (blank display output) at 4th time power on/off.
> > Board # 2: LVDS display failure (blank display output) at 3rd time power on/off.
> >
> > Tested-by: Charles Kang <Charles.Kang@advantech.com.tw>
> 
> Thanks for testing it.
> 
> > Since the patch has been reviewed and confirmed to fix display
> > failures on 2 different boards (imx6q-evi and imx6q-b450v3), can it be
> > applied?
> 
> Yes, I think this series is in a good shape now.
> 
> Shawn,
> 
> Are you happy with it?

Acked-by: Shawn Guo <shawnguo@kernel.org>

Please send it to clk maintainers for applying.

Shawn

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Fabio Estevam]

On Thu, Sep 15, 2016 at 10:36 PM, Shawn Guo <shawnguo@kernel.org> wrote:

>> Shawn,
>>
>> Are you happy with it?
>
> Acked-by: Shawn Guo <shawnguo@kernel.org>
>
> Please send it to clk maintainers for applying.

Thanks. Just sent it to the clk folks.

[PATCH v5 3/3] clk: imx6: Fix procedure to switch the parent of LDB_DI_CLK

[Philipp Zabel]

Am Freitag, den 16.09.2016, 11:33 -0300 schrieb Fabio Estevam:
> On Thu, Sep 15, 2016 at 10:36 PM, Shawn Guo <shawnguo@kernel.org> wrote:
> 
> >> Shawn,
> >>
> >> Are you happy with it?
> >
> > Acked-by: Shawn Guo <shawnguo@kernel.org>
> >
> > Please send it to clk maintainers for applying.
> 
> Thanks. Just sent it to the clk folks.

Thanks!