[RFC,PATCH 0/7 v2] Common struct clk implementation

[RFC,PATCH 0/7 v2] Common struct clk implementation

[Jeremy Kerr]

Hi all,

These patches are an attempt to allow platforms to share clock code. At
present, the definitions of struct clk are local to platform code, which
makes allocating and initialising cross-platform clock sources
difficult.

The first two patches are for the architecture-independent kernel code,
introducing the common clk API. The remaining patches are specific to
the ARM 'versatile' and 'realview' platforms.  I've included them in
this series to illustrate how the clock API is used, by using a generic
interface for the icst307 clock, and sharing it between realview and
versatile.

Ben Herrenschmidt is looking at using common struct clk code for powerpc
too, hence the kernel-wide approach.

Comments most welcome.

Cheers,


Jeremy

--
v2:
 * no longer ARM-specific
 * use clk_operations

---
Jeremy Kerr (7):
      Add a common struct clk
      Generic support for fixed-rate clocks
      arm/versatile: use generic struct clk
      arm/versatile: remove oscoff from clk_versatile
      arm/realview: use generic struct clk
      arm/icst307: use common struct clk, unify realview and versatile clocks
      arm/icst307: remove icst307_ps_to_vco

[RFC,PATCH 5/7 v2] arm/realview: use generic struct clk

[Jeremy Kerr]

Use the common struct clk interface for the realview clocks.

Compile tested only.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 arch/arm/Kconfig               |    1 
 arch/arm/mach-realview/clock.c |   48 +++++++++++++--------------------
 arch/arm/mach-realview/clock.h |   17 ++++++-----
 arch/arm/mach-realview/core.c  |   46 +++++++++++++++----------------
 4 files changed, 54 insertions(+), 58 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 34497ce..2ecef6b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -232,6 +232,7 @@ config ARCH_REALVIEW
 	select ICST307
 	select GENERIC_TIME
 	select GENERIC_CLOCKEVENTS
+	select USE_COMMON_STRUCT_CLK
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	help
 	  This enables support for ARM Ltd RealView boards.
diff --git a/arch/arm/mach-realview/clock.c b/arch/arm/mach-realview/clock.c
index a704311..472721c 100644
--- a/arch/arm/mach-realview/clock.c
+++ b/arch/arm/mach-realview/clock.c
@@ -22,43 +22,35 @@
 
 #include "clock.h"
 
-int clk_enable(struct clk *clk)
-{
-	return 0;
-}
-EXPORT_SYMBOL(clk_enable);
+#define to_clk_realview(clk) (container_of(clk, struct clk_realview, clk))
 
-void clk_disable(struct clk *clk)
+static unsigned long clk_realview_get_rate(struct clk *clk)
 {
+	return to_clk_realview(clk)->rate;
 }
-EXPORT_SYMBOL(clk_disable);
 
-unsigned long clk_get_rate(struct clk *clk)
-{
-	return clk->rate;
-}
-EXPORT_SYMBOL(clk_get_rate);
-
-long clk_round_rate(struct clk *clk, unsigned long rate)
+static long clk_realview_round_rate(struct clk *clk, unsigned long rate)
 {
+	const struct icst307_params *params = &to_clk_realview(clk)->params;
 	struct icst307_vco vco;
-	vco = icst307_khz_to_vco(clk->params, rate / 1000);
-	return icst307_khz(clk->params, vco) * 1000;
+	vco = icst307_khz_to_vco(params, rate / 1000);
+	return icst307_khz(params, vco) * 1000;
 }
-EXPORT_SYMBOL(clk_round_rate);
 
-int clk_set_rate(struct clk *clk, unsigned long rate)
+static int clk_realview_set_rate(struct clk *clk, unsigned long rate)
 {
-	int ret = -EIO;
+	struct clk_realview *r_clk = to_clk_realview(clk);
+	struct icst307_vco vco;
 
-	if (clk->setvco) {
-		struct icst307_vco vco;
+	vco = icst307_khz_to_vco(&r_clk->params, rate / 1000);
+	r_clk->rate = icst307_khz(&r_clk->params, vco) * 1000;
+	r_clk->setvco(r_clk, vco);
 
-		vco = icst307_khz_to_vco(clk->params, rate / 1000);
-		clk->rate = icst307_khz(clk->params, vco) * 1000;
-		clk->setvco(clk, vco);
-		ret = 0;
-	}
-	return ret;
+	return 0;
 }
-EXPORT_SYMBOL(clk_set_rate);
+
+struct clk_operations clk_realview_operations = {
+	.get_rate = clk_realview_get_rate,
+	.round_rate = clk_realview_round_rate,
+	.set_rate = clk_realview_set_rate,
+};
diff --git a/arch/arm/mach-realview/clock.h b/arch/arm/mach-realview/clock.h
index ebbb0f0..639a381 100644
--- a/arch/arm/mach-realview/clock.h
+++ b/arch/arm/mach-realview/clock.h
@@ -8,12 +8,15 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-struct module;
-struct icst307_params;
 
-struct clk {
-	unsigned long		rate;
-	const struct icst307_params *params;
-	void			*data;
-	void			(*setvco)(struct clk *, struct icst307_vco vco);
+#include <linux/clk.h>
+
+struct clk_realview {
+	struct clk			clk;
+	unsigned long			rate;
+	const struct icst307_params	params;
+	void				(*setvco)(struct clk_realview *,
+						  struct icst307_vco);
 };
+
+extern struct clk_operations clk_realview_operations;
diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index 9f29343..8102c75 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -273,16 +273,8 @@ struct mmci_platform_data realview_mmc1_plat_data = {
 /*
  * Clock handling
  */
-static const struct icst307_params realview_oscvco_params = {
-	.ref		= 24000,
-	.vco_max	= 200000,
-	.vd_min		= 4 + 8,
-	.vd_max		= 511 + 8,
-	.rd_min		= 1 + 2,
-	.rd_max		= 127 + 2,
-};
 
-static void realview_oscvco_set(struct clk *clk, struct icst307_vco vco)
+static void realview_oscvco_set(struct clk_realview *clk, struct icst307_vco vco)
 {
 	void __iomem *sys_lock = __io_address(REALVIEW_SYS_BASE) + REALVIEW_SYS_LOCK_OFFSET;
 	void __iomem *sys_osc;
@@ -301,46 +293,54 @@ static void realview_oscvco_set(struct clk *clk, struct icst307_vco vco)
 	writel(0, sys_lock);
 }
 
-static struct clk oscvco_clk = {
-	.params	= &realview_oscvco_params,
+static struct clk_realview oscvco_clk = {
+	.clk = {
+		.ops = &clk_realview_operations,
+	},
+	.params	= {
+		.ref		= 24000,
+		.vco_max	= 200000,
+		.vd_min		= 4 + 8,
+		.vd_max		= 511 + 8,
+		.rd_min		= 1 + 2,
+		.rd_max		= 127 + 2,
+	},
 	.setvco = realview_oscvco_set,
 };
 
 /*
  * These are fixed clocks.
  */
-static struct clk ref24_clk = {
-	.rate	= 24000000,
-};
+static struct clk_fixed ref24_clk = DEFINE_CLK_FIXED(24000000);
 
 static struct clk_lookup lookups[] = {
 	{	/* UART0 */
 		.dev_id		= "dev:uart0",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* UART1 */
 		.dev_id		= "dev:uart1",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* UART2 */
 		.dev_id		= "dev:uart2",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* UART3 */
 		.dev_id		= "fpga:uart3",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* KMI0 */
 		.dev_id		= "fpga:kmi0",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* KMI1 */
 		.dev_id		= "fpga:kmi1",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* MMC0 */
 		.dev_id		= "fpga:mmc0",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* EB:CLCD */
 		.dev_id		= "dev:clcd",
-		.clk		= &oscvco_clk,
+		.clk		= &oscvco_clk.clk,
 	}, {	/* PB:CLCD */
 		.dev_id		= "issp:clcd",
-		.clk		= &oscvco_clk,
+		.clk		= &oscvco_clk.clk,
 	}
 };
 

[RFC,PATCH 7/7 v2] arm/icst307: remove icst307_ps_to_vco

[Jeremy Kerr]

icst307_ps_to_vco isn't used, so remove it.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 arch/arm/common/icst307.c |   62 --------------------------------------
 1 file changed, 62 deletions(-)

diff --git a/arch/arm/common/icst307.c b/arch/arm/common/icst307.c
index cd45b88..9d8d087 100644
--- a/arch/arm/common/icst307.c
+++ b/arch/arm/common/icst307.c
@@ -96,68 +96,6 @@ icst307_khz_to_vco(const struct icst307_params *p, unsigned long freq)
 	return vco;
 }
 
-static struct icst307_vco
-icst307_ps_to_vco(const struct icst307_params *p, unsigned long period)
-{
-	struct icst307_vco vco = { .s = 1, .v = p->vd_max, .r = p->rd_max };
-	unsigned long f, ps;
-	unsigned int i = 0, rd, best = (unsigned int)-1;
-
-	ps = 1000000000UL / p->vco_max;
-
-	/*
-	 * First, find the PLL output divisor such
-	 * that the PLL output is within spec.
-	 */
-	do {
-		f = period / s2div[idx2s[i]];
-
-		/*
-		 * f must be between 6MHz and 200MHz (3.3 or 5V)
-		 */
-		if (f >= ps && f < 1000000000UL / 6000 + 1)
-			break;
-	} while (i < ARRAY_SIZE(idx2s));
-
-	if (i >= ARRAY_SIZE(idx2s))
-		return vco;
-
-	vco.s = idx2s[i];
-
-	ps = 500000000UL / p->ref;
-
-	/*
-	 * Now find the closest divisor combination
-	 * which gives a PLL output of 'f'.
-	 */
-	for (rd = p->rd_min; rd <= p->rd_max; rd++) {
-		unsigned long f_in_div, f_pll;
-		unsigned int vd;
-		int f_diff;
-
-		f_in_div = ps * rd;
-
-		vd = (f_in_div + f / 2) / f;
-		if (vd < p->vd_min || vd > p->vd_max)
-			continue;
-
-		f_pll = (f_in_div + vd / 2) / vd;
-		f_diff = f_pll - f;
-		if (f_diff < 0)
-			f_diff = -f_diff;
-
-		if ((unsigned)f_diff < best) {
-			vco.v = vd - 8;
-			vco.r = rd - 2;
-			if (f_diff == 0)
-				break;
-			best = f_diff;
-		}
-	}
-
-	return vco;
-}
-
 static unsigned long clk_icst307_get_rate(struct clk *clk)
 {
 	return to_clk_icst307(clk)->rate;

[RFC,PATCH 3/7 v2] arm/versatile: use generic struct clk

[Jeremy Kerr]

Use the common struct clk interface for the versatile clocks.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 arch/arm/Kconfig                |    1 
 arch/arm/common/clkdev.c        |    2 +
 arch/arm/mach-versatile/clock.c |   51 +++++++++++++-------------------
 arch/arm/mach-versatile/clock.h |   20 +++++++-----
 arch/arm/mach-versatile/core.c  |   50 +++++++++++++++----------------
 5 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 233a222..34497ce 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -245,6 +245,7 @@ config ARCH_VERSATILE
 	select ICST307
 	select GENERIC_TIME
 	select GENERIC_CLOCKEVENTS
+	select USE_COMMON_STRUCT_CLK
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	help
 	  This enables support for ARM Ltd Versatile board.
diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c
index aae5bc0..71e7596 100644
--- a/arch/arm/common/clkdev.c
+++ b/arch/arm/common/clkdev.c
@@ -85,11 +85,13 @@ struct clk *clk_get(struct device *dev, const char *con_id)
 }
 EXPORT_SYMBOL(clk_get);
 
+#ifndef CONFIG_USE_COMMON_STRUCT_CLK
 void clk_put(struct clk *clk)
 {
 	__clk_put(clk);
 }
 EXPORT_SYMBOL(clk_put);
+#endif /* CONFIG_USE_COMMON_STRUCT_CLK */
 
 void clkdev_add(struct clk_lookup *cl)
 {
diff --git a/arch/arm/mach-versatile/clock.c b/arch/arm/mach-versatile/clock.c
index c50a44e..16ab90b 100644
--- a/arch/arm/mach-versatile/clock.c
+++ b/arch/arm/mach-versatile/clock.c
@@ -17,49 +17,42 @@
 #include <linux/string.h>
 #include <linux/clk.h>
 #include <linux/mutex.h>
+#include <linux/clk.h>
 
-#include <asm/clkdev.h>
 #include <asm/hardware/icst307.h>
 
 #include "clock.h"
 
-int clk_enable(struct clk *clk)
-{
-	return 0;
-}
-EXPORT_SYMBOL(clk_enable);
+#define to_clk_versatile(clk) (container_of(clk, struct clk_versatile, clk))
 
-void clk_disable(struct clk *clk)
+static unsigned long clk_versatile_get_rate(struct clk *clk)
 {
+	return to_clk_versatile(clk)->rate;
 }
-EXPORT_SYMBOL(clk_disable);
 
-unsigned long clk_get_rate(struct clk *clk)
-{
-	return clk->rate;
-}
-EXPORT_SYMBOL(clk_get_rate);
-
-long clk_round_rate(struct clk *clk, unsigned long rate)
+static long clk_versatile_round_rate(struct clk *clk, unsigned long rate)
 {
+	const struct icst307_params *params = &to_clk_versatile(clk)->params;
 	struct icst307_vco vco;
-	vco = icst307_khz_to_vco(clk->params, rate / 1000);
-	return icst307_khz(clk->params, vco) * 1000;
+
+	vco = icst307_khz_to_vco(params, rate / 1000);
+	return icst307_khz(params, vco) * 1000;
 }
-EXPORT_SYMBOL(clk_round_rate);
 
-int clk_set_rate(struct clk *clk, unsigned long rate)
+static int clk_versatile_set_rate(struct clk *clk, unsigned long rate)
 {
-	int ret = -EIO;
+	struct clk_versatile *v_clk = to_clk_versatile(clk);
+	struct icst307_vco vco;
 
-	if (clk->setvco) {
-		struct icst307_vco vco;
+	vco = icst307_khz_to_vco(&v_clk->params, rate / 1000);
+	v_clk->rate = icst307_khz(&v_clk->params, vco) * 1000;
+	v_clk->setvco(v_clk, vco);
 
-		vco = icst307_khz_to_vco(clk->params, rate / 1000);
-		clk->rate = icst307_khz(clk->params, vco) * 1000;
-		clk->setvco(clk, vco);
-		ret = 0;
-	}
-	return ret;
+	return 0;
 }
-EXPORT_SYMBOL(clk_set_rate);
+
+struct clk_operations clk_versatile_operations = {
+	.get_rate = clk_versatile_get_rate,
+	.round_rate = clk_versatile_round_rate,
+	.set_rate = clk_versatile_set_rate,
+};
diff --git a/arch/arm/mach-versatile/clock.h b/arch/arm/mach-versatile/clock.h
index 03468fd..d1c8791 100644
--- a/arch/arm/mach-versatile/clock.h
+++ b/arch/arm/mach-versatile/clock.h
@@ -8,13 +8,17 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-struct module;
-struct icst307_params;
 
-struct clk {
-	unsigned long		rate;
-	const struct icst307_params *params;
-	u32			oscoff;
-	void			*data;
-	void			(*setvco)(struct clk *, struct icst307_vco vco);
+#include <linux/clk.h>
+
+struct clk_versatile {
+	struct clk			clk;
+	unsigned long			rate;
+	const struct icst307_params	params;
+	u32				oscoff;
+	void				(*setvco)(struct clk_versatile *,
+						  struct icst307_vco);
 };
+
+extern struct clk_operations clk_versatile_operations;
+
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index e13be7c..b6964bc 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -379,16 +379,8 @@ static struct mmci_platform_data mmc0_plat_data = {
 /*
  * Clock handling
  */
-static const struct icst307_params versatile_oscvco_params = {
-	.ref		= 24000,
-	.vco_max	= 200000,
-	.vd_min		= 4 + 8,
-	.vd_max		= 511 + 8,
-	.rd_min		= 1 + 2,
-	.rd_max		= 127 + 2,
-};
 
-static void versatile_oscvco_set(struct clk *clk, struct icst307_vco vco)
+static void versatile_oscvco_set(struct clk_versatile *clk, struct icst307_vco vco)
 {
 	void __iomem *sys = __io_address(VERSATILE_SYS_BASE);
 	void __iomem *sys_lock = sys + VERSATILE_SYS_LOCK_OFFSET;
@@ -402,47 +394,55 @@ static void versatile_oscvco_set(struct clk *clk, struct icst307_vco vco)
 	writel(0, sys_lock);
 }
 
-static struct clk osc4_clk = {
-	.params	= &versatile_oscvco_params,
-	.oscoff	= VERSATILE_SYS_OSCCLCD_OFFSET,
-	.setvco	= versatile_oscvco_set,
+static struct clk_versatile osc4_clk = {
+	.clk = {
+		.ops = &clk_versatile_operations,
+	},
+	.params = {
+		.ref		= 24000,
+		.vco_max	= 200000,
+		.vd_min		= 4 + 8,
+		.vd_max		= 511 + 8,
+		.rd_min		= 1 + 2,
+		.rd_max		= 127 + 2,
+	},
+	.oscoff = VERSATILE_SYS_OSCCLCD_OFFSET,
+	.setvco = versatile_oscvco_set,
 };
 
 /*
  * These are fixed clocks.
  */
-static struct clk ref24_clk = {
-	.rate	= 24000000,
-};
+static struct clk_fixed ref24_clk = DEFINE_CLK_FIXED(24000000);
 
 static struct clk_lookup lookups[] = {
 	{	/* UART0 */
 		.dev_id		= "dev:f1",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* UART1 */
 		.dev_id		= "dev:f2",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* UART2 */
 		.dev_id		= "dev:f3",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* UART3 */
 		.dev_id		= "fpga:09",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* KMI0 */
 		.dev_id		= "fpga:06",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* KMI1 */
 		.dev_id		= "fpga:07",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* MMC0 */
 		.dev_id		= "fpga:05",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* MMC1 */
 		.dev_id		= "fpga:0b",
-		.clk		= &ref24_clk,
+		.clk		= &ref24_clk.clk,
 	}, {	/* CLCD */
 		.dev_id		= "dev:20",
-		.clk		= &osc4_clk,
+		.clk		= &osc4_clk.clk,
 	}
 };
 

Re: [RFC,PATCH 3/7 v2] arm/versatile: use generic struct clk

[Russell King - ARM Linux]

On Tue, Jan 12, 2010 at 05:58:31PM +1100, Jeremy Kerr wrote:
> diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c
> index aae5bc0..71e7596 100644
> --- a/arch/arm/common/clkdev.c
> +++ b/arch/arm/common/clkdev.c
> @@ -85,11 +85,13 @@ struct clk *clk_get(struct device *dev, const char *con_id)
>  }
>  EXPORT_SYMBOL(clk_get);
>  
> +#ifndef CONFIG_USE_COMMON_STRUCT_CLK
>  void clk_put(struct clk *clk)
>  {
>  	__clk_put(clk);
>  }
>  EXPORT_SYMBOL(clk_put);
> +#endif /* CONFIG_USE_COMMON_STRUCT_CLK */

This doesn't make any sense.  What are you trying to do here?

The get/put operations go together as one logical set - that's why you
get both if you're using clkdev, and why you're asked to implement both
__clk_get() and __clk_put() in arch code to do whatever's necessary
with the clock.

Let me guess, and say that clk_put() is not a release method.  It's a
method for drivers to say "I'm done with this clock" and for the arch
code to reverse the effects of __clk_get()/clk_get().

Currently, its only user is to balance module counts.

Re: [RFC,PATCH 3/7 v2] arm/versatile: use generic struct clk

[Jeremy Kerr]

Hi Russell,

> This doesn't make any sense.  What are you trying to do here?
> 
> The get/put operations go together as one logical set - that's why you
> get both if you're using clkdev, and why you're asked to implement both
> __clk_get() and __clk_put() in arch code to do whatever's necessary
> with the clock.

I'm assuming that clk_put will be specific to the implementation; fixed clocks 
probably won't need to do anything, but others may need a refcount, etc.

We don't have a struct clk to use clk_get on, so it remains a non-clock-
specific function.

We'll probably need the symmetry for some cases though, I think that a 
__clk_get(struct clk *) member of clk_operations would work for this, to be 
called by clk_get.

Cheers,


Jeremy

[RFC,PATCH 4/7 v2] arm/versatile: remove oscoff from clk_versatile

[Jeremy Kerr]

We only use one value for oscoff, so remove it from the struct.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 arch/arm/mach-versatile/clock.h |    1 -
 arch/arm/mach-versatile/core.c  |    6 +++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-versatile/clock.h b/arch/arm/mach-versatile/clock.h
index d1c8791..6ac30c5 100644
--- a/arch/arm/mach-versatile/clock.h
+++ b/arch/arm/mach-versatile/clock.h
@@ -15,7 +15,6 @@ struct clk_versatile {
 	struct clk			clk;
 	unsigned long			rate;
 	const struct icst307_params	params;
-	u32				oscoff;
 	void				(*setvco)(struct clk_versatile *,
 						  struct icst307_vco);
 };
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index b6964bc..2b670bb 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -384,13 +384,14 @@ static void versatile_oscvco_set(struct clk_versatile *clk, struct icst307_vco v
 {
 	void __iomem *sys = __io_address(VERSATILE_SYS_BASE);
 	void __iomem *sys_lock = sys + VERSATILE_SYS_LOCK_OFFSET;
+	void __iomem *sys_osc = sys + VERSATILE_SYS_OSCCLCD_OFFSET;
 	u32 val;
 
-	val = readl(sys + clk->oscoff) & ~0x7ffff;
+	val = readl(sys_osc) & ~0x7ffff;
 	val |= vco.v | (vco.r << 9) | (vco.s << 16);
 
 	writel(0xa05f, sys_lock);
-	writel(val, sys + clk->oscoff);
+	writel(val, sys_osc);
 	writel(0, sys_lock);
 }
 
@@ -406,7 +407,6 @@ static struct clk_versatile osc4_clk = {
 		.rd_min		= 1 + 2,
 		.rd_max		= 127 + 2,
 	},
-	.oscoff = VERSATILE_SYS_OSCCLCD_OFFSET,
 	.setvco = versatile_oscvco_set,
 };
 

[RFC,PATCH 2/7 v2] Generic support for fixed-rate clocks

[Jeremy Kerr]

Since most platforms will need a fixed-rate clock, add one. This will
also serve as a basic example of an implementation of struct clk.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 include/linux/clk.h |   12 ++++++++++++
 kernel/Makefile     |    1 +
 kernel/clk.c        |   25 +++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/include/linux/clk.h b/include/linux/clk.h
index 1a6199e..8293421 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -97,6 +97,18 @@ static inline struct clk *clk_get_parent(struct clk *clk)
 	return ERR_PTR(-ENOSYS);
 }
 
+/* Simple fixed-rate clock */
+struct clk_fixed {
+	struct clk	clk;
+	unsigned long	rate;
+};
+
+extern struct clk_operations clk_fixed_operations;
+
+#define DEFINE_CLK_FIXED(r) { \
+	.clk = { .ops = &clk_fixed_operations }, \
+	.rate = (r) \
+};
 
 #else /* !CONFIG_USE_COMMON_STRUCT_CLK */
 
diff --git a/kernel/Makefile b/kernel/Makefile
index 864ff75..f63d021 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_SLOW_WORK_DEBUG) += slow-work-debugfs.o
 obj-$(CONFIG_PERF_EVENTS) += perf_event.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
 obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
+obj-$(CONFIG_USE_COMMON_STRUCT_CLK) += clk.o
 
 ifneq ($(CONFIG_SCHED_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <alan@linuxcare.com.au>, the -fno-omit-frame-pointer is
diff --git a/kernel/clk.c b/kernel/clk.c
new file mode 100644
index 0000000..353736c
--- /dev/null
+++ b/kernel/clk.c
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2010 Canonical Ltd <jeremy.kerr@canonical.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Standard functionality for the common clock API.
+ */
+
+#include <linux/module.h>
+#include <linux/clk.h>
+
+#define to_clk_fixed(clk) (container_of(clk, struct clk_fixed, clk))
+
+static unsigned long clk_fixed_get_rate(struct clk *clk)
+{
+	return to_clk_fixed(clk)->rate;
+}
+
+struct clk_operations clk_fixed_operations = {
+	.get_rate = clk_fixed_get_rate,
+};
+
+EXPORT_SYMBOL_GPL(clk_fixed_operations);

[RFC,PATCH 1/7 v2] Add a common struct clk

[Jeremy Kerr]

We currently have 21 definitions of struct clk in the ARM architecture,
each defined on a per-platform basis. This makes it difficult to define
platform- (or architecture-) independent clock sources without making
assumptions about struct clk.

This change is an effort to unify struct clk where possible, by defining
a common struct clk, containing a set of clock operations. Different
clock implementations can set their own operations, and have a standard
interface for generic code. The callback interface is exposed to the
kernel proper, while the clock implementations only need to be seen by
the platform internals.

This allows us to share clock code among platforms, and makes it
possible to dynamically create clock devices in platform-independent
code.

Platforms can enable the generic struct clock through
CONFIG_USE_COMMON_STRUCT_CLK.

The common clock definitions are based on a development patch from Ben
Herrenschmidt <benh@kernel.crashing.org>.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 arch/Kconfig        |    3 
 include/linux/clk.h |  134 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 110 insertions(+), 27 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 9d055b4..cefc026 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -140,4 +140,7 @@ config HAVE_HW_BREAKPOINT
 config HAVE_USER_RETURN_NOTIFIER
 	bool
 
+config USE_COMMON_STRUCT_CLK
+	bool
+
 source "kernel/gcov/Kconfig"
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 1d37f42..1a6199e 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -11,36 +11,101 @@
 #ifndef __LINUX_CLK_H
 #define __LINUX_CLK_H
 
-struct device;
+#include <linux/err.h>
 
-/*
- * The base API.
+#ifdef CONFIG_USE_COMMON_STRUCT_CLK
+
+/* If we're using the common struct clk, we define the base clk object here,
+ * which will be 'subclassed' by device-specific implementations. For example:
+ *
+ *  struct clk_foo {
+ *      struct clk;
+ *      [device specific fields]
+ *  };
+ *
+ * We define the common clock API through a set of static inlines that call the
+ * corresponding clk_operations. The API is exactly the same as that documented
+ * in the !CONFIG_USE_COMMON_STRUCT_CLK case.
  */
 
+struct clk {
+	const struct clk_operations *ops;
+};
+
+struct clk_operations {
+       int             (*enable)(struct clk *);
+       void            (*disable)(struct clk *);
+       unsigned long   (*get_rate)(struct clk *);
+       void            (*put)(struct clk *);
+       long            (*round_rate)(struct clk *, unsigned long);
+       int             (*set_rate)(struct clk *, unsigned long);
+       int             (*set_parent)(struct clk *, struct clk *);
+       struct clk*     (*get_parent)(struct clk *);
+};
+
+static inline int clk_enable(struct clk *clk)
+{
+	if (clk->ops->enable)
+		return clk->ops->enable(clk);
+	return 0;
+}
+
+static inline void clk_disable(struct clk *clk)
+{
+	if (clk->ops->disable)
+		clk->ops->disable(clk);
+}
+
+static inline unsigned long clk_get_rate(struct clk *clk)
+{
+	if (clk->ops->get_rate)
+		return clk->ops->get_rate(clk);
+	return 0;
+}
+
+static inline void clk_put(struct clk *clk)
+{
+	if (clk->ops->put)
+		clk->ops->put(clk);
+}
+
+static inline long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk->ops->round_rate)
+		return clk->ops->round_rate(clk, rate);
+	return -ENOSYS;
+}
+
+static inline int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk->ops->set_rate)
+		return clk->ops->set_rate(clk, rate);
+	return -ENOSYS;
+}
+
+static inline int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	if (clk->ops->set_parent)
+		return clk->ops->set_parent(clk, parent);
+	return -ENOSYS;
+}
+
+static inline struct clk *clk_get_parent(struct clk *clk)
+{
+	if (clk->ops->get_parent)
+		return clk->ops->get_parent(clk);
+	return ERR_PTR(-ENOSYS);
+}
+
+
+#else /* !CONFIG_USE_COMMON_STRUCT_CLK */
 
 /*
- * struct clk - an machine class defined object / cookie.
+ * Global clock object, actual structure is declared per-machine
  */
 struct clk;
 
 /**
- * clk_get - lookup and obtain a reference to a clock producer.
- * @dev: device for clock "consumer"
- * @id: clock comsumer ID
- *
- * Returns a struct clk corresponding to the clock producer, or
- * valid IS_ERR() condition containing errno.  The implementation
- * uses @dev and @id to determine the clock consumer, and thereby
- * the clock producer.  (IOW, @id may be identical strings, but
- * clk_get may return different clock producers depending on @dev.)
- *
- * Drivers must assume that the clock source is not enabled.
- *
- * clk_get should not be called from within interrupt context.
- */
-struct clk *clk_get(struct device *dev, const char *id);
-
-/**
  * clk_enable - inform the system when the clock source should be running.
  * @clk: clock source
  *
@@ -83,12 +148,6 @@ unsigned long clk_get_rate(struct clk *clk);
  */
 void clk_put(struct clk *clk);
 
-
-/*
- * The remaining APIs are optional for machine class support.
- */
-
-
 /**
  * clk_round_rate - adjust a rate to the exact rate a clock can provide
  * @clk: clock source
@@ -125,6 +184,27 @@ int clk_set_parent(struct clk *clk, struct clk *parent);
  */
 struct clk *clk_get_parent(struct clk *clk);
 
+#endif /* !CONFIG_USE_COMMON_STRUCT_CLK */
+
+struct device;
+
+/**
+ * clk_get - lookup and obtain a reference to a clock producer.
+ * @dev: device for clock "consumer"
+ * @id: clock comsumer ID
+ *
+ * Returns a struct clk corresponding to the clock producer, or
+ * valid IS_ERR() condition containing errno.  The implementation
+ * uses @dev and @id to determine the clock consumer, and thereby
+ * the clock producer.  (IOW, @id may be identical strings, but
+ * clk_get may return different clock producers depending on @dev.)
+ *
+ * Drivers must assume that the clock source is not enabled.
+ *
+ * clk_get should not be called from within interrupt context.
+ */
+struct clk *clk_get(struct device *dev, const char *id);
+
 /**
  * clk_get_sys - get a clock based upon the device name
  * @dev_id: device name

Re: [RFC,PATCH 1/7 v2] Add a common struct clk

[Francesco VIRLINZI]

Hi Jeremy
In November I already sent a proposal on
  a generic linux clk framework.
On that I would suggest:

>
> +struct clk {
> +	const struct clk_operations *ops;
>    
            spinlock_t lock;
            const char *name;
            int id;
            unsigned long rate;
> +};
> +
> +struct clk_operations {
>    
                int (*init)(struc clk *);
> +       int             (*enable)(struct clk *);
> +       void            (*disable)(struct clk *);
> +       unsigned long   (*get_rate)(struct clk *);
> +       void            (*put)(struct clk *);
> +       long            (*round_rate)(struct clk *, unsigned long);
> +       int             (*set_rate)(struct clk *, unsigned long);
> +       int             (*set_parent)(struct clk *, struct clk *);
> +       struct clk*     (*get_parent)(struct clk *);
> +};
>    

+static inline int clk_enable(struct clk *clk)
+{
	unsigned long flags;
	int ret = 0;
+	if (clk->ops->enable) {
		spinlock_irq_save(&clk->lock, flags);
+		ret = clk->ops->enable(clk);
		spinlock_irq_restore(&clk->lock, flags);
	}
+	return 0;
+}

Something similar in the other function...

> +
> +static inline unsigned long clk_get_rate(struct clk *clk)
> +{
> +	if (clk->ops->get_rate)
> +		return clk->ops->get_rate(clk);
>    
> 	return clk->rate;
> +}
>
>    
Moreover if you support .set_parent and .get_parent  a .parent field in 
the struct clk
  is useful.

As

struct clk {
const struct clk_operations *ops;

     spinlock_t lock;
     const char *name;
     int id;
     unsigned long rate;
     struct clk *parent;
};

Regards
  Francesco

Re: [RFC,PATCH 1/7 v2] Add a common struct clk

[Russell King - ARM Linux]

On Tue, Jan 12, 2010 at 09:48:44AM +0100, Francesco VIRLINZI wrote:
> Hi Jeremy
> In November I already sent a proposal on
>  a generic linux clk framework.
> On that I would suggest:
>
>>
>> +struct clk {
>> +	const struct clk_operations *ops;
>>    
>            spinlock_t lock;
>            const char *name;
>            int id;

Name and ID are totally pointless, unless you insist on using the clk
API in the wrong way (like S3C does.)

Re: [RFC,PATCH 1/7 v2] Add a common struct clk

[Ben Dooks]

On Tue, Jan 12, 2010 at 09:01:49AM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 12, 2010 at 09:48:44AM +0100, Francesco VIRLINZI wrote:
> > Hi Jeremy
> > In November I already sent a proposal on
> >  a generic linux clk framework.
> > On that I would suggest:
> >
> >>
> >> +struct clk {
> >> +	const struct clk_operations *ops;
> >>    
> >            spinlock_t lock;
> >            const char *name;
> >            int id;
> 
> Name and ID are totally pointless, unless you insist on using the clk
> API in the wrong way (like S3C does.)

I do intend to change the clock lookup on the Samsung series, but we're
currently in the process of trying to do a whole pile of work on not
only adding new SoCs but also cleaning up the existing support and making
a whole pile of code common to all the Samsung SoC family.

I never got the time to go throughly through Francesco's clock framework
but it seemed rathe rcomplicated for our current requirements and also
had a whole pile of style problems that made it really difficult to read.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [RFC,PATCH 1/7 v2] Add a common struct clk

[Ben Dooks]

On Tue, Jan 12, 2010 at 09:01:49AM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 12, 2010 at 09:48:44AM +0100, Francesco VIRLINZI wrote:
> > Hi Jeremy
> > In November I already sent a proposal on
> >  a generic linux clk framework.
> > On that I would suggest:
> >
> >>
> >> +struct clk {
> >> +	const struct clk_operations *ops;
> >>    
> >            spinlock_t lock;
> >            const char *name;
> >            int id;
> 
> Name and ID are totally pointless, unless you insist on using the clk
> API in the wrong way (like S3C does.)

I assume by this you mean that a number of the s3c drives use clk_get with
their platform device and a name? if so, I agree this needs to get sorted
out.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [RFC,PATCH 1/7 v2] Add a common struct clk

[Ben Dooks]

On Tue, Jan 12, 2010 at 05:58:31PM +1100, Jeremy Kerr wrote:
> We currently have 21 definitions of struct clk in the ARM architecture,
> each defined on a per-platform basis. This makes it difficult to define
> platform- (or architecture-) independent clock sources without making
> assumptions about struct clk.
> 
> This change is an effort to unify struct clk where possible, by defining
> a common struct clk, containing a set of clock operations. Different
> clock implementations can set their own operations, and have a standard
> interface for generic code. The callback interface is exposed to the
> kernel proper, while the clock implementations only need to be seen by
> the platform internals.
> 
> This allows us to share clock code among platforms, and makes it
> possible to dynamically create clock devices in platform-independent
> code.
> 
> Platforms can enable the generic struct clock through
> CONFIG_USE_COMMON_STRUCT_CLK.
> 
> The common clock definitions are based on a development patch from Ben
> Herrenschmidt <benh@kernel.crashing.org>.
> 
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> ---
>  arch/Kconfig        |    3 
>  include/linux/clk.h |  134 +++++++++++++++++++++++++++++++++++---------
>  2 files changed, 110 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 9d055b4..cefc026 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -140,4 +140,7 @@ config HAVE_HW_BREAKPOINT
>  config HAVE_USER_RETURN_NOTIFIER
>  	bool
>  
> +config USE_COMMON_STRUCT_CLK
> +	bool
> +
>  source "kernel/gcov/Kconfig"
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 1d37f42..1a6199e 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -11,36 +11,101 @@
>  #ifndef __LINUX_CLK_H
>  #define __LINUX_CLK_H
>  
> -struct device;
> +#include <linux/err.h>
>  
> -/*
> - * The base API.
> +#ifdef CONFIG_USE_COMMON_STRUCT_CLK
> +
> +/* If we're using the common struct clk, we define the base clk object here,
> + * which will be 'subclassed' by device-specific implementations. For example:
> + *
> + *  struct clk_foo {
> + *      struct clk;
> + *      [device specific fields]
> + *  };
> + *
> + * We define the common clock API through a set of static inlines that call the
> + * corresponding clk_operations. The API is exactly the same as that documented
> + * in the !CONFIG_USE_COMMON_STRUCT_CLK case.
>   */
>  
> +struct clk {
> +	const struct clk_operations *ops;
> +};
> +
> +struct clk_operations {
> +       int             (*enable)(struct clk *);

Personaly I'd leave the enable/disable as one call which takes
(struct clk *, bool on). these code paths tend to be pretty much the
same read/modify/write with only the modify changing depending on the
on parameter.

> +       void            (*disable)(struct clk *);
> +       unsigned long   (*get_rate)(struct clk *);
> +       void            (*put)(struct clk *);
> +       long            (*round_rate)(struct clk *, unsigned long);
> +       int             (*set_rate)(struct clk *, unsigned long);
> +       int             (*set_parent)(struct clk *, struct clk *);
> +       struct clk*     (*get_parent)(struct clk *);
> +};
> +
> +static inline int clk_enable(struct clk *clk)
> +{
> +	if (clk->ops->enable)
> +		return clk->ops->enable(clk);
> +	return 0;
> +}
> +
> +static inline void clk_disable(struct clk *clk)
> +{
> +	if (clk->ops->disable)
> +		clk->ops->disable(clk);
> +}
> +
> +static inline unsigned long clk_get_rate(struct clk *clk)
> +{
> +	if (clk->ops->get_rate)
> +		return clk->ops->get_rate(clk);
> +	return 0;
> +}
> +
> +static inline void clk_put(struct clk *clk)
> +{
> +	if (clk->ops->put)
> +		clk->ops->put(clk);
> +}
> +
> +static inline long clk_round_rate(struct clk *clk, unsigned long rate)
> +{
> +	if (clk->ops->round_rate)
> +		return clk->ops->round_rate(clk, rate);
> +	return -ENOSYS;
> +}
> +
> +static inline int clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> +	if (clk->ops->set_rate)
> +		return clk->ops->set_rate(clk, rate);
> +	return -ENOSYS;
> +}
> +
> +static inline int clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> +	if (clk->ops->set_parent)
> +		return clk->ops->set_parent(clk, parent);
> +	return -ENOSYS;
> +}
> +
> +static inline struct clk *clk_get_parent(struct clk *clk)
> +{
> +	if (clk->ops->get_parent)
> +		return clk->ops->get_parent(clk);
> +	return ERR_PTR(-ENOSYS);
> +}

In the Samsung impelemtnations, the clk keeps a parent pointer and
we just return that as the esult of the clk_get_parent nless the clock
wants to override it.

> +
> +#else /* !CONFIG_USE_COMMON_STRUCT_CLK */
>  
>  /*
> - * struct clk - an machine class defined object / cookie.
> + * Global clock object, actual structure is declared per-machine
>   */
>  struct clk;
>  
>  /**
> - * clk_get - lookup and obtain a reference to a clock producer.
> - * @dev: device for clock "consumer"
> - * @id: clock comsumer ID
> - *
> - * Returns a struct clk corresponding to the clock producer, or
> - * valid IS_ERR() condition containing errno.  The implementation
> - * uses @dev and @id to determine the clock consumer, and thereby
> - * the clock producer.  (IOW, @id may be identical strings, but
> - * clk_get may return different clock producers depending on @dev.)
> - *
> - * Drivers must assume that the clock source is not enabled.
> - *
> - * clk_get should not be called from within interrupt context.
> - */
> -struct clk *clk_get(struct device *dev, const char *id);
> -
> -/**
>   * clk_enable - inform the system when the clock source should be running.
>   * @clk: clock source
>   *
> @@ -83,12 +148,6 @@ unsigned long clk_get_rate(struct clk *clk);
>   */
>  void clk_put(struct clk *clk);
>  
> -
> -/*
> - * The remaining APIs are optional for machine class support.
> - */
> -
> -
>  /**
>   * clk_round_rate - adjust a rate to the exact rate a clock can provide
>   * @clk: clock source
> @@ -125,6 +184,27 @@ int clk_set_parent(struct clk *clk, struct clk *parent);
>   */
>  struct clk *clk_get_parent(struct clk *clk);
>  
> +#endif /* !CONFIG_USE_COMMON_STRUCT_CLK */
> +
> +struct device;
> +
> +/**
> + * clk_get - lookup and obtain a reference to a clock producer.
> + * @dev: device for clock "consumer"
> + * @id: clock comsumer ID
> + *
> + * Returns a struct clk corresponding to the clock producer, or
> + * valid IS_ERR() condition containing errno.  The implementation
> + * uses @dev and @id to determine the clock consumer, and thereby
> + * the clock producer.  (IOW, @id may be identical strings, but
> + * clk_get may return different clock producers depending on @dev.)
> + *
> + * Drivers must assume that the clock source is not enabled.
> + *
> + * clk_get should not be called from within interrupt context.
> + */
> +struct clk *clk_get(struct device *dev, const char *id);
> +
>  /**
>   * clk_get_sys - get a clock based upon the device name
>   * @dev_id: device name
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[RFC, PATCH 6/7 v2] arm/icst307: use common struct clk, unify realview and versatile clocks

[Jeremy Kerr]

Both the realview and versatile platforms use an icst307 clock, but have
their own clock structures.

This change introduces a struct icst307_clk, which is common between
realview and versatile. This allows us to take out the platform-specific
clock defintions.

Tested on versatile, compiled only on realview.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 arch/arm/common/Kconfig                 |    1 
 arch/arm/common/icst307.c               |   39 ++++++++++++++--
 arch/arm/include/asm/hardware/icst307.h |   14 ++++-
 arch/arm/mach-realview/Makefile         |    2 
 arch/arm/mach-realview/clock.c          |   56 -----------------------
 arch/arm/mach-realview/clock.h          |   22 ---------
 arch/arm/mach-realview/core.c           |    7 +-
 arch/arm/mach-realview/realview_pba8.c  |    1 
 arch/arm/mach-versatile/Makefile        |    2 
 arch/arm/mach-versatile/clock.c         |   58 ------------------------
 arch/arm/mach-versatile/clock.h         |   23 ---------
 arch/arm/mach-versatile/core.c          |    7 +-
 12 files changed, 54 insertions(+), 178 deletions(-)

diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 4efbb9d..05e334c 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -17,6 +17,7 @@ config ICST525
 
 config ICST307
 	bool
+	depends on USE_COMMON_STRUCT_CLK
 
 config SA1111
 	bool
diff --git a/arch/arm/common/icst307.c b/arch/arm/common/icst307.c
index 6d094c1..cd45b88 100644
--- a/arch/arm/common/icst307.c
+++ b/arch/arm/common/icst307.c
@@ -19,6 +19,8 @@
 
 #include <asm/hardware/icst307.h>
 
+#define to_clk_icst307(clk) (container_of(clk, struct clk_icst307, clk))
+
 /*
  * Divisors for each OD setting.
  */
@@ -36,7 +38,7 @@ EXPORT_SYMBOL(icst307_khz);
  */
 static unsigned char idx2s[8] = { 1, 6, 3, 4, 7, 5, 2, 0 };
 
-struct icst307_vco
+static struct icst307_vco
 icst307_khz_to_vco(const struct icst307_params *p, unsigned long freq)
 {
 	struct icst307_vco vco = { .s = 1, .v = p->vd_max, .r = p->rd_max };
@@ -94,9 +96,7 @@ icst307_khz_to_vco(const struct icst307_params *p, unsigned long freq)
 	return vco;
 }
 
-EXPORT_SYMBOL(icst307_khz_to_vco);
-
-struct icst307_vco
+static struct icst307_vco
 icst307_ps_to_vco(const struct icst307_params *p, unsigned long period)
 {
 	struct icst307_vco vco = { .s = 1, .v = p->vd_max, .r = p->rd_max };
@@ -158,4 +158,33 @@ icst307_ps_to_vco(const struct icst307_params *p, unsigned long period)
 	return vco;
 }
 
-EXPORT_SYMBOL(icst307_ps_to_vco);
+static unsigned long clk_icst307_get_rate(struct clk *clk)
+{
+	return to_clk_icst307(clk)->rate;
+}
+
+static long clk_icst307_round_rate(struct clk *clk, unsigned long rate)
+{
+	const struct icst307_params *params = &to_clk_icst307(clk)->params;
+	struct icst307_vco vco;
+	vco = icst307_khz_to_vco(params, rate / 1000);
+	return icst307_khz(params, vco) * 1000;
+}
+
+static int clk_icst307_set_rate(struct clk *clk, unsigned long rate)
+{
+	struct clk_icst307 *v_clk = to_clk_icst307(clk);
+	struct icst307_vco vco;
+
+	vco = icst307_khz_to_vco(&v_clk->params, rate / 1000);
+	v_clk->rate = icst307_khz(&v_clk->params, vco) * 1000;
+	v_clk->setvco(v_clk, vco);
+
+	return 0;
+}
+
+struct clk_operations clk_icst307_operations = {
+	.get_rate = clk_icst307_get_rate,
+	.round_rate = clk_icst307_round_rate,
+	.set_rate = clk_icst307_set_rate,
+};
diff --git a/arch/arm/include/asm/hardware/icst307.h b/arch/arm/include/asm/hardware/icst307.h
index 554f128..c7a3b83 100644
--- a/arch/arm/include/asm/hardware/icst307.h
+++ b/arch/arm/include/asm/hardware/icst307.h
@@ -16,6 +16,8 @@
 #ifndef ASMARM_HARDWARE_ICST307_H
 #define ASMARM_HARDWARE_ICST307_H
 
+#include <linux/clk.h>
+
 struct icst307_params {
 	unsigned long	ref;
 	unsigned long	vco_max;	/* inclusive */
@@ -31,8 +33,14 @@ struct icst307_vco {
 	unsigned char	s;
 };
 
-unsigned long icst307_khz(const struct icst307_params *p, struct icst307_vco vco);
-struct icst307_vco icst307_khz_to_vco(const struct icst307_params *p, unsigned long freq);
-struct icst307_vco icst307_ps_to_vco(const struct icst307_params *p, unsigned long period);
+struct clk_icst307 {
+	struct clk			clk;
+	unsigned long			rate;
+	const struct icst307_params	params;
+	void				(*setvco)(struct clk_icst307 *,
+						  struct icst307_vco);
+};
+
+extern struct clk_operations clk_icst307_operations;
 
 #endif
diff --git a/arch/arm/mach-realview/Makefile b/arch/arm/mach-realview/Makefile
index e704edb..a01b76b 100644
--- a/arch/arm/mach-realview/Makefile
+++ b/arch/arm/mach-realview/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the linux kernel.
 #
 
-obj-y					:= core.o clock.o
+obj-y					:= core.o
 obj-$(CONFIG_MACH_REALVIEW_EB)		+= realview_eb.o
 obj-$(CONFIG_MACH_REALVIEW_PB11MP)	+= realview_pb11mp.o
 obj-$(CONFIG_MACH_REALVIEW_PB1176)	+= realview_pb1176.o
diff --git a/arch/arm/mach-realview/clock.c b/arch/arm/mach-realview/clock.c
deleted file mode 100644
index 472721c..0000000
--- a/arch/arm/mach-realview/clock.c
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- *  linux/arch/arm/mach-realview/clock.c
- *
- *  Copyright (C) 2004 ARM Limited.
- *  Written by Deep Blue Solutions Limited.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/device.h>
-#include <linux/list.h>
-#include <linux/errno.h>
-#include <linux/err.h>
-#include <linux/string.h>
-#include <linux/clk.h>
-#include <linux/mutex.h>
-
-#include <asm/hardware/icst307.h>
-
-#include "clock.h"
-
-#define to_clk_realview(clk) (container_of(clk, struct clk_realview, clk))
-
-static unsigned long clk_realview_get_rate(struct clk *clk)
-{
-	return to_clk_realview(clk)->rate;
-}
-
-static long clk_realview_round_rate(struct clk *clk, unsigned long rate)
-{
-	const struct icst307_params *params = &to_clk_realview(clk)->params;
-	struct icst307_vco vco;
-	vco = icst307_khz_to_vco(params, rate / 1000);
-	return icst307_khz(params, vco) * 1000;
-}
-
-static int clk_realview_set_rate(struct clk *clk, unsigned long rate)
-{
-	struct clk_realview *r_clk = to_clk_realview(clk);
-	struct icst307_vco vco;
-
-	vco = icst307_khz_to_vco(&r_clk->params, rate / 1000);
-	r_clk->rate = icst307_khz(&r_clk->params, vco) * 1000;
-	r_clk->setvco(r_clk, vco);
-
-	return 0;
-}
-
-struct clk_operations clk_realview_operations = {
-	.get_rate = clk_realview_get_rate,
-	.round_rate = clk_realview_round_rate,
-	.set_rate = clk_realview_set_rate,
-};
diff --git a/arch/arm/mach-realview/clock.h b/arch/arm/mach-realview/clock.h
deleted file mode 100644
index 639a381..0000000
--- a/arch/arm/mach-realview/clock.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- *  linux/arch/arm/mach-realview/clock.h
- *
- *  Copyright (C) 2004 ARM Limited.
- *  Written by Deep Blue Solutions Limited.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/clk.h>
-
-struct clk_realview {
-	struct clk			clk;
-	unsigned long			rate;
-	const struct icst307_params	params;
-	void				(*setvco)(struct clk_realview *,
-						  struct icst307_vco);
-};
-
-extern struct clk_operations clk_realview_operations;
diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index 8102c75..4e1c87f 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -52,7 +52,6 @@
 #include <mach/irqs.h>
 
 #include "core.h"
-#include "clock.h"
 
 #define REALVIEW_REFCOUNTER	(__io_address(REALVIEW_SYS_BASE) + REALVIEW_SYS_24MHz_OFFSET)
 
@@ -274,7 +273,7 @@ struct mmci_platform_data realview_mmc1_plat_data = {
  * Clock handling
  */
 
-static void realview_oscvco_set(struct clk_realview *clk, struct icst307_vco vco)
+static void realview_oscvco_set(struct clk_icst307 *clk, struct icst307_vco vco)
 {
 	void __iomem *sys_lock = __io_address(REALVIEW_SYS_BASE) + REALVIEW_SYS_LOCK_OFFSET;
 	void __iomem *sys_osc;
@@ -293,9 +292,9 @@ static void realview_oscvco_set(struct clk_realview *clk, struct icst307_vco vco
 	writel(0, sys_lock);
 }
 
-static struct clk_realview oscvco_clk = {
+static struct clk_icst307 oscvco_clk = {
 	.clk = {
-		.ops = &clk_realview_operations,
+		.ops = &clk_icst307_operations,
 	},
 	.params	= {
 		.ref		= 24000,
diff --git a/arch/arm/mach-realview/realview_pba8.c b/arch/arm/mach-realview/realview_pba8.c
index fe861e9..0b43c07 100644
--- a/arch/arm/mach-realview/realview_pba8.c
+++ b/arch/arm/mach-realview/realview_pba8.c
@@ -42,7 +42,6 @@
 #include <mach/irqs.h>
 
 #include "core.h"
-#include "clock.h"
 
 static struct map_desc realview_pba8_io_desc[] __initdata = {
 	{
diff --git a/arch/arm/mach-versatile/Makefile b/arch/arm/mach-versatile/Makefile
index ba81e70..97cf4d8 100644
--- a/arch/arm/mach-versatile/Makefile
+++ b/arch/arm/mach-versatile/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the linux kernel.
 #
 
-obj-y					:= core.o clock.o
+obj-y					:= core.o
 obj-$(CONFIG_ARCH_VERSATILE_PB)		+= versatile_pb.o
 obj-$(CONFIG_MACH_VERSATILE_AB)		+= versatile_ab.o
 obj-$(CONFIG_PCI)			+= pci.o
diff --git a/arch/arm/mach-versatile/clock.c b/arch/arm/mach-versatile/clock.c
deleted file mode 100644
index 16ab90b..0000000
--- a/arch/arm/mach-versatile/clock.c
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- *  linux/arch/arm/mach-versatile/clock.c
- *
- *  Copyright (C) 2004 ARM Limited.
- *  Written by Deep Blue Solutions Limited.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/device.h>
-#include <linux/list.h>
-#include <linux/errno.h>
-#include <linux/err.h>
-#include <linux/string.h>
-#include <linux/clk.h>
-#include <linux/mutex.h>
-#include <linux/clk.h>
-
-#include <asm/hardware/icst307.h>
-
-#include "clock.h"
-
-#define to_clk_versatile(clk) (container_of(clk, struct clk_versatile, clk))
-
-static unsigned long clk_versatile_get_rate(struct clk *clk)
-{
-	return to_clk_versatile(clk)->rate;
-}
-
-static long clk_versatile_round_rate(struct clk *clk, unsigned long rate)
-{
-	const struct icst307_params *params = &to_clk_versatile(clk)->params;
-	struct icst307_vco vco;
-
-	vco = icst307_khz_to_vco(params, rate / 1000);
-	return icst307_khz(params, vco) * 1000;
-}
-
-static int clk_versatile_set_rate(struct clk *clk, unsigned long rate)
-{
-	struct clk_versatile *v_clk = to_clk_versatile(clk);
-	struct icst307_vco vco;
-
-	vco = icst307_khz_to_vco(&v_clk->params, rate / 1000);
-	v_clk->rate = icst307_khz(&v_clk->params, vco) * 1000;
-	v_clk->setvco(v_clk, vco);
-
-	return 0;
-}
-
-struct clk_operations clk_versatile_operations = {
-	.get_rate = clk_versatile_get_rate,
-	.round_rate = clk_versatile_round_rate,
-	.set_rate = clk_versatile_set_rate,
-};
diff --git a/arch/arm/mach-versatile/clock.h b/arch/arm/mach-versatile/clock.h
deleted file mode 100644
index 6ac30c5..0000000
--- a/arch/arm/mach-versatile/clock.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- *  linux/arch/arm/mach-versatile/clock.h
- *
- *  Copyright (C) 2004 ARM Limited.
- *  Written by Deep Blue Solutions Limited.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/clk.h>
-
-struct clk_versatile {
-	struct clk			clk;
-	unsigned long			rate;
-	const struct icst307_params	params;
-	void				(*setvco)(struct clk_versatile *,
-						  struct icst307_vco);
-};
-
-extern struct clk_operations clk_versatile_operations;
-
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index 2b670bb..e2323d8 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -50,7 +50,6 @@
 #include <asm/mach/map.h>
 
 #include "core.h"
-#include "clock.h"
 
 /*
  * All IO addresses are mapped onto VA 0xFFFx.xxxx, where x.xxxx
@@ -380,7 +379,7 @@ static struct mmci_platform_data mmc0_plat_data = {
  * Clock handling
  */
 
-static void versatile_oscvco_set(struct clk_versatile *clk, struct icst307_vco vco)
+static void versatile_oscvco_set(struct clk_icst307 *clk, struct icst307_vco vco)
 {
 	void __iomem *sys = __io_address(VERSATILE_SYS_BASE);
 	void __iomem *sys_lock = sys + VERSATILE_SYS_LOCK_OFFSET;
@@ -395,9 +394,9 @@ static void versatile_oscvco_set(struct clk_versatile *clk, struct icst307_vco v
 	writel(0, sys_lock);
 }
 
-static struct clk_versatile osc4_clk = {
+static struct clk_icst307 osc4_clk = {
 	.clk = {
-		.ops = &clk_versatile_operations,
+		.ops = &clk_icst307_operations,
 	},
 	.params = {
 		.ref		= 24000,

Re: [RFC,PATCH 0/7 v2] Common struct clk implementation

[Russell King - ARM Linux]

On Tue, Jan 12, 2010 at 05:58:31PM +1100, Jeremy Kerr wrote:
> The first two patches are for the architecture-independent kernel code,
> introducing the common clk API. The remaining patches are specific to
> the ARM 'versatile' and 'realview' platforms.

You're still only touching the "easy" platforms (as I pointed out
previously).  This is no real test of the new implementation.  To make
a decision on this based upon the easy implementations would be
completely wreckless.

As I said previously:

> Having struct clk be a set of function pointers gets really expensive on
> some platforms due to the shere number of struct clks - about 900; this
> will be a bar to them adopting this not only due to the size but the
> problems of ensuring correct initialisation.
> 
> Conversely, having those platforms use a pointer to a set of clk operations
> structures is also prohibitive - some operations such as enable and disable
> can be common, but the rest are extremely variable.

The 900 figure was the result of a bad grep - it's more around 220
for one OMAP CPU - if you include all OMAP CPUs which share the same
implementation then its around 600 clk structures that need to be
changed.

But the point I was trying to convey is that OMAP doesn't work with
_either_ a pure operations struct _or_ a bunch of per-clock function
pointers - it currently uses a mixture of the two.

Maybe this is because there was no proper classing of clocks (to
separate clock masking from clock muxing from PLLs, etc.)  The result
of this is that with a pure operations struct, you're likely to end
up with as many operations structures as there are clocks on OMAP.

The answer "OMAP shouldn't use this then" is not one I want to hear;
as I've already pointed out, OMAP is one of the platforms which should
use it.

Re: [RFC,PATCH 0/7 v2] Common struct clk implementation

[Jeremy Kerr]

Hi Russell,

> But the point I was trying to convey is that OMAP doesn't work with
> _either_ a pure operations struct _or_ a bunch of per-clock function
> pointers - it currently uses a mixture of the two.

With the common clk, you can do exactly that:

struct clk_foo {
	/* ->ops provides functions common to clk_foo */
	struct clk;

	/* provides a function for only this clock */
	int (*single_clock_func)(struct clk_foo *);
}

The only real difference is that the public API is provided through struct =
clk=20
rather than redefined clk_* functions; whatever is implementing the clock-
type-specific struct clk can add whatever fields necessary.

=46rom your earlier mail about sizes on omap:

> There are two function pointers in the struct clk which would be
> identical to the versions proposed in this generic struct clk.
> There's a total of 219 clk structures in OMAP3.  So, 219 * (4 + 8)
> =3D 2628.  Switching OMAP means 219 * (4 + 32) =3D 7884, which is an
> increase in overhead of 3x.

But we also can reduce the size of the struct clk in most cases; I believe =
the=20
separate clk_operations in v2 of this series will help with this.

Taking OMAP3 for example (I'm not very familiar with that platform, so am=20
basing this on a brief look through the clock code), the first step to a=20
common clk port would be to wrap the existing struct clk:

struct clk_omap {
	struct clk			clk;
	struct list_head		node;
	const struct clkops	*ops;
    [...]
};

and define the clk_operations to be the current omap clk_* functions.

This results in one extra pointer per clock, plus the clk_operations, so 90=
8=20
bytes (4 * 219 + 32) overhead.

Next, we can start removing fields that are not used by all clocks; the fix=
ed=20
top-level clocks would be a good start; it looks like we can represent thos=
e=20
with a:

struct clk_omap_fixed {
	struct clk	clk;
	char			*name;
	unsigned long	rate;
}

[For these fixed clocks, we don't need to propagate changes to children, he=
nce=20
I'm assuming no child/sibling members]

The original struct clk is 96 bytes; clk_omap_fixed is 12, but we still nee=
d=20
one clk_operations (32 bytes). Since there are 8 of these, we save 640 byte=
s=20
((96 - 12) * 8 - 32).

If we then take the 'follow parent' clocks, it looks like we can represent=
=20
those with something like:

struct clk_omap_followparent =3D {
	struct clk	clk;
	char			*name;
	struct clk	*parent;
	struct list_head children, siblings;
	unsigned long	rate;
	void __iomem	*enable_reg;
	__u8			enable_bit;
	char			*clkdm_name;
	int			flags;
};

This would be 48 bytes, there are 140 of these, saving 6688 bytes ( (96 - 4=
8)=20
* 140 - 32).

Now, we could stop here, or keep looking for common usage patterns of struc=
t=20
clk to find cases where creating another clock type makes sense.

I know I've only looked at the easy OMAP cases here, but the principle stil=
l=20
applies: keep the original struct clk around as a fallback, but use the=20
smaller struct clks where possible.

Cheers,


Jeremy