[PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Huang Shijie]

The clocks for Quadspi controller may be different when different
NOR flashes are connected to the board.

This patch adds a new helper to register the clock which needs the
re-parent capability.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 arch/arm/mach-imx/clk.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
index e29f6eb..2e3c4fe 100644
--- a/arch/arm/mach-imx/clk.h
+++ b/arch/arm/mach-imx/clk.h
@@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
 			&imx_ccm_lock);
 }
 
+/* we can use this helper to register the clock which needs the re-parent. */
+static inline struct clk *imx_clk_mux_flags_reparent(const char *name,
+		void __iomem *reg, u8 shift, u8 width, const char **parents,
+		int num_parents, unsigned long flags)
+{
+	return clk_register_mux(NULL, name, parents, num_parents,
+			flags, reg, shift, width, 0,
+			&imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_fixed_factor(const char *name,
 		const char *parent, unsigned int mult, unsigned int div)
 {
-- 
1.7.8

[PATCH 2/2] ARM: clk-imx6sx: register the qspi's clock with re-parent capability

[Huang Shijie]

The clocks for Quadspi controller may be different when different NOR
flashes are connected to the board.

So the IMX6SX_CLK_QSPI1_SEL/IMX6SX_CLK_QSPI2_SEL should have the re-parent
capability.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 arch/arm/mach-imx/clk-imx6sx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6sx.c b/arch/arm/mach-imx/clk-imx6sx.c
index 72f8902..2e6e24b 100644
--- a/arch/arm/mach-imx/clk-imx6sx.c
+++ b/arch/arm/mach-imx/clk-imx6sx.c
@@ -241,13 +241,13 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 	clks[IMX6SX_CLK_SSI3_SEL]           = imx_clk_mux("ssi3_sel",         base + 0x1c,  14,     2,      ssi_sels,          ARRAY_SIZE(ssi_sels));
 	clks[IMX6SX_CLK_SSI2_SEL]           = imx_clk_mux("ssi2_sel",         base + 0x1c,  12,     2,      ssi_sels,          ARRAY_SIZE(ssi_sels));
 	clks[IMX6SX_CLK_SSI1_SEL]           = imx_clk_mux("ssi1_sel",         base + 0x1c,  10,     2,      ssi_sels,          ARRAY_SIZE(ssi_sels));
-	clks[IMX6SX_CLK_QSPI1_SEL]          = imx_clk_mux_flags("qspi1_sel", base + 0x1c,  7, 3, qspi1_sels, ARRAY_SIZE(qspi1_sels), CLK_SET_RATE_PARENT);
+	clks[IMX6SX_CLK_QSPI1_SEL]          = imx_clk_mux_flags_reparent("qspi1_sel", base + 0x1c,  7, 3, qspi1_sels, ARRAY_SIZE(qspi1_sels), CLK_SET_RATE_PARENT);
 	clks[IMX6SX_CLK_PERCLK_SEL]         = imx_clk_mux("perclk_sel",       base + 0x1c,  6,      1,      perclk_sels,       ARRAY_SIZE(perclk_sels));
 	clks[IMX6SX_CLK_VID_SEL]            = imx_clk_mux("vid_sel",          base + 0x20,  21,     3,      vid_sels,          ARRAY_SIZE(vid_sels));
 	clks[IMX6SX_CLK_ESAI_SEL]           = imx_clk_mux("esai_sel",         base + 0x20,  19,     2,      audio_sels,        ARRAY_SIZE(audio_sels));
 	clks[IMX6SX_CLK_CAN_SEL]            = imx_clk_mux("can_sel",          base + 0x20,  8,      2,      can_sels,          ARRAY_SIZE(can_sels));
 	clks[IMX6SX_CLK_UART_SEL]           = imx_clk_mux("uart_sel",         base + 0x24,  6,      1,      uart_sels,         ARRAY_SIZE(uart_sels));
-	clks[IMX6SX_CLK_QSPI2_SEL]          = imx_clk_mux_flags("qspi2_sel", base + 0x2c, 15, 3, qspi2_sels, ARRAY_SIZE(qspi2_sels), CLK_SET_RATE_PARENT);
+	clks[IMX6SX_CLK_QSPI2_SEL]          = imx_clk_mux_flags_reparent("qspi2_sel", base + 0x2c, 15, 3, qspi2_sels, ARRAY_SIZE(qspi2_sels), CLK_SET_RATE_PARENT);
 	clks[IMX6SX_CLK_SPDIF_SEL]          = imx_clk_mux("spdif_sel",        base + 0x30,  20,     2,      audio_sels,        ARRAY_SIZE(audio_sels));
 	clks[IMX6SX_CLK_AUDIO_SEL]          = imx_clk_mux("audio_sel",        base + 0x30,  7,      2,      audio_sels,        ARRAY_SIZE(audio_sels));
 	clks[IMX6SX_CLK_ENET_PRE_SEL]       = imx_clk_mux("enet_pre_sel",     base + 0x34,  15,     3,      enet_pre_sels,     ARRAY_SIZE(enet_pre_sels));
-- 
1.7.8

Re: [PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Alexander Shiyan]

Mon, 26 May 2014 13:20:32 +0800 ?? Huang Shijie <b32955@freescale.com>:
> The clocks for Quadspi controller may be different when different
> NOR flashes are connected to the board.
> 
> This patch adds a new helper to register the clock which needs the
> re-parent capability.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  arch/arm/mach-imx/clk.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
> index e29f6eb..2e3c4fe 100644
> --- a/arch/arm/mach-imx/clk.h
> +++ b/arch/arm/mach-imx/clk.h
> @@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
>  			&imx_ccm_lock);
>  }
>  
> +/* we can use this helper to register the clock which needs the re-parent. */
> +static inline struct clk *imx_clk_mux_flags_reparent(const char *name,
> +		void __iomem *reg, u8 shift, u8 width, const char **parents,
> +		int num_parents, unsigned long flags)
> +{
> +	return clk_register_mux(NULL, name, parents, num_parents,
> +			flags, reg, shift, width, 0,
> +			&imx_ccm_lock);
> +}

So you can use CLK_SET_RATE_PARENT for flags here and completely remove
flags parameter for this call. Is not it?

---

[PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Huang Shijie]

On Mon, May 26, 2014 at 10:54:52AM +0400, Alexander Shiyan wrote:
> Mon, 26 May 2014 13:20:32 +0800 ?? Huang Shijie <b32955@freescale.com>:
> > The clocks for Quadspi controller may be different when different
> > NOR flashes are connected to the board.
> > 
> > This patch adds a new helper to register the clock which needs the
> > re-parent capability.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > ---
> >  arch/arm/mach-imx/clk.h |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
> > index e29f6eb..2e3c4fe 100644
> > --- a/arch/arm/mach-imx/clk.h
> > +++ b/arch/arm/mach-imx/clk.h
> > @@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
> >  			&imx_ccm_lock);
> >  }
> >  
> > +/* we can use this helper to register the clock which needs the re-parent. */
> > +static inline struct clk *imx_clk_mux_flags_reparent(const char *name,
> > +		void __iomem *reg, u8 shift, u8 width, const char **parents,
> > +		int num_parents, unsigned long flags)
> > +{
> > +	return clk_register_mux(NULL, name, parents, num_parents,
> > +			flags, reg, shift, width, 0,
> > +			&imx_ccm_lock);
> > +}
> 
> So you can use CLK_SET_RATE_PARENT for flags here and completely remove
> flags parameter for this call. Is not it?
Do you mean make the code like this :

----------------------------------------------------------------
	return clk_register_mux(NULL, name, parents, num_parents,
			CLK_SET_RATE_PARENT, reg, shift, width, 0,
			&imx_ccm_lock);
----------------------------------------------------------------

Without the "flags" argument, the helper is too specific.

But if Shawn also agree to use the fixed CLK_SET_RATE_PARENT, i am okay too.

thanks
Huang Shijie

Re: [PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Alexander Shiyan]

Mon, 26 May 2014 14:03:51 +0800 ?? Huang Shijie <b32955@freescale.com>:
> On Mon, May 26, 2014 at 10:54:52AM +0400, Alexander Shiyan wrote:
> > Mon, 26 May 2014 13:20:32 +0800 ?? Huang Shijie <b32955@freescale.com>:
> > > The clocks for Quadspi controller may be different when different
> > > NOR flashes are connected to the board.
> > > 
> > > This patch adds a new helper to register the clock which needs the
> > > re-parent capability.
> > > 
> > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > > ---
> > >  arch/arm/mach-imx/clk.h |   10 ++++++++++
> > >  1 files changed, 10 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
> > > index e29f6eb..2e3c4fe 100644
> > > --- a/arch/arm/mach-imx/clk.h
> > > +++ b/arch/arm/mach-imx/clk.h
> > > @@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
> > >  			&imx_ccm_lock);
> > >  }
> > >  
> > > +/* we can use this helper to register the clock which needs the re-parent. */
> > > +static inline struct clk *imx_clk_mux_flags_reparent(const char *name,
> > > +		void __iomem *reg, u8 shift, u8 width, const char **parents,
> > > +		int num_parents, unsigned long flags)
> > > +{
> > > +	return clk_register_mux(NULL, name, parents, num_parents,
> > > +			flags, reg, shift, width, 0,
> > > +			&imx_ccm_lock);
> > > +}
> > 
> > So you can use CLK_SET_RATE_PARENT for flags here and completely remove
> > flags parameter for this call. Is not it?
> Do you mean make the code like this :
> 
> ----------------------------------------------------------------
> 	return clk_register_mux(NULL, name, parents, num_parents,
> 			CLK_SET_RATE_PARENT, reg, shift, width, 0,
> 			&imx_ccm_lock);
> ----------------------------------------------------------------

Yes.

> Without the "flags" argument, the helper is too specific.

Or let's just OR with CLK_SET_RATE_PARENT, like as:

return clk_register_mux(NULL, name, parents, num_parents,
 			flags | CLK_SET_RATE_PARENT, reg, shift, width, 0,
			&imx_ccm_lock);

---

[PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Shawn Guo]

On Mon, May 26, 2014 at 01:20:32PM +0800, Huang Shijie wrote:
> The clocks for Quadspi controller may be different when different
> NOR flashes are connected to the board.
> 
> This patch adds a new helper to register the clock which needs the
> re-parent capability.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>

So what you need is to register a mux clock without
CLK_SET_RATE_NO_REPARENT flag, right?

I'm think about just dropping the flag from imx_clk_mux() and
imx_clk_mux_flags(), since this is the default behavior of basic mux
clock, and I doubt dropping the flag will cause problem for mux clocks
on i.MX.

Shawn

> ---
>  arch/arm/mach-imx/clk.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
> index e29f6eb..2e3c4fe 100644
> --- a/arch/arm/mach-imx/clk.h
> +++ b/arch/arm/mach-imx/clk.h
> @@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
>  			&imx_ccm_lock);
>  }
>  
> +/* we can use this helper to register the clock which needs the re-parent. */
> +static inline struct clk *imx_clk_mux_flags_reparent(const char *name,
> +		void __iomem *reg, u8 shift, u8 width, const char **parents,
> +		int num_parents, unsigned long flags)
> +{
> +	return clk_register_mux(NULL, name, parents, num_parents,
> +			flags, reg, shift, width, 0,
> +			&imx_ccm_lock);
> +}
> +
>  static inline struct clk *imx_clk_fixed_factor(const char *name,
>  		const char *parent, unsigned int mult, unsigned int div)
>  {
> -- 
> 1.7.8
> 

[PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Huang Shijie]

On Mon, May 26, 2014 at 04:01:21PM +0800, Shawn Guo wrote:
> On Mon, May 26, 2014 at 01:20:32PM +0800, Huang Shijie wrote:
> > The clocks for Quadspi controller may be different when different
> > NOR flashes are connected to the board.
> > 
> > This patch adds a new helper to register the clock which needs the
> > re-parent capability.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> 
> So what you need is to register a mux clock without
> CLK_SET_RATE_NO_REPARENT flag, right?
yes.

Without this fixed flag, I can pass the CLK_SET_RATE_PARENT in the "flags".


> 
> I'm think about just dropping the flag from imx_clk_mux() and
> imx_clk_mux_flags(), since this is the default behavior of basic mux
> clock, and I doubt dropping the flag will cause problem for mux clocks
> on i.MX.
this is what i was worry about.

So i do not change the imx_clk_mux/imx_clk_mux_flags, and add new helper which
has the minimum impact to the current code.

thanks
Huang Shijie

[PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Shawn Guo]

On Mon, May 26, 2014 at 02:47:46PM +0800, Huang Shijie wrote:
> > I'm think about just dropping the flag from imx_clk_mux() and
> > imx_clk_mux_flags(), since this is the default behavior of basic mux
> > clock, and I doubt dropping the flag will cause problem for mux clocks
> > on i.MX.
> this is what i was worry about.
> 
> So i do not change the imx_clk_mux/imx_clk_mux_flags, and add new helper which
> has the minimum impact to the current code.

Can you just try to drop the flag from imx_clk_mux/imx_clk_mux_flags
calls, and see if it causes any immediate problem for us?  If not, we
may want to create a patch for that, and throw it into linux-next for
wider testing.

Shawn

[PATCH 1/2] ARM: clk: imx: add a new helper which can re-parent the clock

[Huang Shijie]

On Mon, May 26, 2014 at 04:11:59PM +0800, Shawn Guo wrote:
> On Mon, May 26, 2014 at 02:47:46PM +0800, Huang Shijie wrote:
> > > I'm think about just dropping the flag from imx_clk_mux() and
> > > imx_clk_mux_flags(), since this is the default behavior of basic mux
> > > clock, and I doubt dropping the flag will cause problem for mux clocks
> > > on i.MX.
> > this is what i was worry about.
> > 
> > So i do not change the imx_clk_mux/imx_clk_mux_flags, and add new helper which
> > has the minimum impact to the current code.
> 
> Can you just try to drop the flag from imx_clk_mux/imx_clk_mux_flags
> calls, and see if it causes any immediate problem for us?  If not, we
> may want to create a patch for that, and throw it into linux-next for
> wider testing.
okay. I will send out the new patch set as soon as possible.

thanks
Huang Shijie