Skip to contentSkip to conversation

ARM: clk: imx: add a new helper which can re-parent the clock

PATCH In review Huang Shijie 3 participants 9 messages May 26, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

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

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.

---
 arch/arm/mach-imx/clk.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach-imx/clk.h +10 -0
index e29f6eb..2e3c4fe 100644
@@ -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)
 {
Signature · 1 line
1.7.8
#1 lore raw

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

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.

---
 arch/arm/mach-imx/clk-imx6sx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-imx/clk-imx6sx.c +2 -2
index 72f8902..2e6e24b 100644
@@ -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));
Signature · 1 line
1.7.8
#2 lore raw
In reply to Huang Shijie

Mon, 26 May 2014 13:20:32 +0800 ?? Huang Shijie <[email protected]>:

Quoted 18 lines

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.

---
 arch/arm/mach-imx/clk.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach-imx/clk.h +0 -0
index e29f6eb..2e3c4fe 100644
@@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
 			&imx_ccm_lock);
 }

arch/arm/mach-imx/clk.h static inline struct clk *imx_clk_mux_flags(const char *name, · line 123

Quoted 10 lines
arch/arm/mach-imx/clk.h +9 -0
 
+/* 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?

---

#3 lore raw

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

Quoted 20 lines · Alexander Shiyan

Mon, 26 May 2014 13:20:32 +0800 ?? Huang Shijie <[email protected]>:

Quoted 19 lines

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.

---
 arch/arm/mach-imx/clk.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach-imx/clk.h +0 -0
index e29f6eb..2e3c4fe 100644
@@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
 			&imx_ccm_lock);
 }
 
Quoted 12 lines

arch/arm/mach-imx/clk.h static inline struct clk *imx_clk_mux_flags(const char *name, · line 123

Quoted 9 lines
arch/arm/mach-imx/clk.h +9 -0
+/* 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

#4 lore raw

Mon, 26 May 2014 14:03:51 +0800 ?? Huang Shijie <[email protected]>:

Quoted 33 lines · Alexander Shiyan
Quoted 20 lines

Mon, 26 May 2014 13:20:32 +0800 ?? Huang Shijie <[email protected]>:

Quoted 19 lines

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.

---
 arch/arm/mach-imx/clk.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach-imx/clk.h +0 -0
index e29f6eb..2e3c4fe 100644
@@ -112,6 +112,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
 			&imx_ccm_lock);
 }
 
Quoted 12 lines

arch/arm/mach-imx/clk.h static inline struct clk *imx_clk_mux_flags(const char *name, · line 123

Quoted 9 lines
arch/arm/mach-imx/clk.h +9 -0
+/* 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?

Quoted 7 lines

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.

Quoted 1 line

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);

---

#5 lore raw

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

In reply to Huang Shijie
Quoted 7 lines · 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.

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

Quoted 28 lines
---
 arch/arm/mach-imx/clk.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach-imx/clk.h +10 -0
index e29f6eb..2e3c4fe 100644
@@ -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)
 {
Signature · 1 line
1.7.8
#6 lore raw

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

Quoted 11 lines · Shawn Guo
Quoted 7 lines · 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.

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".

Quoted 5 lines

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

#7 lore raw

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

Quoted 8 lines · Huang Shijie
Quoted 4 lines

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

#8 lore raw

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

Quoted 7 lines · Shawn Guo
Quoted 6 lines · Huang Shijie
Quoted 4 lines

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.

Quoted 7 lines
Quoted 2 lines

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