Patchwork [19/33] ARM i.MX: prepare for common clock framework

login
register
mail settings
Submitter Sascha Hauer
Date April 25, 2012, 3:28 p.m.
Message ID <1335367703-19929-20-git-send-email-s.hauer@pengutronix.de>
Download mbox | patch
Permalink /patch/154999/
State New
Headers show

Comments

Sascha Hauer - April 25, 2012, 3:28 p.m.
- 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
Shawn Guo - April 27, 2012, 6:40 a.m.
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);
> +}
> +
Sascha Hauer - April 27, 2012, 7:16 a.m.
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
Shawn Guo - April 27, 2012, 7:55 a.m.
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.
>
Sascha Hauer - April 27, 2012, 8:09 a.m.
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

Patch

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