[RFC PATCH 0/2] ARM: imx6: add BYPASS support for PLLs

This RFC patch series adds BYPASS support for i.MX6 PLLs.  It's
motivated by an ESAI audio use case, where on-board audio codec provides
the master clock to ESAI controller via path anaclk pad -->
PLL_AUDIO --> ESAI.  This is not possible with the current clock driver
implementation, where clock source of PLL is hard-coded to be OSC24M.
The first patch adds an example implementation for i.MX6Q clock driver
to make above clock path become possible.

The second patch is only for showing code removal from clk-pllv3 driver,
and shouldn't be applied until all i.MX6 clock drivers handle BYPASS and
ENABLE on their own, just like i.MX6Q clock driver does in the first
patch.

Shawn Guo (2):
  ARM: imx: add BYPASS support for PLL clocks
  ARM: imx: remove ENABLE and BYPASS bits from clk-pllv3 driver

 arch/arm/mach-imx/clk-imx6q.c             | 57 ++++++++++++++++++++++++++-----
 arch/arm/mach-imx/clk-pllv3.c             | 37 --------------------
 include/dt-bindings/clock/imx6qdl-clock.h | 23 ++++++++++++-
 3 files changed, 71 insertions(+), 46 deletions(-)

-- 
1.9.1

The i.MX6 clock drivers currently hard-code all PLL clocks sourcing from
OSC24M without BYPASS support.  The patch adds BYPASS and BYPASS_CLK_SRC
selection for PLL clocks as per Figure 10-3. Primary Clock Generation
in IMX6DQRM, i.e. both BYPASS_CLK_SRC and BYPASS bits are implemented as
mux clocks, and ENABLE bit of PLL clocks is implemented as a gate clock
after BYPASS mux.

Signed-off-by: Shawn Guo <redacted>
---
 arch/arm/mach-imx/clk-imx6q.c             | 57 ++++++++++++++++++++++++++-----
 include/dt-bindings/clock/imx6qdl-clock.h | 23 ++++++++++++-
 2 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index e6c60188ec14..c1199a26c24e 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c

@@ -73,6 +73,14 @@ static const char *lvds_sels[] = {
 	"pll4_audio", "pll5_video", "pll8_mlb", "enet_ref",
 	"pcie_ref_125m", "sata_ref_100m",
 };
+static const char *pll_bypass_src_sels[] = { "osc", "anaclk1", "anaclk2", "dummy", };
+static const char *pll1_bypass_sels[] = { "pll1", "pll1_bypass_src", };
+static const char *pll2_bypass_sels[] = { "pll2", "pll2_bypass_src", };
+static const char *pll3_bypass_sels[] = { "pll3", "pll3_bypass_src", };
+static const char *pll4_bypass_sels[] = { "pll4", "pll4_bypass_src", };
+static const char *pll5_bypass_sels[] = { "pll5", "pll5_bypass_src", };
+static const char *pll6_bypass_sels[] = { "pll6", "pll6_bypass_src", };
+static const char *pll7_bypass_sels[] = { "pll7", "pll7_bypass_src", };
 
 static struct clk *clk[IMX6QDL_CLK_END];
 static struct clk_onecell_data clk_data;

@@ -135,14 +143,47 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 		video_div_table[2].div = 1;
 	};
 
-	/*                                             type             name         parent_name  base     div_mask */
-	clk[IMX6QDL_CLK_PLL1_SYS]      = imx_clk_pllv3(IMX_PLLV3_SYS,	"pll1_sys",	"osc", base,        0x7f);
-	clk[IMX6QDL_CLK_PLL2_BUS]      = imx_clk_pllv3(IMX_PLLV3_GENERIC,	"pll2_bus",	"osc", base + 0x30, 0x1);
-	clk[IMX6QDL_CLK_PLL3_USB_OTG]  = imx_clk_pllv3(IMX_PLLV3_USB,	"pll3_usb_otg",	"osc", base + 0x10, 0x3);
-	clk[IMX6QDL_CLK_PLL4_AUDIO]    = imx_clk_pllv3(IMX_PLLV3_AV,	"pll4_audio",	"osc", base + 0x70, 0x7f);
-	clk[IMX6QDL_CLK_PLL5_VIDEO]    = imx_clk_pllv3(IMX_PLLV3_AV,	"pll5_video",	"osc", base + 0xa0, 0x7f);
-	clk[IMX6QDL_CLK_PLL6_ENET]     = imx_clk_pllv3(IMX_PLLV3_ENET,	"pll6_enet",	"osc", base + 0xe0, 0x3);
-	clk[IMX6QDL_CLK_PLL7_USB_HOST] = imx_clk_pllv3(IMX_PLLV3_USB,	"pll7_usb_host","osc", base + 0x20, 0x3);
+	clk[IMX6QDL_PLL1_BYPASS_SRC] = imx_clk_mux("pll1_bypass_src", base + 0x00, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
+	clk[IMX6QDL_PLL2_BYPASS_SRC] = imx_clk_mux("pll2_bypass_src", base + 0x30, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
+	clk[IMX6QDL_PLL3_BYPASS_SRC] = imx_clk_mux("pll3_bypass_src", base + 0x10, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
+	clk[IMX6QDL_PLL4_BYPASS_SRC] = imx_clk_mux("pll4_bypass_src", base + 0x70, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
+	clk[IMX6QDL_PLL5_BYPASS_SRC] = imx_clk_mux("pll5_bypass_src", base + 0xa0, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
+	clk[IMX6QDL_PLL6_BYPASS_SRC] = imx_clk_mux("pll6_bypass_src", base + 0xe0, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
+	clk[IMX6QDL_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", base + 0x20, 14, 2, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
+
+	/*                                    type               name    parent_name        base         div_mask */
+	clk[IMX6QDL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS,     "pll1", "pll1_bypass_src", base + 0x00, 0x7f);
+	clk[IMX6QDL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", base + 0x30, 0x1);
+	clk[IMX6QDL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll3", "pll3_bypass_src", base + 0x10, 0x3);
+	clk[IMX6QDL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "pll4_bypass_src", base + 0x70, 0x7f);
+	clk[IMX6QDL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll5", "pll5_bypass_src", base + 0xa0, 0x7f);
+	clk[IMX6QDL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll6", "pll6_bypass_src", base + 0xe0, 0x3);
+	clk[IMX6QDL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB,     "pll7", "pll7_bypass_src", base + 0x20, 0x3);
+
+	clk[IMX6QDL_PLL1_BYPASS] = imx_clk_mux("pll1_bypass", base + 0x00, 16, 1, pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels));
+	clk[IMX6QDL_PLL2_BYPASS] = imx_clk_mux("pll2_bypass", base + 0x30, 16, 1, pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels));
+	clk[IMX6QDL_PLL3_BYPASS] = imx_clk_mux("pll3_bypass", base + 0x10, 16, 1, pll3_bypass_sels, ARRAY_SIZE(pll3_bypass_sels));
+	clk[IMX6QDL_PLL4_BYPASS] = imx_clk_mux("pll4_bypass", base + 0x70, 16, 1, pll4_bypass_sels, ARRAY_SIZE(pll4_bypass_sels));
+	clk[IMX6QDL_PLL5_BYPASS] = imx_clk_mux("pll5_bypass", base + 0xa0, 16, 1, pll5_bypass_sels, ARRAY_SIZE(pll5_bypass_sels));
+	clk[IMX6QDL_PLL6_BYPASS] = imx_clk_mux("pll6_bypass", base + 0xe0, 16, 1, pll6_bypass_sels, ARRAY_SIZE(pll6_bypass_sels));
+	clk[IMX6QDL_PLL7_BYPASS] = imx_clk_mux("pll7_bypass", base + 0x20, 16, 1, pll7_bypass_sels, ARRAY_SIZE(pll7_bypass_sels));
+
+	/* Do not bypass PLLs initially */
+	clk_set_parent(clk[IMX6QDL_PLL1_BYPASS], clk[IMX6QDL_CLK_PLL1]);
+	clk_set_parent(clk[IMX6QDL_PLL2_BYPASS], clk[IMX6QDL_CLK_PLL2]);
+	clk_set_parent(clk[IMX6QDL_PLL3_BYPASS], clk[IMX6QDL_CLK_PLL3]);
+	clk_set_parent(clk[IMX6QDL_PLL4_BYPASS], clk[IMX6QDL_CLK_PLL4]);
+	clk_set_parent(clk[IMX6QDL_PLL5_BYPASS], clk[IMX6QDL_CLK_PLL5]);
+	clk_set_parent(clk[IMX6QDL_PLL6_BYPASS], clk[IMX6QDL_CLK_PLL6]);
+	clk_set_parent(clk[IMX6QDL_PLL7_BYPASS], clk[IMX6QDL_CLK_PLL7]);
+
+	clk[IMX6QDL_CLK_PLL1_SYS]      = imx_clk_gate("pll1_sys",      "pll1_bypass", base + 0x00, 13);
+	clk[IMX6QDL_CLK_PLL2_BUS]      = imx_clk_gate("pll2_bus",      "pll2_bypass", base + 0x30, 13);
+	clk[IMX6QDL_CLK_PLL3_USB_OTG]  = imx_clk_gate("pll3_usb_otg",  "pll3_bypass", base + 0x10, 13);
+	clk[IMX6QDL_CLK_PLL4_AUDIO]    = imx_clk_gate("pll4_audio",    "pll4_bypass", base + 0x70, 13);
+	clk[IMX6QDL_CLK_PLL5_VIDEO]    = imx_clk_gate("pll5_video",    "pll5_bypass", base + 0xa0, 13);
+	clk[IMX6QDL_CLK_PLL6_ENET]     = imx_clk_gate("pll6_enet",     "pll6_bypass", base + 0xe0, 13);
+	clk[IMX6QDL_CLK_PLL7_USB_HOST] = imx_clk_gate("pll7_usb_host", "pll7_bypass", base + 0xe0, 13);
 
 	/*
 	 * Bit 20 is the reserved and read-only bit, we do this only for:

diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
index e992ce5e05a5..361afd09c2b7 100644
--- a/include/dt-bindings/clock/imx6qdl-clock.h
+++ b/include/dt-bindings/clock/imx6qdl-clock.h

@@ -224,6 +224,27 @@
 #define IMX6QDL_CLK_LVDS2_IN			211
 #define IMX6QDL_CLK_ANACLK1			212
 #define IMX6QDL_CLK_ANACLK2			213
-#define IMX6QDL_CLK_END				214
+#define IMX6QDL_PLL1_BYPASS_SRC			214
+#define IMX6QDL_PLL2_BYPASS_SRC			215
+#define IMX6QDL_PLL3_BYPASS_SRC			216
+#define IMX6QDL_PLL4_BYPASS_SRC			217
+#define IMX6QDL_PLL5_BYPASS_SRC			218
+#define IMX6QDL_PLL6_BYPASS_SRC			219
+#define IMX6QDL_PLL7_BYPASS_SRC			220
+#define IMX6QDL_CLK_PLL1			221
+#define IMX6QDL_CLK_PLL2			222
+#define IMX6QDL_CLK_PLL3			223
+#define IMX6QDL_CLK_PLL4			224
+#define IMX6QDL_CLK_PLL5			225
+#define IMX6QDL_CLK_PLL6			226
+#define IMX6QDL_CLK_PLL7			227
+#define IMX6QDL_PLL1_BYPASS			228
+#define IMX6QDL_PLL2_BYPASS			229
+#define IMX6QDL_PLL3_BYPASS			230
+#define IMX6QDL_PLL4_BYPASS			231
+#define IMX6QDL_PLL5_BYPASS			232
+#define IMX6QDL_PLL6_BYPASS			233
+#define IMX6QDL_PLL7_BYPASS			234
+#define IMX6QDL_CLK_END				235
 
 #endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */

-- 
1.9.1

Since ENABLE and BYPASS bits of PLLs are now implemented as separate
gate and mux clocks by clock drivers, the code handling these two bits
can be removed from clk-pllv3 driver.

Signed-off-by: Shawn Guo <redacted>
---
 arch/arm/mach-imx/clk-pllv3.c | 37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/arch/arm/mach-imx/clk-pllv3.c b/arch/arm/mach-imx/clk-pllv3.c
index 61364050fccd..57de74da0acf 100644
--- a/arch/arm/mach-imx/clk-pllv3.c
+++ b/arch/arm/mach-imx/clk-pllv3.c

@@ -23,8 +23,6 @@
 #define PLL_DENOM_OFFSET	0x20
 
 #define BM_PLL_POWER		(0x1 << 12)
-#define BM_PLL_ENABLE		(0x1 << 13)
-#define BM_PLL_BYPASS		(0x1 << 16)
 #define BM_PLL_LOCK		(0x1 << 31)
 
 /**

@@ -84,10 +82,6 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 	if (ret)
 		return ret;
 
-	val = readl_relaxed(pll->base);
-	val &= ~BM_PLL_BYPASS;
-	writel_relaxed(val, pll->base);
-
 	return 0;
 }
 

@@ -97,7 +91,6 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 	u32 val;
 
 	val = readl_relaxed(pll->base);
-	val |= BM_PLL_BYPASS;
 	if (pll->powerup_set)
 		val &= ~BM_PLL_POWER;
 	else

@@ -105,28 +98,6 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
 	writel_relaxed(val, pll->base);
 }
 
-static int clk_pllv3_enable(struct clk_hw *hw)
-{
-	struct clk_pllv3 *pll = to_clk_pllv3(hw);
-	u32 val;
-
-	val = readl_relaxed(pll->base);
-	val |= BM_PLL_ENABLE;
-	writel_relaxed(val, pll->base);
-
-	return 0;
-}
-
-static void clk_pllv3_disable(struct clk_hw *hw)
-{
-	struct clk_pllv3 *pll = to_clk_pllv3(hw);
-	u32 val;
-
-	val = readl_relaxed(pll->base);
-	val &= ~BM_PLL_ENABLE;
-	writel_relaxed(val, pll->base);
-}
-
 static unsigned long clk_pllv3_recalc_rate(struct clk_hw *hw,
 					   unsigned long parent_rate)
 {

@@ -169,8 +140,6 @@ static int clk_pllv3_set_rate(struct clk_hw *hw, unsigned long rate,
 static const struct clk_ops clk_pllv3_ops = {
 	.prepare	= clk_pllv3_prepare,
 	.unprepare	= clk_pllv3_unprepare,
-	.enable		= clk_pllv3_enable,
-	.disable	= clk_pllv3_disable,
 	.recalc_rate	= clk_pllv3_recalc_rate,
 	.round_rate	= clk_pllv3_round_rate,
 	.set_rate	= clk_pllv3_set_rate,

@@ -225,8 +194,6 @@ static int clk_pllv3_sys_set_rate(struct clk_hw *hw, unsigned long rate,
 static const struct clk_ops clk_pllv3_sys_ops = {
 	.prepare	= clk_pllv3_prepare,
 	.unprepare	= clk_pllv3_unprepare,
-	.enable		= clk_pllv3_enable,
-	.disable	= clk_pllv3_disable,
 	.recalc_rate	= clk_pllv3_sys_recalc_rate,
 	.round_rate	= clk_pllv3_sys_round_rate,
 	.set_rate	= clk_pllv3_sys_set_rate,

@@ -299,8 +266,6 @@ static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
 static const struct clk_ops clk_pllv3_av_ops = {
 	.prepare	= clk_pllv3_prepare,
 	.unprepare	= clk_pllv3_unprepare,
-	.enable		= clk_pllv3_enable,
-	.disable	= clk_pllv3_disable,
 	.recalc_rate	= clk_pllv3_av_recalc_rate,
 	.round_rate	= clk_pllv3_av_round_rate,
 	.set_rate	= clk_pllv3_av_set_rate,

@@ -315,8 +280,6 @@ static unsigned long clk_pllv3_enet_recalc_rate(struct clk_hw *hw,
 static const struct clk_ops clk_pllv3_enet_ops = {
 	.prepare	= clk_pllv3_prepare,
 	.unprepare	= clk_pllv3_unprepare,
-	.enable		= clk_pllv3_enable,
-	.disable	= clk_pllv3_disable,
 	.recalc_rate	= clk_pllv3_enet_recalc_rate,
 };
 

-- 
1.9.1

On Tue, Aug 26, 2014 at 11:35:12PM +0800, Shawn Guo wrote:

The i.MX6 clock drivers currently hard-code all PLL clocks sourcing from
OSC24M without BYPASS support.  The patch adds BYPASS and BYPASS_CLK_SRC
selection for PLL clocks as per Figure 10-3. Primary Clock Generation
in IMX6DQRM, i.e. both BYPASS_CLK_SRC and BYPASS bits are implemented as
mux clocks, and ENABLE bit of PLL clocks is implemented as a gate clock
after BYPASS mux.

Signed-off-by: Shawn Guo <redacted>

Shengjiu,

Can you please test if this BYPASS support works good for your ESAI use
case?

Shawn

On Thu, Aug 28, 2014 at 04:44:21PM +0800, Shawn Guo wrote:

On Tue, Aug 26, 2014 at 11:35:12PM +0800, Shawn Guo wrote:

quoted

The i.MX6 clock drivers currently hard-code all PLL clocks sourcing from
OSC24M without BYPASS support.  The patch adds BYPASS and BYPASS_CLK_SRC
selection for PLL clocks as per Figure 10-3. Primary Clock Generation
in IMX6DQRM, i.e. both BYPASS_CLK_SRC and BYPASS bits are implemented as
mux clocks, and ENABLE bit of PLL clocks is implemented as a gate clock
after BYPASS mux.

Signed-off-by: Shawn Guo <redacted>

Shengjiu,

Can you please test if this BYPASS support works good for your ESAI use
case?

Shawn

Yes, after test, one issue found. Please check.

-static const char *pll_bypass_src_sels[] = { "osc", "anaclk1", "anaclk2", "dummy", };
+static const char *pll_bypass_src_sels[] = { "osc", "lvds1_in", "lvds2_in", "dummy", };

best regrads
wang shengjiu

On Fri, Aug 29, 2014 at 09:49:50AM +0800, Shengjiu Wang wrote:

On Thu, Aug 28, 2014 at 04:44:21PM +0800, Shawn Guo wrote:

quoted

On Tue, Aug 26, 2014 at 11:35:12PM +0800, Shawn Guo wrote:

quoted

The i.MX6 clock drivers currently hard-code all PLL clocks sourcing from
OSC24M without BYPASS support.  The patch adds BYPASS and BYPASS_CLK_SRC
selection for PLL clocks as per Figure 10-3. Primary Clock Generation
in IMX6DQRM, i.e. both BYPASS_CLK_SRC and BYPASS bits are implemented as
mux clocks, and ENABLE bit of PLL clocks is implemented as a gate clock
after BYPASS mux.

Signed-off-by: Shawn Guo <redacted>

Shengjiu,

Can you please test if this BYPASS support works good for your ESAI use
case?

Shawn

Yes, after test, one issue found. Please check.

-static const char *pll_bypass_src_sels[] = { "osc", "anaclk1", "anaclk2", "dummy", };
+static const char *pll_bypass_src_sels[] = { "osc", "lvds1_in", "lvds2_in", "dummy", };

Ah, yes.  I will fix my code.

So doest that mean with the above fix, we can now source 24.576MHz from
board directly to ESAI with audio PLL in bypass mode?

Shawn

On Fri, Aug 29, 2014 at 10:53:51AM +0800, Shawn Guo wrote:

On Fri, Aug 29, 2014 at 09:49:50AM +0800, Shengjiu Wang wrote:

quoted

On Thu, Aug 28, 2014 at 04:44:21PM +0800, Shawn Guo wrote:

quoted

On Tue, Aug 26, 2014 at 11:35:12PM +0800, Shawn Guo wrote:

quoted

The i.MX6 clock drivers currently hard-code all PLL clocks sourcing from
OSC24M without BYPASS support.  The patch adds BYPASS and BYPASS_CLK_SRC
selection for PLL clocks as per Figure 10-3. Primary Clock Generation
in IMX6DQRM, i.e. both BYPASS_CLK_SRC and BYPASS bits are implemented as
mux clocks, and ENABLE bit of PLL clocks is implemented as a gate clock
after BYPASS mux.

Signed-off-by: Shawn Guo <redacted>

Shengjiu,

Can you please test if this BYPASS support works good for your ESAI use
case?

Shawn

Yes, after test, one issue found. Please check.

-static const char *pll_bypass_src_sels[] = { "osc", "anaclk1", "anaclk2", "dummy", };
+static const char *pll_bypass_src_sels[] = { "osc", "lvds1_in", "lvds2_in", "dummy", };

Ah, yes.  I will fix my code.

So doest that mean with the above fix, we can now source 24.576MHz from
board directly to ESAI with audio PLL in bypass mode?

Shawn

Otherwise I still need to do some step in below to source the clock to ESAI.

        clk_set_parent(pll4_bypass_src, lvds2_in);
        clk_set_parent(pll4_bypass, pll4_bypass_src);
        clk_set_rate(pll4_audio_div, 24576000);
        clk_set_rate(esai_extal, 24576000);

I think it is what we can expect, right? if yes, the patch is ok for ESAI.

wang shengjiu

On Fri, Aug 29, 2014 at 11:33:47AM +0800, Shengjiu Wang wrote:

Otherwise I still need to do some step in below to source the clock to ESAI.

        clk_set_parent(pll4_bypass_src, lvds2_in);
        clk_set_parent(pll4_bypass, pll4_bypass_src);
        clk_set_rate(pll4_audio_div, 24576000);
        clk_set_rate(esai_extal, 24576000);

I think it is what we can expect, right? if yes, the patch is ok for ESAI.

Yes, we still need to set up parent and rate specifically for
sabreauto ESAI use case.

But these setup can be done in device tree now with commit 86be408bfbd8
(clk: Support for clock parents and rates assigned from device tree).
Here is change that I tested with.  (Not sure if the info in esai node
is all correct)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 6692115b0138..9fe4169c79dd 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi

@@ -268,8 +268,18 @@
                                };

                                esai: esai at 02024000 {
+                                       compatible = "fsl,imx6q-esai", "fsl,imx35-esai";
                                        reg = <0x02024000 0x4000>;
                                        interrupts = <0 51 IRQ_TYPE_LEVEL_HIGH>;
+                                       clocks = <&clks IMX6QDL_CLK_ESAI_IPG>,
+                                                <&clks IMX6QDL_CLK_ESAI_EXTAL>,
+                                                <&clks IMX6QDL_CLK_ESAI_IPG>;
+                                       clock-names = "core", "extal", "fsys";
+                                       dmas = <&sdma 23 21 0>, <&sdma 24 21 0>;
+                                       dma-names = "rx", "tx";
+                                       fsl,fifo-depth = <128>;
+                                       fsl,esai-synchronous;
+                                       status = "disabled";
                                };

                                ssi1: ssi at 02028000 {

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd69385d..1084394197ad 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi

@@ -17,6 +17,14 @@
                reg = <0x10000000 0x80000000>;
        };

+       clocks {
+               anaclk2 { /* on-board 24.576MHz audio oscillator */
+                       compatible = "fixed-clock";
+                       #clock-cells = <0>;
+                       clock-frequency = <24576000>;
+               };
+       };
+
        leds {
                compatible = "gpio-leds";
                pinctrl-names = "default";

@@ -45,6 +53,17 @@
        };
 };

+&clks {
+       assigned-clocks = <&clks IMX6QDL_PLL4_BYPASS_SRC>,
+                         <&clks IMX6QDL_PLL4_BYPASS>,
+                         <&clks IMX6QDL_CLK_ESAI_SEL>,
+                         <&clks IMX6QDL_CLK_PLL4_POST_DIV>;
+       assigned-clock-parents = <&clks IMX6QDL_CLK_LVDS2_IN>,
+                                <&clks IMX6QDL_PLL4_BYPASS_SRC>,
+                                <&clks IMX6QDL_CLK_PLL4_AUDIO_DIV>;
+       assigned-clock-rates = <0>, <0>, <0>, <24576000>;
+};
+
 &ecspi1 {
        fsl,spi-num-chipselects = <1>;
        cs-gpios = <&gpio3 19 0>;

@@ -61,6 +80,12 @@
        };
 };

+&esai {
+       assigned-clocks = <&clks IMX6QDL_CLK_ESAI_EXTAL>;
+       assigned-clock-rates = <24576000>;
+       status = "okay";
+};
+
 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;

With above dts changes, I can get the following clock setup for ESAI.

 anaclk2                                  0            0    24576000          0
    lvds2_in                              0            0    24576000          0
       pll4_bypass_src                    0            0    24576000          0
          pll4_bypass                     0            0    24576000          0
             pll4_audio                   0            0    24576000          0
                pll4_post_div             0            0    24576000          0
                   pll4_audio_div           0            0    24576000          0
                      esai_sel            0            0    24576000          0
                         esai_pred           0            0    24576000          0
                            esai_podf           0            0    24576000          0
                               esai_extal           0            0    24576000          0

Shawn

On Fri, Aug 29, 2014 at 04:41:28PM +0800, Shawn Guo wrote:

On Fri, Aug 29, 2014 at 11:33:47AM +0800, Shengjiu Wang wrote:

quoted

Otherwise I still need to do some step in below to source the clock to ESAI.

        clk_set_parent(pll4_bypass_src, lvds2_in);
        clk_set_parent(pll4_bypass, pll4_bypass_src);
        clk_set_rate(pll4_audio_div, 24576000);
        clk_set_rate(esai_extal, 24576000);

I think it is what we can expect, right? if yes, the patch is ok for ESAI.

Yes, we still need to set up parent and rate specifically for
sabreauto ESAI use case.

But these setup can be done in device tree now with commit 86be408bfbd8
(clk: Support for clock parents and rates assigned from device tree).
Here is change that I tested with.  (Not sure if the info in esai node
is all correct)

Great. Thanks.

we alway don't set fsl,fifo-depth and fsl,esai-synchronous, just use the
default value in driver.

Can we move the clocks for &esai to &clks ? I just think that will look better.
Anyway, this is just my view. you can do it follow the formal principle.

 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;


With above dts changes, I can get the following clock setup for ESAI.

 anaclk2                                  0            0    24576000          0
    lvds2_in                              0            0    24576000          0
       pll4_bypass_src                    0            0    24576000          0
          pll4_bypass                     0            0    24576000          0
             pll4_audio                   0            0    24576000          0
                pll4_post_div             0            0    24576000          0
                   pll4_audio_div           0            0    24576000          0
                      esai_sel            0            0    24576000          0
                         esai_pred           0            0    24576000          0
                            esai_podf           0            0    24576000          0
                               esai_extal           0            0    24576000          0

Shawn

On Fri, Aug 29, 2014 at 05:32:40PM +0800, Shengjiu Wang wrote:

quoted

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 6692115b0138..9fe4169c79dd 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi

@@ -268,8 +268,18 @@
                                };

                                esai: esai at 02024000 {
+                                       compatible = "fsl,imx6q-esai", "fsl,imx35-esai";
                                        reg = <0x02024000 0x4000>;
                                        interrupts = <0 51 IRQ_TYPE_LEVEL_HIGH>;
+                                       clocks = <&clks IMX6QDL_CLK_ESAI_IPG>,
+                                                <&clks IMX6QDL_CLK_ESAI_EXTAL>,
+                                                <&clks IMX6QDL_CLK_ESAI_IPG>;
+                                       clock-names = "core", "extal", "fsys";
+                                       dmas = <&sdma 23 21 0>, <&sdma 24 21 0>;
+                                       dma-names = "rx", "tx";
+                                       fsl,fifo-depth = <128>;
+                                       fsl,esai-synchronous;

we alway don't set fsl,fifo-depth and fsl,esai-synchronous, just use the
default value in driver.

Okay.  The change here is just to get ESAI driver probed, so that the
assigned-clocks in ESAI node can be handled.  I suppose you will send me
a patch to get ESAI node ready for working.

quoted

+                                       status = "disabled";
                                };

                                ssi1: ssi at 02028000 {

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd69385d..1084394197ad 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi

@@ -17,6 +17,14 @@
                reg = <0x10000000 0x80000000>;
        };

+       clocks {
+               anaclk2 { /* on-board 24.576MHz audio oscillator */
+                       compatible = "fixed-clock";
+                       #clock-cells = <0>;
+                       clock-frequency = <24576000>;
+               };
+       };
+
        leds {
                compatible = "gpio-leds";
                pinctrl-names = "default";

@@ -45,6 +53,17 @@
        };
 };

+&clks {
+       assigned-clocks = <&clks IMX6QDL_PLL4_BYPASS_SRC>,
+                         <&clks IMX6QDL_PLL4_BYPASS>,
+                         <&clks IMX6QDL_CLK_ESAI_SEL>,
+                         <&clks IMX6QDL_CLK_PLL4_POST_DIV>;
+       assigned-clock-parents = <&clks IMX6QDL_CLK_LVDS2_IN>,
+                                <&clks IMX6QDL_PLL4_BYPASS_SRC>,
+                                <&clks IMX6QDL_CLK_PLL4_AUDIO_DIV>;
+       assigned-clock-rates = <0>, <0>, <0>, <24576000>;
+};
+
 &ecspi1 {
        fsl,spi-num-chipselects = <1>;
        cs-gpios = <&gpio3 19 0>;

@@ -61,6 +80,12 @@
        };
 };

+&esai {
+       assigned-clocks = <&clks IMX6QDL_CLK_ESAI_EXTAL>;
+       assigned-clock-rates = <24576000>;
+       status = "okay";
+};
+

Can we move the clocks for &esai to &clks ? I just think that will look better.
Anyway, this is just my view. you can do it follow the formal principle.

The general principle is that clocks used by multiple clients like bus
clock and pll are configured in &clk node, and leaf clocks only used by
particular client device should be configured in that device node.

Shawn