| Message ID | 1335367703-19929-20-git-send-email-s.hauer@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 25, 2012 at 05:28:09PM +0200, Sascha Hauer wrote: > - Add necessary #ifdefs for CONFIG_COMMON_CLOCK > - Add a global spinlock to protect the CCM registers > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > arch/arm/mach-imx/clk.h | 44 ++++++++++++++++++++++++++++++++ > arch/arm/plat-mxc/clock.c | 11 ++++++++ > arch/arm/plat-mxc/include/mach/clock.h | 4 +++ > 3 files changed, 59 insertions(+) > create mode 100644 arch/arm/mach-imx/clk.h > > diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h > new file mode 100644 > index 0000000..00f2590 > --- /dev/null > +++ b/arch/arm/mach-imx/clk.h > @@ -0,0 +1,44 @@ > +#ifndef __MACH_IMX_CLK_H > +#define __MACH_IMX_CLK_H > + > +#include <linux/spinlock.h> > +#include <linux/clk-provider.h> > +#include <mach/clock.h> > + > +struct clk *imx_clk_pllv1(const char *name, char *parent, > + void __iomem *base); > + > +static inline struct clk *imx_clk_fixed(const char *name, int rate) > +{ > + return clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate); > +} > + > +static inline struct clk *imx_clk_divider(const char *name, const char *parent, > + void __iomem *reg, u8 shift, u8 width) > +{ > + return clk_register_divider(NULL, name, parent, CLK_SET_RATE_PARENT, > + reg, shift, width, 0, &imx_ccm_lock); > +} > + > +static inline struct clk *imx_clk_gate(const char *name, const char *parent, > + void __iomem *reg, u8 shift) > +{ > + return clk_register_gate(NULL, name, parent, CLK_SET_RATE_PARENT, reg, > + shift, 0, &imx_ccm_lock); > +} > + > +static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg, > + u8 shift, u8 width, const char **parents, int num_parents) > +{ > + return clk_register_mux(NULL, name, parents, num_parents, 0, reg, shift, I think the fourth parameter should be CLK_SET_RATE_PARENT too, as mux could also likely be in a clk_set_rate propagation path, saying it has a parent clk who has .round_rate and .set_rate operations. > + width, 0, &imx_ccm_lock); > +} > +
On Fri, Apr 27, 2012 at 02:40:09PM +0800, Shawn Guo wrote: > On Wed, Apr 25, 2012 at 05:28:09PM +0200, Sascha Hauer wrote: > > - Add necessary #ifdefs for CONFIG_COMMON_CLOCK > > - Add a global spinlock to protect the CCM registers > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > + > > +static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg, > > + u8 shift, u8 width, const char **parents, int num_parents) > > +{ > > + return clk_register_mux(NULL, name, parents, num_parents, 0, reg, shift, > > I think the fourth parameter should be CLK_SET_RATE_PARENT too, as mux > could also likely be in a clk_set_rate propagation path, saying it has > a parent clk who has .round_rate and .set_rate operations. Nope, we can't do this. When we do this every clock_set_rate on a leaf node will propagate up to the ipg, ahb, main_clk, plls and whatever is up there. I made the assumption that we can safely propagate up to the next mux but not further. It may turn out that this is to simple and needs adjustments, but generally adding a CLK_SET_RATE_PARENT to a mux won't work. Sascha
On Fri, Apr 27, 2012 at 09:16:22AM +0200, Sascha Hauer wrote: > On Fri, Apr 27, 2012 at 02:40:09PM +0800, Shawn Guo wrote: > > On Wed, Apr 25, 2012 at 05:28:09PM +0200, Sascha Hauer wrote: > > > - Add necessary #ifdefs for CONFIG_COMMON_CLOCK > > > - Add a global spinlock to protect the CCM registers > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > --- > > > + > > > +static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg, > > > + u8 shift, u8 width, const char **parents, int num_parents) > > > +{ > > > + return clk_register_mux(NULL, name, parents, num_parents, 0, reg, shift, > > > > I think the fourth parameter should be CLK_SET_RATE_PARENT too, as mux > > could also likely be in a clk_set_rate propagation path, saying it has > > a parent clk who has .round_rate and .set_rate operations. > > Nope, we can't do this. When we do this every clock_set_rate on a leaf > node will propagate up to the ipg, ahb, main_clk, plls and whatever is > up there. PLL does not have CLK_SET_RATE_PARENT set, right? What's the problem with that propagation? Isn't it designed so? Now on imx6q, 792MHz is a set point for cpu frequency, we have to propagate up to pll's set_rate to get that frequency. There is a mux in the middle of the propagation path. If we do not set CLK_SET_RATE_PARENT for mux, how can we get this frequency? Regards, Shawn > > I made the assumption that we can safely propagate up to the next mux > but not further. It may turn out that this is to simple and needs > adjustments, but generally adding a CLK_SET_RATE_PARENT to a mux won't > work. >
On Fri, Apr 27, 2012 at 03:55:41PM +0800, Shawn Guo wrote: > On Fri, Apr 27, 2012 at 09:16:22AM +0200, Sascha Hauer wrote: > > On Fri, Apr 27, 2012 at 02:40:09PM +0800, Shawn Guo wrote: > > > On Wed, Apr 25, 2012 at 05:28:09PM +0200, Sascha Hauer wrote: > > > > - Add necessary #ifdefs for CONFIG_COMMON_CLOCK > > > > - Add a global spinlock to protect the CCM registers > > > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > > --- > > > > + > > > > +static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg, > > > > + u8 shift, u8 width, const char **parents, int num_parents) > > > > +{ > > > > + return clk_register_mux(NULL, name, parents, num_parents, 0, reg, shift, > > > > > > I think the fourth parameter should be CLK_SET_RATE_PARENT too, as mux > > > could also likely be in a clk_set_rate propagation path, saying it has > > > a parent clk who has .round_rate and .set_rate operations. > > > > Nope, we can't do this. When we do this every clock_set_rate on a leaf > > node will propagate up to the ipg, ahb, main_clk, plls and whatever is > > up there. > > PLL does not have CLK_SET_RATE_PARENT set, right? > > What's the problem with that propagation? Isn't it designed so? Now > on imx6q, 792MHz is a set point for cpu frequency, we have to propagate > up to pll's set_rate to get that frequency. There is a mux in the > middle of the propagation path. If we do not set CLK_SET_RATE_PARENT > for mux, how can we get this frequency? We need a way to set the propagation flag on some muxes (maybe also a way to not set it on some dividers or gates). As said, the assumption that all dividers/gates can propagate whereas all muxes cannot propagate may turn out to be too simple. You found one case where this is wrong, I'm sure there are others. Sascha
diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h new file mode 100644 index 0000000..00f2590 --- /dev/null +++ b/arch/arm/mach-imx/clk.h @@ -0,0 +1,44 @@ +#ifndef __MACH_IMX_CLK_H +#define __MACH_IMX_CLK_H + +#include <linux/spinlock.h> +#include <linux/clk-provider.h> +#include <mach/clock.h> + +struct clk *imx_clk_pllv1(const char *name, char *parent, + void __iomem *base); + +static inline struct clk *imx_clk_fixed(const char *name, int rate) +{ + return clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate); +} + +static inline struct clk *imx_clk_divider(const char *name, const char *parent, + void __iomem *reg, u8 shift, u8 width) +{ + return clk_register_divider(NULL, name, parent, CLK_SET_RATE_PARENT, + reg, shift, width, 0, &imx_ccm_lock); +} + +static inline struct clk *imx_clk_gate(const char *name, const char *parent, + void __iomem *reg, u8 shift) +{ + return clk_register_gate(NULL, name, parent, CLK_SET_RATE_PARENT, reg, + shift, 0, &imx_ccm_lock); +} + +static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg, + u8 shift, u8 width, const char **parents, int num_parents) +{ + return clk_register_mux(NULL, name, parents, num_parents, 0, 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) +{ + return clk_register_fixed_factor(NULL, name, parent, + CLK_SET_RATE_PARENT, mult, div); +} + +#endif diff --git a/arch/arm/plat-mxc/clock.c b/arch/arm/plat-mxc/clock.c index 2ed3ab1..5079787 100644 --- a/arch/arm/plat-mxc/clock.c +++ b/arch/arm/plat-mxc/clock.c @@ -41,6 +41,7 @@ #include <mach/clock.h> #include <mach/hardware.h> +#ifndef CONFIG_COMMON_CLK static LIST_HEAD(clocks); static DEFINE_MUTEX(clocks_mutex); @@ -200,6 +201,16 @@ struct clk *clk_get_parent(struct clk *clk) } EXPORT_SYMBOL(clk_get_parent); +#else + +/* + * Lock to protect the clock module (ccm) registers. Used + * on all i.MXs + */ +DEFINE_SPINLOCK(imx_ccm_lock); + +#endif /* CONFIG_COMMON_CLK */ + /* * Get the resulting clock rate from a PLL register value and the input * frequency. PLLs with this register layout can at least be found on diff --git a/arch/arm/plat-mxc/include/mach/clock.h b/arch/arm/plat-mxc/include/mach/clock.h index 753a598..bd940c7 100644 --- a/arch/arm/plat-mxc/include/mach/clock.h +++ b/arch/arm/plat-mxc/include/mach/clock.h @@ -23,6 +23,7 @@ #ifndef __ASSEMBLY__ #include <linux/list.h> +#ifndef CONFIG_COMMON_CLK struct module; struct clk { @@ -59,6 +60,9 @@ struct clk { int clk_register(struct clk *clk); void clk_unregister(struct clk *clk); +#endif /* CONFIG_COMMON_CLK */ + +extern spinlock_t imx_ccm_lock; unsigned long mxc_decode_pll(unsigned int pll, u32 f_ref);
- Add necessary #ifdefs for CONFIG_COMMON_CLOCK - Add a global spinlock to protect the CCM registers Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- arch/arm/mach-imx/clk.h | 44 ++++++++++++++++++++++++++++++++ arch/arm/plat-mxc/clock.c | 11 ++++++++ arch/arm/plat-mxc/include/mach/clock.h | 4 +++ 3 files changed, 59 insertions(+) create mode 100644 arch/arm/mach-imx/clk.h