[PATCH V3 0/3] refine clock tree for esai in imx6q

[PATCH V3 0/3] refine clock tree for esai in imx6q

[Shengjiu Wang]

refine clock tree for esai in imx6q

Changes for V3
- Drop patch3, for waiting bypass mode implementation.
- As Lucas Stach suggestion, add new API for exclusive clock.

Changes for V2
- Add pll5_sel And update the comments for patch 2.
- As the bypass mode is not supported in clk tree, so update the 
- comments for patch 3.


Shengjiu Wang (3):
  ARM: clk-imx6q: refine clock tree for ESAI
  ARM: clk-gate2: Add API imx_clk_gate2_exclusive for clk_gate2
  ARM: clk-imx6q: Add missing lvds and anaclk clock to the clock tree

 arch/arm/mach-imx/clk-gate2.c             |   18 +++++++++++++++++-
 arch/arm/mach-imx/clk-imx6q.c             |   25 ++++++++++++++++++-------
 arch/arm/mach-imx/clk.h                   |    2 ++
 include/dt-bindings/clock/imx6qdl-clock.h |   13 ++++++++++---
 4 files changed, 47 insertions(+), 11 deletions(-)

-- 
1.7.9.5

[PATCH V3 1/3] ARM: clk-imx6q: refine clock tree for ESAI

[Shengjiu Wang]

There are three clock for ESAI, esai_extal, esai_ipg, esai_mem. Rename
'esai' to 'esai_extal', 'esai_ahb' to 'esai_mem', and add 'esai_ipg'.
Make the clock for ESAI more clear and align them with imx6sx.

Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
 arch/arm/mach-imx/clk-imx6q.c             |    7 ++++---
 include/dt-bindings/clock/imx6qdl-clock.h |    7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 6cceb77..1d6dd59 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -64,7 +64,7 @@ static const char *cko2_sels[] = {
 	"ipu2", "vdo_axi", "osc", "gpu2d_core",
 	"gpu3d_core", "usdhc2", "ssi1", "ssi2",
 	"ssi3", "gpu3d_shader", "vpu_axi", "can_root",
-	"ldb_di0", "ldb_di1", "esai", "eim_slow",
+	"ldb_di0", "ldb_di1", "esai_extal", "eim_slow",
 	"uart_serial", "spdif", "asrc", "hsi_tx",
 };
 static const char *cko_sels[] = { "cko1", "cko2", };
@@ -325,8 +325,9 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	else
 		clk[IMX6Q_CLK_ECSPI5] = imx_clk_gate2("ecspi5",        "ecspi_root",        base + 0x6c, 8);
 	clk[IMX6QDL_CLK_ENET]         = imx_clk_gate2("enet",          "ipg",               base + 0x6c, 10);
-	clk[IMX6QDL_CLK_ESAI]         = imx_clk_gate2_shared("esai",   "esai_podf",         base + 0x6c, 16, &share_count_esai);
-	clk[IMX6QDL_CLK_ESAI_AHB]     = imx_clk_gate2_shared("esai_ahb", "ahb",             base + 0x6c, 16, &share_count_esai);
+	clk[IMX6QDL_CLK_ESAI_EXTAL]   = imx_clk_gate2_shared("esai_extal",   "esai_podf",   base + 0x6c, 16, &share_count_esai);
+	clk[IMX6QDL_CLK_ESAI_IPG]     = imx_clk_gate2_shared("esai_ipg",   "ipg",           base + 0x6c, 16, &share_count_esai);
+	clk[IMX6QDL_CLK_ESAI_MEM]     = imx_clk_gate2_shared("esai_mem", "ahb",             base + 0x6c, 16, &share_count_esai);
 	clk[IMX6QDL_CLK_GPT_IPG]      = imx_clk_gate2("gpt_ipg",       "ipg",               base + 0x6c, 20);
 	clk[IMX6QDL_CLK_GPT_IPG_PER]  = imx_clk_gate2("gpt_ipg_per",   "ipg_per",           base + 0x6c, 22);
 	if (cpu_is_imx6dl())
diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
index 654151e..323e865 100644
--- a/include/dt-bindings/clock/imx6qdl-clock.h
+++ b/include/dt-bindings/clock/imx6qdl-clock.h
@@ -128,7 +128,7 @@
 #define IMX6Q_CLK_ECSPI5			116
 #define IMX6DL_CLK_I2C4				116
 #define IMX6QDL_CLK_ENET			117
-#define IMX6QDL_CLK_ESAI			118
+#define IMX6QDL_CLK_ESAI_EXTAL			118
 #define IMX6QDL_CLK_GPT_IPG			119
 #define IMX6QDL_CLK_GPT_IPG_PER			120
 #define IMX6QDL_CLK_GPU2D_CORE			121
@@ -218,7 +218,8 @@
 #define IMX6QDL_CLK_LVDS2_SEL			205
 #define IMX6QDL_CLK_LVDS1_GATE			206
 #define IMX6QDL_CLK_LVDS2_GATE			207
-#define IMX6QDL_CLK_ESAI_AHB			208
-#define IMX6QDL_CLK_END				209
+#define IMX6QDL_CLK_ESAI_IPG			208
+#define IMX6QDL_CLK_ESAI_MEM			209
+#define IMX6QDL_CLK_END				210
 
 #endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */
-- 
1.7.9.5

[PATCH V3 2/3] ARM: clk-gate2: Add API imx_clk_gate2_exclusive for clk_gate2

[Shengjiu Wang]

As some clocks are mutually exlcusive, they can't be enabled simultaneously,
So add this new API for registering exclusive clock, the enable function will
check if there is exclusive clock and it is not enabled, then this clock can
be enabled, otherwise, it will return error.

Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
 arch/arm/mach-imx/clk-gate2.c |   18 +++++++++++++++++-
 arch/arm/mach-imx/clk.h       |    2 ++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/clk-gate2.c b/arch/arm/mach-imx/clk-gate2.c
index 84acdfd..df64c4d 100644
--- a/arch/arm/mach-imx/clk-gate2.c
+++ b/arch/arm/mach-imx/clk-gate2.c
@@ -8,7 +8,7 @@
  *
  * Gated clock implementation
  */
-
+#include <linux/clk-private.h>
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -34,6 +34,7 @@ struct clk_gate2 {
 	u8		flags;
 	spinlock_t	*lock;
 	unsigned int	*share_count;
+	struct clk	*clk_exclusive;
 };
 
 #define to_clk_gate2(_hw) container_of(_hw, struct clk_gate2, hw)
@@ -46,6 +47,11 @@ static int clk_gate2_enable(struct clk_hw *hw)
 
 	spin_lock_irqsave(gate->lock, flags);
 
+	if (gate->clk_exclusive && __clk_is_enabled(gate->clk_exclusive)) {
+		spin_unlock_irqrestore(gate->lock, flags);
+		return -EBUSY;
+	}
+
 	if (gate->share_count && (*gate->share_count)++ > 0)
 		goto out;
 
@@ -147,3 +153,13 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
 
 	return clk;
 }
+
+int imx_clk_gate2_exclusive(struct clk *clk1, struct clk *clk2)
+{
+	struct clk_gate2 *gate1 = to_clk_gate2(clk1->hw);
+	struct clk_gate2 *gate2 = to_clk_gate2(clk2->hw);
+
+	gate1->clk_exclusive = clk2;
+	gate2->clk_exclusive = clk1;
+	return 0;
+}
diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
index d5ba76f..2b07376 100644
--- a/arch/arm/mach-imx/clk.h
+++ b/arch/arm/mach-imx/clk.h
@@ -36,6 +36,8 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
 struct clk * imx_obtain_fixed_clock(
 			const char *name, unsigned long rate);
 
+int imx_clk_gate2_exclusive(struct clk *clk1, struct clk *clk2);
+
 static inline struct clk *imx_clk_gate2(const char *name, const char *parent,
 		void __iomem *reg, u8 shift)
 {
-- 
1.7.9.5

[PATCH V3 2/3] ARM: clk-gate2: Add API imx_clk_gate2_exclusive for clk_gate2

[Shawn Guo]

On Fri, Aug 08, 2014 at 03:02:48PM +0800, Shengjiu Wang wrote:
> As some clocks are mutually exlcusive, they can't be enabled simultaneously,
> So add this new API for registering exclusive clock, the enable function will
> check if there is exclusive clock and it is not enabled, then this clock can
> be enabled, otherwise, it will return error.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> ---
>  arch/arm/mach-imx/clk-gate2.c |   18 +++++++++++++++++-
>  arch/arm/mach-imx/clk.h       |    2 ++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/clk-gate2.c b/arch/arm/mach-imx/clk-gate2.c
> index 84acdfd..df64c4d 100644
> --- a/arch/arm/mach-imx/clk-gate2.c
> +++ b/arch/arm/mach-imx/clk-gate2.c
> @@ -8,7 +8,7 @@
>   *
>   * Gated clock implementation
>   */
> -
> +#include <linux/clk-private.h>

I do not see why this header is needed.

Shawn

>  #include <linux/clk-provider.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> @@ -34,6 +34,7 @@ struct clk_gate2 {
>  	u8		flags;
>  	spinlock_t	*lock;
>  	unsigned int	*share_count;
> +	struct clk	*clk_exclusive;
>  };
>  
>  #define to_clk_gate2(_hw) container_of(_hw, struct clk_gate2, hw)
> @@ -46,6 +47,11 @@ static int clk_gate2_enable(struct clk_hw *hw)
>  
>  	spin_lock_irqsave(gate->lock, flags);
>  
> +	if (gate->clk_exclusive && __clk_is_enabled(gate->clk_exclusive)) {
> +		spin_unlock_irqrestore(gate->lock, flags);
> +		return -EBUSY;
> +	}
> +
>  	if (gate->share_count && (*gate->share_count)++ > 0)
>  		goto out;
>  
> @@ -147,3 +153,13 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
>  
>  	return clk;
>  }
> +
> +int imx_clk_gate2_exclusive(struct clk *clk1, struct clk *clk2)
> +{
> +	struct clk_gate2 *gate1 = to_clk_gate2(clk1->hw);
> +	struct clk_gate2 *gate2 = to_clk_gate2(clk2->hw);
> +
> +	gate1->clk_exclusive = clk2;
> +	gate2->clk_exclusive = clk1;
> +	return 0;
> +}
> diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
> index d5ba76f..2b07376 100644
> --- a/arch/arm/mach-imx/clk.h
> +++ b/arch/arm/mach-imx/clk.h
> @@ -36,6 +36,8 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
>  struct clk * imx_obtain_fixed_clock(
>  			const char *name, unsigned long rate);
>  
> +int imx_clk_gate2_exclusive(struct clk *clk1, struct clk *clk2);
> +
>  static inline struct clk *imx_clk_gate2(const char *name, const char *parent,
>  		void __iomem *reg, u8 shift)
>  {
> -- 
> 1.7.9.5
> 

[PATCH V3 2/3] ARM: clk-gate2: Add API imx_clk_gate2_exclusive for clk_gate2

[Shengjiu Wang]

On Sat, Aug 09, 2014 at 09:33:17PM +0800, Shawn Guo wrote:
> On Fri, Aug 08, 2014 at 03:02:48PM +0800, Shengjiu Wang wrote:
> > As some clocks are mutually exlcusive, they can't be enabled simultaneously,
> > So add this new API for registering exclusive clock, the enable function will
> > check if there is exclusive clock and it is not enabled, then this clock can
> > be enabled, otherwise, it will return error.
> > 
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> > ---
> >  arch/arm/mach-imx/clk-gate2.c |   18 +++++++++++++++++-
> >  arch/arm/mach-imx/clk.h       |    2 ++
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-imx/clk-gate2.c b/arch/arm/mach-imx/clk-gate2.c
> > index 84acdfd..df64c4d 100644
> > --- a/arch/arm/mach-imx/clk-gate2.c
> > +++ b/arch/arm/mach-imx/clk-gate2.c
> > @@ -8,7 +8,7 @@
> >   *
> >   * Gated clock implementation
> >   */
> > -
> > +#include <linux/clk-private.h>
> 
> I do not see why this header is needed.
> 
> Shawn
> 
I add new function imx_clk_gate2_exclusive(), which need to access "struct clk_hw"
in "struct clk", So here need include clk-private.h

> >  #include <linux/clk-provider.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> > @@ -34,6 +34,7 @@ struct clk_gate2 {
> >  	u8		flags;
> >  	spinlock_t	*lock;
> >  	unsigned int	*share_count;
> > +	struct clk	*clk_exclusive;
> >  };
> >  
> >  #define to_clk_gate2(_hw) container_of(_hw, struct clk_gate2, hw)
> > @@ -46,6 +47,11 @@ static int clk_gate2_enable(struct clk_hw *hw)
> >  
> >  	spin_lock_irqsave(gate->lock, flags);
> >  
> > +	if (gate->clk_exclusive && __clk_is_enabled(gate->clk_exclusive)) {
> > +		spin_unlock_irqrestore(gate->lock, flags);
> > +		return -EBUSY;
> > +	}
> > +
> >  	if (gate->share_count && (*gate->share_count)++ > 0)
> >  		goto out;
> >  
> > @@ -147,3 +153,13 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
> >  
> >  	return clk;
> >  }
> > +
> > +int imx_clk_gate2_exclusive(struct clk *clk1, struct clk *clk2)
> > +{
> > +	struct clk_gate2 *gate1 = to_clk_gate2(clk1->hw);
> > +	struct clk_gate2 *gate2 = to_clk_gate2(clk2->hw);
> > +
> > +	gate1->clk_exclusive = clk2;
> > +	gate2->clk_exclusive = clk1;
> > +	return 0;
> > +}
> > diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
> > index d5ba76f..2b07376 100644
> > --- a/arch/arm/mach-imx/clk.h
> > +++ b/arch/arm/mach-imx/clk.h
> > @@ -36,6 +36,8 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
> >  struct clk * imx_obtain_fixed_clock(
> >  			const char *name, unsigned long rate);
> >  
> > +int imx_clk_gate2_exclusive(struct clk *clk1, struct clk *clk2);
> > +
> >  static inline struct clk *imx_clk_gate2(const char *name, const char *parent,
> >  		void __iomem *reg, u8 shift)
> >  {
> > -- 
> > 1.7.9.5
> > 

[PATCH V3 3/3] ARM: clk-imx6q: Add missing lvds and anaclk clock to the clock tree

[Shengjiu Wang]

anaclk1 and anaclk2 is the clock source for lvds1_in and lvds2_in. lvds1_in
and lvds2_in can be used to provide external clock source to the internal
pll, such as pll4_audio and pll5_video.
pll4_audio and pll5_video can have multiple source, not only "osc", so
add them.

Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
 arch/arm/mach-imx/clk-imx6q.c             |   18 ++++++++++++++----
 include/dt-bindings/clock/imx6qdl-clock.h |    8 +++++++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 1d6dd59..c1f285c 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -73,6 +73,7 @@ static const char *lvds_sels[] = {
 	"pll4_audio", "pll5_video", "pll8_mlb", "enet_ref",
 	"pcie_ref_125m", "sata_ref_100m",
 };
+static const char *pll_av_sels[] = { "osc", "lvds1_in", "lvds2_in", "dummy", };
 
 static struct clk *clk[IMX6QDL_CLK_END];
 static struct clk_onecell_data clk_data;
@@ -119,6 +120,9 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[IMX6QDL_CLK_CKIL] = imx_obtain_fixed_clock("ckil", 0);
 	clk[IMX6QDL_CLK_CKIH] = imx_obtain_fixed_clock("ckih1", 0);
 	clk[IMX6QDL_CLK_OSC] = imx_obtain_fixed_clock("osc", 0);
+	/* Clock source from external clock via ANACLK1/2 PADs */
+	clk[IMX6QDL_CLK_ANACLK1] = imx_obtain_fixed_clock("anaclk1", 0);
+	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);
@@ -136,8 +140,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	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_PLL4_AUDIO]    = imx_clk_pllv3(IMX_PLLV3_AV,	"pll4_audio",	"pll4_sel", base + 0x70, 0x7f);
+	clk[IMX6QDL_CLK_PLL5_VIDEO]    = imx_clk_pllv3(IMX_PLLV3_AV,	"pll5_video",	"pll5_sel", 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);
 
@@ -169,6 +173,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 
 	clk[IMX6QDL_CLK_LVDS1_SEL] = imx_clk_mux("lvds1_sel", base + 0x160, 0, 5, lvds_sels, ARRAY_SIZE(lvds_sels));
 	clk[IMX6QDL_CLK_LVDS2_SEL] = imx_clk_mux("lvds2_sel", base + 0x160, 5, 5, lvds_sels, ARRAY_SIZE(lvds_sels));
+	clk[IMX6QDL_CLK_PLL4_SEL]  = imx_clk_mux("pll4_sel",  base + 0x70, 14, 2, pll_av_sels, ARRAY_SIZE(pll_av_sels));
+	clk[IMX6QDL_CLK_PLL5_SEL]  = imx_clk_mux("pll5_sel",  base + 0xa0, 14, 2, pll_av_sels, ARRAY_SIZE(pll_av_sels));
 
 	/*
 	 * lvds1_gate and lvds2_gate are pseudo-gates.  Both can be
@@ -176,8 +182,12 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	 * the "output_enable" bit as a gate, even though it's really just
 	 * enabling clock output.
 	 */
-	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160, 10);
-	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate("lvds2_gate", "lvds2_sel", base + 0x160, 11);
+	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate2("lvds1_gate", "lvds1_sel", base + 0x160, 10);
+	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate2("lvds2_gate", "lvds2_sel", base + 0x160, 11);
+	clk[IMX6QDL_CLK_LVDS1_IN] = imx_clk_gate2("lvds1_in", "anaclk1", base + 0x160, 12);
+	clk[IMX6QDL_CLK_LVDS2_IN] = imx_clk_gate2("lvds2_in", "anaclk2", base + 0x160, 13);
+	imx_clk_gate2_exclusive(clk[IMX6QDL_CLK_LVDS1_GATE], clk[IMX6QDL_CLK_LVDS1_IN]);
+	imx_clk_gate2_exclusive(clk[IMX6QDL_CLK_LVDS2_GATE], clk[IMX6QDL_CLK_LVDS2_IN]);
 
 	/*                                            name              parent_name        reg       idx */
 	clk[IMX6QDL_CLK_PLL2_PFD0_352M] = imx_clk_pfd("pll2_pfd0_352m", "pll2_bus",     base + 0x100, 0);
diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
index 323e865..5519526 100644
--- a/include/dt-bindings/clock/imx6qdl-clock.h
+++ b/include/dt-bindings/clock/imx6qdl-clock.h
@@ -220,6 +220,12 @@
 #define IMX6QDL_CLK_LVDS2_GATE			207
 #define IMX6QDL_CLK_ESAI_IPG			208
 #define IMX6QDL_CLK_ESAI_MEM			209
-#define IMX6QDL_CLK_END				210
+#define IMX6QDL_CLK_LVDS1_IN			210
+#define IMX6QDL_CLK_LVDS2_IN			211
+#define IMX6QDL_CLK_ANACLK1			212
+#define IMX6QDL_CLK_ANACLK2			213
+#define IMX6QDL_CLK_PLL4_SEL			214
+#define IMX6QDL_CLK_PLL5_SEL			215
+#define IMX6QDL_CLK_END				216
 
 #endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */
-- 
1.7.9.5

[PATCH V3 3/3] ARM: clk-imx6q: Add missing lvds and anaclk clock to the clock tree

[Shawn Guo]

On Fri, Aug 08, 2014 at 03:02:49PM +0800, Shengjiu Wang wrote:
> @@ -176,8 +182,12 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>  	 * the "output_enable" bit as a gate, even though it's really just
>  	 * enabling clock output.
>  	 */
> -	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> -	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> +	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate2("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> +	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate2("lvds2_gate", "lvds2_sel", base + 0x160, 11);

I do not think you can simply change to use imx_clk_gate2() here.  It's
designed for those CCGR gate clocks, each of which is controlled by two
bits.

Shawn

> +	clk[IMX6QDL_CLK_LVDS1_IN] = imx_clk_gate2("lvds1_in", "anaclk1", base + 0x160, 12);
> +	clk[IMX6QDL_CLK_LVDS2_IN] = imx_clk_gate2("lvds2_in", "anaclk2", base + 0x160, 13);
> +	imx_clk_gate2_exclusive(clk[IMX6QDL_CLK_LVDS1_GATE], clk[IMX6QDL_CLK_LVDS1_IN]);
> +	imx_clk_gate2_exclusive(clk[IMX6QDL_CLK_LVDS2_GATE], clk[IMX6QDL_CLK_LVDS2_IN]);
>  
>  	/*                                            name              parent_name        reg       idx */
>  	clk[IMX6QDL_CLK_PLL2_PFD0_352M] = imx_clk_pfd("pll2_pfd0_352m", "pll2_bus",     base + 0x100, 0);

[PATCH V3 3/3] ARM: clk-imx6q: Add missing lvds and anaclk clock to the clock tree

[Shengjiu Wang]

On Sat, Aug 09, 2014 at 09:58:42PM +0800, Shawn Guo wrote:
> On Fri, Aug 08, 2014 at 03:02:49PM +0800, Shengjiu Wang wrote:
> > @@ -176,8 +182,12 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> >  	 * the "output_enable" bit as a gate, even though it's really just
> >  	 * enabling clock output.
> >  	 */
> > -	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > -	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> > +	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate2("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > +	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate2("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> 
> I do not think you can simply change to use imx_clk_gate2() here.  It's
> designed for those CCGR gate clocks, each of which is controlled by two
> bits.
> 
> Shawn
>
As Lucas Stach's suggestion, we need to do add some method for mutually exclusive clock, 
lvds1_gate with lvds1_in, lvds2_gate with lvds2_in. I add imx_clk_gate2_exclusive() function in clk-gate2.c.
So I change imx_clk_gate() to imx_clk_gate2() here.
As you said, this is not good solution. So I need your suggestion, how can I do?
First, is it allowable that to add imx_clk_gate2_exclusive() function, is there a more better way?
second, or should I change the clk-gate.c to add exclusive control?

Thanks
wang shengjiu

> > +	clk[IMX6QDL_CLK_LVDS1_IN] = imx_clk_gate2("lvds1_in", "anaclk1", base + 0x160, 12);
> > +	clk[IMX6QDL_CLK_LVDS2_IN] = imx_clk_gate2("lvds2_in", "anaclk2", base + 0x160, 13);
> > +	imx_clk_gate2_exclusive(clk[IMX6QDL_CLK_LVDS1_GATE], clk[IMX6QDL_CLK_LVDS1_IN]);
> > +	imx_clk_gate2_exclusive(clk[IMX6QDL_CLK_LVDS2_GATE], clk[IMX6QDL_CLK_LVDS2_IN]);
> >  
> >  	/*                                            name              parent_name        reg       idx */
> >  	clk[IMX6QDL_CLK_PLL2_PFD0_352M] = imx_clk_pfd("pll2_pfd0_352m", "pll2_bus",     base + 0x100, 0);

[PATCH V3 3/3] ARM: clk-imx6q: Add missing lvds and anaclk clock to the clock tree

[Shawn Guo]

On Mon, Aug 11, 2014 at 11:09:36AM +0800, Shengjiu Wang wrote:
> On Sat, Aug 09, 2014 at 09:58:42PM +0800, Shawn Guo wrote:
> > On Fri, Aug 08, 2014 at 03:02:49PM +0800, Shengjiu Wang wrote:
> > > @@ -176,8 +182,12 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> > >  	 * the "output_enable" bit as a gate, even though it's really just
> > >  	 * enabling clock output.
> > >  	 */
> > > -	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > > -	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> > > +	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate2("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > > +	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate2("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> > 
> > I do not think you can simply change to use imx_clk_gate2() here.  It's
> > designed for those CCGR gate clocks, each of which is controlled by two
> > bits.
> > 
> > Shawn
> >
> As Lucas Stach's suggestion, we need to do add some method for mutually exclusive clock, 
> lvds1_gate with lvds1_in, lvds2_gate with lvds2_in. I add imx_clk_gate2_exclusive() function in clk-gate2.c.
> So I change imx_clk_gate() to imx_clk_gate2() here.
> As you said, this is not good solution.

It's not just a "not good" solution but wrong and broken one.  The net
result of that is if you call clk_enable() on lvds1_gate, both bit 10
and 11 will be set.

> So I need your suggestion, how can I do?

I guess we will need a new clock type to handle such mutually exclusive
clocks, rather than patching clk-gate2.

> First, is it allowable that to add imx_clk_gate2_exclusive() function, is there a more better way?

Again, this is completely wrong.

> second, or should I change the clk-gate.c to add exclusive control?

If such mutually exclusive clocks are somehow common across different
clock controllers, we can propose to change clk-gate.c for handling
them.  But I'm not sure this is a common case.

Shawn

[PATCH V3 3/3] ARM: clk-imx6q: Add missing lvds and anaclk clock to the clock tree

[Shengjiu Wang]

On Mon, Aug 18, 2014 at 02:06:07PM +0800, Shawn Guo wrote:
> On Mon, Aug 11, 2014 at 11:09:36AM +0800, Shengjiu Wang wrote:
> > On Sat, Aug 09, 2014 at 09:58:42PM +0800, Shawn Guo wrote:
> > > On Fri, Aug 08, 2014 at 03:02:49PM +0800, Shengjiu Wang wrote:
> > > > @@ -176,8 +182,12 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> > > >  	 * the "output_enable" bit as a gate, even though it's really just
> > > >  	 * enabling clock output.
> > > >  	 */
> > > > -	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > > > -	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> > > > +	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate2("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > > > +	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate2("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> > > 
> > > I do not think you can simply change to use imx_clk_gate2() here.  It's
> > > designed for those CCGR gate clocks, each of which is controlled by two
> > > bits.
> > > 
> > > Shawn
> > >
> > As Lucas Stach's suggestion, we need to do add some method for mutually exclusive clock, 
> > lvds1_gate with lvds1_in, lvds2_gate with lvds2_in. I add imx_clk_gate2_exclusive() function in clk-gate2.c.
> > So I change imx_clk_gate() to imx_clk_gate2() here.
> > As you said, this is not good solution.
> 
> It's not just a "not good" solution but wrong and broken one.  The net
> result of that is if you call clk_enable() on lvds1_gate, both bit 10
> and 11 will be set.
> 
> > So I need your suggestion, how can I do?
> 
> I guess we will need a new clock type to handle such mutually exclusive
> clocks, rather than patching clk-gate2.
> 
Could you please help to implement this feature?

Furthermore, I'd like to drop patch 2 and patch 3, wait the implementation from
you.

Could you please review the patch 1?  do you have any comments?

Wang Shengjiu

> > First, is it allowable that to add imx_clk_gate2_exclusive() function, is there a more better way?
> 
> Again, this is completely wrong.
> 
> > second, or should I change the clk-gate.c to add exclusive control?
> 
> If such mutually exclusive clocks are somehow common across different
> clock controllers, we can propose to change clk-gate.c for handling
> them.  But I'm not sure this is a common case.
> 
> Shawn

[PATCH V3 3/3] ARM: clk-imx6q: Add missing lvds and anaclk clock to the clock tree

[Shawn Guo]

On Mon, Aug 25, 2014 at 03:40:20PM +0800, Shengjiu Wang wrote:
> On Mon, Aug 18, 2014 at 02:06:07PM +0800, Shawn Guo wrote:
> > On Mon, Aug 11, 2014 at 11:09:36AM +0800, Shengjiu Wang wrote:
> > > On Sat, Aug 09, 2014 at 09:58:42PM +0800, Shawn Guo wrote:
> > > > On Fri, Aug 08, 2014 at 03:02:49PM +0800, Shengjiu Wang wrote:
> > > > > @@ -176,8 +182,12 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> > > > >  	 * the "output_enable" bit as a gate, even though it's really just
> > > > >  	 * enabling clock output.
> > > > >  	 */
> > > > > -	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > > > > -	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> > > > > +	clk[IMX6QDL_CLK_LVDS1_GATE] = imx_clk_gate2("lvds1_gate", "lvds1_sel", base + 0x160, 10);
> > > > > +	clk[IMX6QDL_CLK_LVDS2_GATE] = imx_clk_gate2("lvds2_gate", "lvds2_sel", base + 0x160, 11);
> > > > 
> > > > I do not think you can simply change to use imx_clk_gate2() here.  It's
> > > > designed for those CCGR gate clocks, each of which is controlled by two
> > > > bits.
> > > > 
> > > > Shawn
> > > >
> > > As Lucas Stach's suggestion, we need to do add some method for mutually exclusive clock, 
> > > lvds1_gate with lvds1_in, lvds2_gate with lvds2_in. I add imx_clk_gate2_exclusive() function in clk-gate2.c.
> > > So I change imx_clk_gate() to imx_clk_gate2() here.
> > > As you said, this is not good solution.
> > 
> > It's not just a "not good" solution but wrong and broken one.  The net
> > result of that is if you call clk_enable() on lvds1_gate, both bit 10
> > and 11 will be set.
> > 
> > > So I need your suggestion, how can I do?
> > 
> > I guess we will need a new clock type to handle such mutually exclusive
> > clocks, rather than patching clk-gate2.
> > 
> Could you please help to implement this feature?

Okay, I will give it a try soon.

> 
> Furthermore, I'd like to drop patch 2 and patch 3, wait the implementation from
> you.
> 
> Could you please review the patch 1?  do you have any comments?

Just applied #1.

Shawn