| Message ID | 1401081633-12190-1-git-send-email-b32955@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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? ---
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
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); ---
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 >
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
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) {
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(-)