[PATCH 0/3] clock (clk.h) framework for mpc52xx

[PATCH 0/3] clock (clk.h) framework for mpc52xx

[Domen Puncer]

Hi!

Clock managing in mpc52xx psc spi driver grew a bit, to do it
"the right way" ;-)

1/3 - Generic powerpc clock interface, platforms that implement
      it should fill clk_functions.
      This is needed because powerpc supports multiple platforms
      in same kernel image.
2/3 - psc spi clocks (mclk) for mpc52xx
3/3 - Use the spi clocks in mpc52xx psc spi driver.


Thanks to David Brownell and Sylvain Munaut for feedback and
help on patches.


	Domen

[PATCH 1/3] powerpc clk.h interface for platforms

[Domen Puncer]

clk interface for arch/powerpc, platforms should fill
clk_functions.

Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 arch/powerpc/kernel/Makefile        |    3 -
 arch/powerpc/kernel/clock.c         |   82 ++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/clk_interface.h |   20 ++++++++
 3 files changed, 104 insertions(+), 1 deletion(-)

Index: work-powerpc.git/arch/powerpc/kernel/clock.c
===================================================================
--- /dev/null
+++ work-powerpc.git/arch/powerpc/kernel/clock.c
@@ -0,0 +1,82 @@
+/*
+ * Dummy clk implementations for powerpc.
+ * These need to be overridden in platform code.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <asm/clk_interface.h>
+
+struct clk_interface clk_functions;
+
+struct clk *clk_get(struct device *dev, const char *id)
+{
+	if (clk_functions.clk_get)
+		return clk_functions.clk_get(dev, id);
+	return ERR_PTR(-ENOSYS);
+}
+EXPORT_SYMBOL(clk_get);
+
+void clk_put(struct clk *clk)
+{
+	if (clk_functions.clk_put)
+		clk_functions.clk_put(clk);
+}
+EXPORT_SYMBOL(clk_put);
+
+int clk_enable(struct clk *clk)
+{
+	if (clk_functions.clk_enable)
+		return clk_functions.clk_enable(clk);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_enable);
+
+void clk_disable(struct clk *clk)
+{
+	if (clk_functions.clk_disable)
+		clk_functions.clk_disable(clk);
+}
+EXPORT_SYMBOL(clk_disable);
+
+unsigned long clk_get_rate(struct clk *clk)
+{
+	if (clk_functions.clk_get_rate)
+		return clk_functions.clk_get_rate(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_get_rate);
+
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk_functions.clk_round_rate)
+		return clk_functions.clk_round_rate(clk, rate);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk_functions.clk_set_rate)
+		return clk_functions.clk_set_rate(clk, rate);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_set_rate);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+	if (clk_functions.clk_get_parent)
+		return clk_functions.clk_get_parent(clk);
+	return ERR_PTR(-ENOSYS);
+}
+EXPORT_SYMBOL(clk_get_parent);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	if (clk_functions.clk_set_parent)
+		return clk_functions.clk_set_parent(clk, parent);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_set_parent);
Index: work-powerpc.git/include/asm-powerpc/clk_interface.h
===================================================================
--- /dev/null
+++ work-powerpc.git/include/asm-powerpc/clk_interface.h
@@ -0,0 +1,20 @@
+#ifndef __ASM_POWERPC_CLK_INTERFACE_H
+#define __ASM_POWERPC_CLK_INTERFACE_H
+
+#include <linux/clk.h>
+
+struct clk_interface {
+	struct clk*	(*clk_get)	(struct device *dev, const char *id);
+	int		(*clk_enable)	(struct clk *clk);
+	void		(*clk_disable)	(struct clk *clk);
+	unsigned long	(*clk_get_rate)	(struct clk *clk);
+	void		(*clk_put)	(struct clk *clk);
+	long		(*clk_round_rate) (struct clk *clk, unsigned long rate);
+	int 		(*clk_set_rate)	(struct clk *clk, unsigned long rate);
+	int		(*clk_set_parent) (struct clk *clk, struct clk *parent);
+	struct clk*	(*clk_get_parent) (struct clk *clk);
+};
+
+extern struct clk_interface clk_functions;
+
+#endif /* __ASM_POWERPC_CLK_INTERFACE_H */
Index: work-powerpc.git/arch/powerpc/kernel/Makefile
===================================================================
--- work-powerpc.git.orig/arch/powerpc/kernel/Makefile
+++ work-powerpc.git/arch/powerpc/kernel/Makefile
@@ -12,7 +12,8 @@ endif
 
 obj-y				:= semaphore.o cputable.o ptrace.o syscalls.o \
 				   irq.o align.o signal_32.o pmc.o vdso.o \
-				   init_task.o process.o systbl.o idle.o
+				   init_task.o process.o systbl.o idle.o \
+				   clock.o
 obj-y				+= vdso32/
 obj-$(CONFIG_PPC64)		+= setup_64.o binfmt_elf32.o sys_ppc32.o \
 				   signal_64.o ptrace32.o \

[PATCH 2/3] mpc52xx clk.h interface

[Domen Puncer]

clk.h interface for mpc52xx
Currently only psc_mclks are defined.


Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 arch/powerpc/platforms/52xx/Makefile         |    2 
 arch/powerpc/platforms/52xx/mpc52xx_clock.c  |  352 +++++++++++++++++++++++++++
 arch/powerpc/platforms/52xx/mpc52xx_common.c |    3 
 include/asm-powerpc/mpc52xx.h                |    2 
 4 files changed, 358 insertions(+), 1 deletion(-)

Index: work-powerpc.git/arch/powerpc/platforms/52xx/Makefile
===================================================================
--- work-powerpc.git.orig/arch/powerpc/platforms/52xx/Makefile
+++ work-powerpc.git/arch/powerpc/platforms/52xx/Makefile
@@ -2,7 +2,7 @@
 # Makefile for 52xx based boards
 #
 ifeq ($(CONFIG_PPC_MERGE),y)
-obj-y				+= mpc52xx_pic.o mpc52xx_common.o
+obj-y				+= mpc52xx_pic.o mpc52xx_common.o mpc52xx_clock.o
 obj-$(CONFIG_PCI)		+= mpc52xx_pci.o
 endif
 
Index: work-powerpc.git/arch/powerpc/platforms/52xx/mpc52xx_clock.c
===================================================================
--- /dev/null
+++ work-powerpc.git/arch/powerpc/platforms/52xx/mpc52xx_clock.c
@@ -0,0 +1,352 @@
+/*
+ * arch/powerpc/platforms/52xx/mpc52xx_clock.c
+ * based on linux/arch/arm/mach-at91/clock.c
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/errno.h>
+#include <linux/spinlock.h>
+#include <linux/clk.h>
+
+#include <asm/io.h>
+#include <asm/mpc52xx.h>
+#include <asm/clk_interface.h>
+
+
+struct clk {
+	struct list_head node;
+	const char	*name;		/* unique clock name */
+	struct device_node *of_node;	/* device node associated with this clock */
+	const char	*function;	/* clock function */
+	unsigned long	rate_hz;
+	struct clk	*parent;
+	void		(*mode)(struct clk *, int);
+	u16		users;
+	int		cdm_id;		/* == bit number in datasheet, or 0 */
+	long		(*set_rate)(struct clk *, unsigned long rate, int round_only);
+};
+
+#define CDM_ID_PSC3 24
+#define CDM_ID_PSC2 25
+#define CDM_ID_PSC1 26
+#define CDM_ID_PSC6 27
+
+static long psc_set_rate(struct clk *clk, unsigned long rate, int round_only);
+
+static struct clk clk_system = {
+	.name		= "system",
+	.rate_hz	= 528000000,	/* default if there's no system-frequency in dts */
+	.users		= 1,		/* always on */
+};
+static struct clk clk_psc1 = {
+	.name		= "psc1_mclk",
+	.cdm_id		= CDM_ID_PSC1,
+	.parent		= &clk_system,
+	.set_rate	= psc_set_rate,
+};
+static struct clk clk_psc2 = {
+	.name		= "psc2_mclk",
+	.cdm_id		= CDM_ID_PSC2,
+	.parent		= &clk_system,
+	.set_rate	= psc_set_rate,
+};
+static struct clk clk_psc3 = {
+	.name		= "psc3_mclk",
+	.cdm_id		= CDM_ID_PSC3,
+	.parent		= &clk_system,
+	.set_rate	= psc_set_rate,
+};
+static struct clk clk_psc6 = {
+	.name		= "psc6_mclk",
+	.cdm_id		= CDM_ID_PSC6,
+	.parent		= &clk_system,
+	.set_rate	= psc_set_rate,
+};
+
+
+
+static LIST_HEAD(clocks);
+static DEFINE_SPINLOCK(clk_lock);
+
+static struct mpc52xx_cdm __iomem *cdm;
+static DEFINE_SPINLOCK(cdm_lock);
+
+
+static long psc_set_rate(struct clk *clk, unsigned long rate, int round_only)
+{
+	u16 mclkdiv;
+	u16 __iomem *divreg;
+
+	/* pick a divider that will get the closest clock */
+	mclkdiv = (clk->parent->rate_hz + rate/2) / rate - 1;
+
+	/* trim to closest possible. or should we return an error? */
+	mclkdiv = min(mclkdiv, (u16)0x1ff);
+	mclkdiv = max(mclkdiv, (u16)1);
+
+	rate = clk->parent->rate_hz / (mclkdiv + 1);
+	mclkdiv |= 0x8000;	/* enable (this is not clk_enable!) */
+
+	if (round_only)
+		return rate;
+
+	if (clk->cdm_id == CDM_ID_PSC1)
+		divreg = &cdm->mclken_div_psc1;
+	else if (clk->cdm_id == CDM_ID_PSC2)
+		divreg = &cdm->mclken_div_psc2;
+	else if (clk->cdm_id == CDM_ID_PSC3)
+		divreg = &cdm->mclken_div_psc3;
+	else if (clk->cdm_id == CDM_ID_PSC6)
+		divreg = &cdm->mclken_div_psc6;
+	else
+		return -ENODEV;
+
+	out_be16(divreg, mclkdiv);
+
+	return 0;
+}
+
+/* clocks cannot be de-registered no refcounting necessary */
+static struct clk *mpc52xx_clk_get(struct device *dev, const char *id)
+{
+	struct clk *clk;
+
+	list_for_each_entry(clk, &clocks, node) {
+		if (strcmp(id, clk->name) == 0)
+			return clk;
+		if (clk->function && strcmp(id, clk->function) == 0 && dev &&
+				dev->archdata.of_node == clk->of_node)
+			return clk;
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+
+static void mpc52xx_clock_associate(const char *id, struct device_node *of_node,
+		const char *function)
+{
+	struct clk *clk = mpc52xx_clk_get(NULL, id);
+
+	if (IS_ERR(clk))
+		return;
+
+	clk->function = function;
+	clk->of_node = of_node;
+}
+
+static void mpc52xx_clk_put(struct clk *clk)
+{
+}
+
+
+static void clk_mode_cdm(int cdm_id, int enabled)
+{
+	unsigned long flags;
+	u32 clk_enables;
+
+	if (cdm_id < 12 || cdm_id > 31) {
+		printk(KERN_ERR "%s: %i invalid cdm_id: %i\n", __func__, __LINE__, cdm_id);
+		return;
+	}
+
+	spin_lock_irqsave(&cdm_lock, flags);
+	clk_enables = in_be32(&cdm->clk_enables);
+	if (enabled)
+		clk_enables |= 1 << (31-cdm_id);
+	else
+		clk_enables &= ~(1 << (31-cdm_id));
+
+	out_be32(&cdm->clk_enables, clk_enables);
+	spin_unlock_irqrestore(&cdm_lock, flags);
+}
+
+static void __clk_enable(struct clk *clk)
+{
+	if (clk->parent)
+		__clk_enable(clk->parent);
+	if (clk->users++ == 0 && clk->mode) {
+		if (clk->cdm_id)
+			clk_mode_cdm(clk->cdm_id, 1);
+		else
+			clk->mode(clk, 1);
+	}
+}
+
+static int mpc52xx_clk_enable(struct clk *clk)
+{
+	unsigned long	flags;
+
+	spin_lock_irqsave(&clk_lock, flags);
+	__clk_enable(clk);
+	spin_unlock_irqrestore(&clk_lock, flags);
+	return 0;
+}
+
+static void __clk_disable(struct clk *clk)
+{
+	BUG_ON(clk->users == 0);
+	if (--clk->users == 0 && clk->mode) {
+		if (clk->cdm_id)
+			clk_mode_cdm(clk->cdm_id, 0);
+		else
+			clk->mode(clk, 0);
+	}
+	if (clk->parent)
+		__clk_disable(clk->parent);
+}
+
+static void mpc52xx_clk_disable(struct clk *clk)
+{
+	unsigned long	flags;
+
+	spin_lock_irqsave(&clk_lock, flags);
+	__clk_disable(clk);
+	spin_unlock_irqrestore(&clk_lock, flags);
+}
+
+static unsigned long mpc52xx_clk_get_rate(struct clk *clk)
+{
+	unsigned long	flags;
+	unsigned long	rate;
+
+	spin_lock_irqsave(&clk_lock, flags);
+	for (;;) {
+		rate = clk->rate_hz;
+		if (rate || !clk->parent)
+			break;
+		clk = clk->parent;
+	}
+	spin_unlock_irqrestore(&clk_lock, flags);
+	return rate;
+}
+
+static long mpc52xx_clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	unsigned long flags;
+	long ret;
+
+	if (!clk->set_rate)
+		return -EINVAL;
+
+	spin_lock_irqsave(&clk_lock, flags);
+	ret = clk->set_rate(clk, rate, 1);
+	spin_unlock_irqrestore(&clk_lock, flags);
+
+	return ret;
+}
+
+static int mpc52xx_clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	unsigned long flags;
+	int ret;
+
+	if (!clk->set_rate)
+		return -EINVAL;
+	if (clk->users)
+		return -EBUSY;
+
+	spin_lock_irqsave(&clk_lock, flags);
+	clk->rate_hz = clk->set_rate(clk, rate, 1);
+	ret = clk->set_rate(clk, rate, 0);
+	spin_unlock_irqrestore(&clk_lock, flags);
+
+	return ret;
+}
+
+static struct clk *mpc52xx_clk_get_parent(struct clk *clk)
+{
+	return clk->parent;
+}
+
+static int mpc52xx_clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	unsigned long	flags;
+
+	if (clk->users)
+		return -EBUSY;
+	spin_lock_irqsave(&clk_lock, flags);
+
+	clk->rate_hz = parent->rate_hz;
+	clk->parent = parent;
+
+	spin_unlock_irqrestore(&clk_lock, flags);
+	return 0;
+}
+
+
+
+static struct clk *const mpc5200_clocks[] __initdata = {
+	&clk_system,
+	&clk_psc1,
+	&clk_psc2,
+	&clk_psc3,
+	&clk_psc6,
+};
+
+int __init mpc52xx_clock_init(void)
+{
+	int i;
+	struct device_node *np, *child = NULL;
+	const unsigned int *fp;
+
+	/* map Clock Distribution Module registers */
+	cdm = mpc52xx_find_and_map("mpc5200-cdm");
+	if (!cdm) {
+		printk(KERN_ERR "%s: %i couldn't map mpc5200-cdm\n", __func__, __LINE__);
+		return -EFAULT;
+	}
+
+	/* register clocks */
+	for (i = 0; i < ARRAY_SIZE(mpc5200_clocks); i++)
+		list_add_tail(&mpc5200_clocks[i]->node, &clocks);
+
+
+	/* get clk_system rate from device tree */
+	np = of_find_node_by_type(NULL, "soc");
+	if (!np)
+		return 0;
+
+	fp = of_get_property(np, "system-frequency", NULL);
+	if (fp && *fp)
+		clk_system.rate_hz = *fp;
+
+	/* associate psc_mclks with device_nodes */
+	while ((child = of_get_next_child(np, child))) {
+		if (of_device_is_compatible(child, "mpc5200-psc")) {
+			char clock[10];
+			int ci = -1;
+			const int *pci;
+
+			pci = of_get_property(child, "cell-index", NULL);
+			if (pci)
+				ci = *pci;
+			if (ci < 0 || ci > 5 || ci == 3 || ci == 4) {
+				printk(KERN_ALERT "%s: %i psc node '%s' has invalid "
+						"cell-index: %i\n", __func__, __LINE__,
+						child->name, ci);
+				continue;
+			}
+
+			snprintf(clock, 10, "psc%i_mclk", ci + 1);
+			mpc52xx_clock_associate(clock, child, "psc_mclk");
+		}
+	}
+	of_node_put(np);
+
+	/* register clocks */
+	clk_functions = (struct clk_interface) {
+		.clk_get	= mpc52xx_clk_get,
+		.clk_enable	= mpc52xx_clk_enable,
+		.clk_disable	= mpc52xx_clk_disable,
+		.clk_get_rate	= mpc52xx_clk_get_rate,
+		.clk_put	= mpc52xx_clk_put,
+		.clk_round_rate	= mpc52xx_clk_round_rate,
+		.clk_set_rate	= mpc52xx_clk_set_rate,
+		.clk_set_parent	= mpc52xx_clk_set_parent,
+		.clk_get_parent	= mpc52xx_clk_get_parent,
+	};
+	return 0;
+}
Index: work-powerpc.git/arch/powerpc/platforms/52xx/mpc52xx_common.c
===================================================================
--- work-powerpc.git.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ work-powerpc.git/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -110,6 +110,9 @@ mpc52xx_setup_cpu(void)
 	    transaction and re-enable it afterwards ...) */
 	out_be32(&xlb->config, in_be32(&xlb->config) | MPC52xx_XLB_CFG_PLDIS);
 
+	/* setup clk system */
+	mpc52xx_clock_init();
+
 	/* Unmap zones */
 unmap_regs:
 	if (cdm) iounmap(cdm);
Index: work-powerpc.git/include/asm-powerpc/mpc52xx.h
===================================================================
--- work-powerpc.git.orig/include/asm-powerpc/mpc52xx.h
+++ work-powerpc.git/include/asm-powerpc/mpc52xx.h
@@ -274,5 +274,7 @@ extern char saved_sram[0x4000]; /* reuse
 #endif
 #endif /* CONFIG_PM */
 
+extern int __init mpc52xx_clock_init(void);
+
 #endif /* __ASM_POWERPC_MPC52xx_H__ */
 

[PATCH 3/3] mpc52xx_psc_spi: use clk.h subsystem

[Domen Puncer]

Use clocks subsystem in spi driver.


Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 drivers/spi/mpc52xx_psc_spi.c |   64 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 57 insertions(+), 7 deletions(-)

Index: work-powerpc.git/drivers/spi/mpc52xx_psc_spi.c
===================================================================
--- work-powerpc.git.orig/drivers/spi/mpc52xx_psc_spi.c
+++ work-powerpc.git/drivers/spi/mpc52xx_psc_spi.c
@@ -18,6 +18,7 @@
 
 #if defined(CONFIG_PPC_MERGE)
 #include <asm/of_platform.h>
+#include <linux/clk.h>
 #else
 #include <linux/platform_device.h>
 #endif
@@ -53,6 +54,8 @@ struct mpc52xx_psc_spi {
 	spinlock_t lock;
 
 	struct completion done;
+
+	struct clk *clk;
 };
 
 /* controller state */
@@ -85,6 +88,13 @@ static void mpc52xx_psc_spi_activate_cs(
 	u32 sicr;
 	u16 ccr;
 
+#ifdef CONFIG_PPC_MERGE
+	u8 bitclkdiv = 2;	/* minimum bitclkdiv */
+	int speed = cs->speed_hz ? cs->speed_hz : 1000000;
+	int mclk;
+	int system;
+#endif
+
 	sicr = in_be32(&psc->sicr);
 
 	/* Set clock phase and polarity */
@@ -106,13 +116,39 @@ static void mpc52xx_psc_spi_activate_cs(
 	/* Set clock frequency and bits per word
 	 * Because psc->ccr is defined as 16bit register instead of 32bit
 	 * just set the lower byte of BitClkDiv
+	 * Because BitClkDiv is 8-bit on mpc5200. Lets stay compatible.
 	 */
 	ccr = in_be16(&psc->ccr);
 	ccr &= 0xFF00;
+
+#ifdef CONFIG_PPC_MERGE
+	/*
+	 * pscclk = mclk/(bitclkdiv+1))		bitclkdiv is 8-bit, >= 2
+	 * mclk = fsys/(mclkdiv+1)		mclkdiv is 9-bit, >= 1
+	 * as mclkdiv has higher precision, we want is as big as possible
+	 * => we get < 0.002*clock error
+	 */
+
+	system = clk_get_rate(clk_get_parent(mps->clk));
+	mclk = speed * (bitclkdiv+1);
+	if (system/mclk > 512) { /* bigger than mclkdiv */
+		bitclkdiv = (system/512) / speed;
+		mclk = speed * (bitclkdiv+1);
+	}
+
+	if (clk_set_rate(mps->clk, mclk))
+		dev_err(&spi->dev, "couldn't set psc_mclk's rate\n");
+
+	dev_info(&spi->dev, "clock: wanted: %i, got: %li\n", speed,
+			clk_round_rate(mps->clk, mclk) / (bitclkdiv+1));
+
+	ccr |= bitclkdiv;
+#else
 	if (cs->speed_hz)
 		ccr |= (MCLK / cs->speed_hz - 1) & 0xFF;
 	else /* by default SPI Clk 1MHz */
 		ccr |= (MCLK / 1000000 - 1) & 0xFF;
+#endif
 	out_be16(&psc->ccr, ccr);
 	mps->bits_per_word = cs->bits_per_word;
 
@@ -321,20 +357,17 @@ static void mpc52xx_psc_spi_cleanup(stru
 
 static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
 {
+	struct mpc52xx_psc __iomem *psc = mps->psc;
+	int ret = 0;
+
+#if !defined(CONFIG_PPC_MERGE)
 	struct mpc52xx_cdm __iomem *cdm;
 	struct mpc52xx_gpio __iomem *gpio;
-	struct mpc52xx_psc __iomem *psc = mps->psc;
 	u32 ul;
 	u32 mclken_div;
-	int ret = 0;
 
-#if defined(CONFIG_PPC_MERGE)
-	cdm = mpc52xx_find_and_map("mpc5200-cdm");
-	gpio = mpc52xx_find_and_map("mpc5200-gpio");
-#else
 	cdm = ioremap(MPC52xx_PA(MPC52xx_CDM_OFFSET), MPC52xx_CDM_SIZE);
 	gpio = ioremap(MPC52xx_PA(MPC52xx_GPIO_OFFSET), MPC52xx_GPIO_SIZE);
-#endif
 	if (!cdm || !gpio) {
 		printk(KERN_ERR "Error mapping CDM/GPIO\n");
 		ret = -EFAULT;
@@ -390,6 +423,7 @@ static int mpc52xx_psc_spi_port_config(i
 		ret = -EINVAL;
 		goto unmap_regs;
 	}
+#endif
 
 	/* Reset the PSC into a known state */
 	out_8(&psc->command, MPC52xx_PSC_RST_RX);
@@ -413,11 +447,13 @@ static int mpc52xx_psc_spi_port_config(i
 
 	mps->bits_per_word = 8;
 
+#if !defined(CONFIG_PPC_MERGE)
 unmap_regs:
 	if (cdm)
 		iounmap(cdm);
 	if (gpio)
 		iounmap(gpio);
+#endif
 
 	return ret;
 }
@@ -502,11 +538,22 @@ static int __init mpc52xx_psc_spi_do_pro
 
 	ret = spi_register_master(master);
 	if (ret < 0)
+		goto destr_wq;
+
+#ifdef CONFIG_PPC_MERGE
+	mps->clk = clk_get(dev, "psc_mclk");
+	if (IS_ERR(mps->clk)) {
+		dev_err(dev, "couldn't get psc_mclk clock\n");
+		ret = -ENOENT;
 		goto unreg_master;
+	}
+#endif
 
 	return ret;
 
 unreg_master:
+	spi_unregister_master(master);
+destr_wq:
 	destroy_workqueue(mps->workqueue);
 free_irq:
 	free_irq(mps->irq, mps);
@@ -523,6 +570,9 @@ static int __exit mpc52xx_psc_spi_do_rem
 	struct spi_master *master = dev_get_drvdata(dev);
 	struct mpc52xx_psc_spi *mps = spi_master_get_devdata(master);
 
+#ifdef CONFIG_PPC_MERGE
+	clk_put(mps->clk);
+#endif
 	flush_workqueue(mps->workqueue);
 	destroy_workqueue(mps->workqueue);
 	spi_unregister_master(master);

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Christoph Hellwig]

On Wed, Jul 11, 2007 at 11:32:20AM +0200, Domen Puncer wrote:
> clk interface for arch/powerpc, platforms should fill
> clk_functions.

Umm, this is about the fifth almost identical implementation of
the clk_ functions.  Please, please put it into common code.

And talk to the mips folks which just got a similar comment from me.

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[David Brownell]

On Wednesday 11 July 2007, Christoph Hellwig wrote:
> On Wed, Jul 11, 2007 at 11:32:20AM +0200, Domen Puncer wrote:
> > clk interface for arch/powerpc, platforms should fill
> > clk_functions.
> 
> Umm, this is about the fifth almost identical implementation of
> the clk_ functions.  Please, please put it into common code.
> 
> And talk to the mips folks which just got a similar comment from me.

You mean like a lib/clock.c core, rather than an opsvector?

ISTR that allowing custom platform-specific implementations
was intended to be a feature.  But it's also true that some
folks see lack of shared implementation code as a drawback;
so I've CC'd Russell King (who originated the interface for
ARM platforms).

- Dave

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Christoph Hellwig]

On Wed, Jul 11, 2007 at 08:56:58AM -0700, David Brownell wrote:
> > Umm, this is about the fifth almost identical implementation of
> > the clk_ functions.  Please, please put it into common code.
> > 
> > And talk to the mips folks which just got a similar comment from me.
> 
> You mean like a lib/clock.c core, rather than an opsvector?

I mean an ops vector and surrounding wrappers.  Every architecture
is reimplementing their own dispatch table which is rather annoying.

What would a lib/clock.c do?

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[David Brownell]

On Wednesday 11 July 2007, Christoph Hellwig wrote:
> On Wed, Jul 11, 2007 at 08:56:58AM -0700, David Brownell wrote:
> > > Umm, this is about the fifth almost identical implementation of
> > > the clk_ functions.  Please, please put it into common code.
> > > 
> > > And talk to the mips folks which just got a similar comment from me.
> > 
> > You mean like a lib/clock.c core, rather than an opsvector?
> 
> I mean an ops vector and surrounding wrappers.  Every architecture
> is reimplementing their own dispatch table which is rather annoying.

ARM doesn't.  :)

But then, nobody expects one kernel to support more than one
vendor's ARM chips; or usually, more than one generation of
that vendor's chips.  So any dispatch table is specific to
a given platform, and tuned to its quirks.  Not much to share
between OMAP and AT91, for example, except in some cases maybe
an arm926ejs block.


> What would a lib/clock.c do?

Some folk have suggested defining a core "struct clk {...}" with
some of the basics -- refcount, parent, maybe enough to support
the clk_get() lookup or even more -- so that the more obvious
stuff doesn't need constant re-implementation, and so that new
implementations become easier.  Platforms would wrap that with
whatever extensions they need.

I've not seen a solid proposal for such a thing, and it's not
clear to me how that would play with with older code (e.g. any
of the ARM implementations).

And I'm sure there are other suggestions ... I was mostly just
wondering just what you were suggesting.

- Dave

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Russell King]

On Wed, Jul 11, 2007 at 10:02:54AM -0700, David Brownell wrote:
> On Wednesday 11 July 2007, Christoph Hellwig wrote:
> > On Wed, Jul 11, 2007 at 08:56:58AM -0700, David Brownell wrote:
> > > > Umm, this is about the fifth almost identical implementation of
> > > > the clk_ functions.  Please, please put it into common code.
> > > > 
> > > > And talk to the mips folks which just got a similar comment from me.
> > > 
> > > You mean like a lib/clock.c core, rather than an opsvector?
> > 
> > I mean an ops vector and surrounding wrappers.  Every architecture
> > is reimplementing their own dispatch table which is rather annoying.
> 
> ARM doesn't.  :)
> 
> But then, nobody expects one kernel to support more than one
> vendor's ARM chips; or usually, more than one generation of
> that vendor's chips.  So any dispatch table is specific to
> a given platform, and tuned to its quirks.  Not much to share
> between OMAP and AT91, for example, except in some cases maybe
> an arm926ejs block.

And also the information stored within a 'struct clk' is very platform
dependent.  In the most basic situation, 'struct clk' may not actually
be a structure, but the clock rate.  All functions with the exception of
clk_get() and clk_get_rate() could well be no-ops, clk_get() just returns
the 'struct clk' representing the rate and 'clk_get_rate' returns that
as an integer.

More complex setups might want 'struct clk' to contain the address of a
clock enable register, the bit position to enable that clock source, the
clock rate, a refcount, and so on, all of which would be utterly useless
for a platform which had fixed rate clocks.

> I've not seen a solid proposal for such a thing, and it's not
> clear to me how that would play with with older code (e.g. any
> of the ARM implementations).

If people are implementing their own incompatible changes without reference
to the API they're invalid implementations as far as I'm concerned.  If
they can't bothered to lift a finger to even _talk_ to me about their
requirements they just don't have any say concerning any future
developments IMO.

IOW, talk to me and I'll talk back.  Ignore me and I'll ignore them.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[David Brownell]

On Wednesday 11 July 2007, Russell King wrote:
> 
> IOW, talk to me and I'll talk back.  Ignore me and I'll ignore them.

Exactly why I cc'd you ... if folk want any more sharable
cross-platform code than just the clk.h interface, I expect
you'll have useful comments.

- Dave

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Domen Puncer]

On 11/07/07 21:34 +0100, Russell King wrote:
> On Wed, Jul 11, 2007 at 10:02:54AM -0700, David Brownell wrote:
> > On Wednesday 11 July 2007, Christoph Hellwig wrote:
> > > On Wed, Jul 11, 2007 at 08:56:58AM -0700, David Brownell wrote:
> > > > > Umm, this is about the fifth almost identical implementation of
> > > > > the clk_ functions.  Please, please put it into common code.
> > > > > 
> > > > > And talk to the mips folks which just got a similar comment from me.
> > > > 
> > > > You mean like a lib/clock.c core, rather than an opsvector?
> > > 
> > > I mean an ops vector and surrounding wrappers.  Every architecture
> > > is reimplementing their own dispatch table which is rather annoying.
> > 
> > ARM doesn't.  :)
> > 
> > But then, nobody expects one kernel to support more than one
> > vendor's ARM chips; or usually, more than one generation of
> > that vendor's chips.  So any dispatch table is specific to
> > a given platform, and tuned to its quirks.  Not much to share
> > between OMAP and AT91, for example, except in some cases maybe
> > an arm926ejs block.
> 
> And also the information stored within a 'struct clk' is very platform
> dependent.  In the most basic situation, 'struct clk' may not actually
> be a structure, but the clock rate.  All functions with the exception of
> clk_get() and clk_get_rate() could well be no-ops, clk_get() just returns
> the 'struct clk' representing the rate and 'clk_get_rate' returns that
> as an integer.
> 
> More complex setups might want 'struct clk' to contain the address of a
> clock enable register, the bit position to enable that clock source, the
> clock rate, a refcount, and so on, all of which would be utterly useless
> for a platform which had fixed rate clocks.

The patch that triggered this discussion is at the end.
It doesn't make any assumption on struct clk, it's just a
wrapper around functions from clk.h.
Point of this patch was to abstract exported functions, since
arch/powerpc/ can support multiple platfroms in one binary.

> 
> > I've not seen a solid proposal for such a thing, and it's not
> > clear to me how that would play with with older code (e.g. any
> > of the ARM implementations).
> 
> If people are implementing their own incompatible changes without reference
> to the API they're invalid implementations as far as I'm concerned.  If
> they can't bothered to lift a finger to even _talk_ to me about their
> requirements they just don't have any say concerning any future
> developments IMO.

My changes were implemented according to clk.h API.
And honestly, I just wanted to rework clocks in some SPI driver and
others made me do all the clk work. ;-)


	Domen

> 
> IOW, talk to me and I'll talk back.  Ignore me and I'll ignore them.
> 
> -- 
> Russell King
>  Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:




clk interface for arch/powerpc, platforms should fill
clk_functions.

Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 arch/powerpc/kernel/Makefile        |    3 -
 arch/powerpc/kernel/clock.c         |   82 ++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/clk_interface.h |   20 ++++++++
 3 files changed, 104 insertions(+), 1 deletion(-)

Index: work-powerpc.git/arch/powerpc/kernel/clock.c
===================================================================
--- /dev/null
+++ work-powerpc.git/arch/powerpc/kernel/clock.c
@@ -0,0 +1,82 @@
+/*
+ * Dummy clk implementations for powerpc.
+ * These need to be overridden in platform code.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <asm/clk_interface.h>
+
+struct clk_interface clk_functions;
+
+struct clk *clk_get(struct device *dev, const char *id)
+{
+	if (clk_functions.clk_get)
+		return clk_functions.clk_get(dev, id);
+	return ERR_PTR(-ENOSYS);
+}
+EXPORT_SYMBOL(clk_get);
+
+void clk_put(struct clk *clk)
+{
+	if (clk_functions.clk_put)
+		clk_functions.clk_put(clk);
+}
+EXPORT_SYMBOL(clk_put);
+
+int clk_enable(struct clk *clk)
+{
+	if (clk_functions.clk_enable)
+		return clk_functions.clk_enable(clk);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_enable);
+
+void clk_disable(struct clk *clk)
+{
+	if (clk_functions.clk_disable)
+		clk_functions.clk_disable(clk);
+}
+EXPORT_SYMBOL(clk_disable);
+
+unsigned long clk_get_rate(struct clk *clk)
+{
+	if (clk_functions.clk_get_rate)
+		return clk_functions.clk_get_rate(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_get_rate);
+
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk_functions.clk_round_rate)
+		return clk_functions.clk_round_rate(clk, rate);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk_functions.clk_set_rate)
+		return clk_functions.clk_set_rate(clk, rate);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_set_rate);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+	if (clk_functions.clk_get_parent)
+		return clk_functions.clk_get_parent(clk);
+	return ERR_PTR(-ENOSYS);
+}
+EXPORT_SYMBOL(clk_get_parent);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	if (clk_functions.clk_set_parent)
+		return clk_functions.clk_set_parent(clk, parent);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_set_parent);
Index: work-powerpc.git/include/asm-powerpc/clk_interface.h
===================================================================
--- /dev/null
+++ work-powerpc.git/include/asm-powerpc/clk_interface.h
@@ -0,0 +1,20 @@
+#ifndef __ASM_POWERPC_CLK_INTERFACE_H
+#define __ASM_POWERPC_CLK_INTERFACE_H
+
+#include <linux/clk.h>
+
+struct clk_interface {
+	struct clk*	(*clk_get)	(struct device *dev, const char *id);
+	int		(*clk_enable)	(struct clk *clk);
+	void		(*clk_disable)	(struct clk *clk);
+	unsigned long	(*clk_get_rate)	(struct clk *clk);
+	void		(*clk_put)	(struct clk *clk);
+	long		(*clk_round_rate) (struct clk *clk, unsigned long rate);
+	int 		(*clk_set_rate)	(struct clk *clk, unsigned long rate);
+	int		(*clk_set_parent) (struct clk *clk, struct clk *parent);
+	struct clk*	(*clk_get_parent) (struct clk *clk);
+};
+
+extern struct clk_interface clk_functions;
+
+#endif /* __ASM_POWERPC_CLK_INTERFACE_H */
Index: work-powerpc.git/arch/powerpc/kernel/Makefile
===================================================================
--- work-powerpc.git.orig/arch/powerpc/kernel/Makefile
+++ work-powerpc.git/arch/powerpc/kernel/Makefile
@@ -12,7 +12,8 @@ endif
 
 obj-y				:= semaphore.o cputable.o ptrace.o syscalls.o \
 				   irq.o align.o signal_32.o pmc.o vdso.o \
-				   init_task.o process.o systbl.o idle.o
+				   init_task.o process.o systbl.o idle.o \
+				   clock.o
 obj-y				+= vdso32/
 obj-$(CONFIG_PPC64)		+= setup_64.o binfmt_elf32.o sys_ppc32.o \
 				   signal_64.o ptrace32.o \

Generic clk.h wrappers? [Was: Re: [PATCH 1/3] powerpc clk.h interface for platforms]

[Domen Puncer]

On 13/07/07 11:12 +0200, Domen Puncer wrote:
> On 11/07/07 21:34 +0100, Russell King wrote:
> > On Wed, Jul 11, 2007 at 10:02:54AM -0700, David Brownell wrote:
> > > On Wednesday 11 July 2007, Christoph Hellwig wrote:
> > > > On Wed, Jul 11, 2007 at 08:56:58AM -0700, David Brownell wrote:
> > > > > > Umm, this is about the fifth almost identical implementation of
> > > > > > the clk_ functions.  Please, please put it into common code.
> > > > > > 
> > > > > > And talk to the mips folks which just got a similar comment from me.
> > > > > 
> > > > > You mean like a lib/clock.c core, rather than an opsvector?
> > > > 
> > > > I mean an ops vector and surrounding wrappers.  Every architecture
> > > > is reimplementing their own dispatch table which is rather annoying.
> > > 
> > > ARM doesn't.  :)
> > > 
> > > But then, nobody expects one kernel to support more than one
> > > vendor's ARM chips; or usually, more than one generation of
> > > that vendor's chips.  So any dispatch table is specific to
> > > a given platform, and tuned to its quirks.  Not much to share
> > > between OMAP and AT91, for example, except in some cases maybe
> > > an arm926ejs block.
> > 
> > And also the information stored within a 'struct clk' is very platform
> > dependent.  In the most basic situation, 'struct clk' may not actually
> > be a structure, but the clock rate.  All functions with the exception of
> > clk_get() and clk_get_rate() could well be no-ops, clk_get() just returns
> > the 'struct clk' representing the rate and 'clk_get_rate' returns that
> > as an integer.
> > 
> > More complex setups might want 'struct clk' to contain the address of a
> > clock enable register, the bit position to enable that clock source, the
> > clock rate, a refcount, and so on, all of which would be utterly useless
> > for a platform which had fixed rate clocks.
> 
> The patch that triggered this discussion is at the end.
> It doesn't make any assumption on struct clk, it's just a
> wrapper around functions from clk.h.
> Point of this patch was to abstract exported functions, since
> arch/powerpc/ can support multiple platfroms in one binary.

So... the thread just ended without any consensus, ACK or whatever.

Choices I see:
- have EXPORT_SYMBOL for clk.h functions in ie. lib/clock.c and have
  every implemenation fill some global struct.
- leave this patch as it is, abstraction only for arch/powerpc/.
- or I can just forget about this, and leave it for the next sucker
  who will want nicer clock handling in some driver.


	Domen

Re: Generic clk.h wrappers? [Was: Re: [PATCH 1/3] powerpc clk.h interface for platforms]

[Christoph Hellwig]

On Wed, Aug 01, 2007 at 09:28:07AM +0200, Domen Puncer wrote:
> > It doesn't make any assumption on struct clk, it's just a
> > wrapper around functions from clk.h.
> > Point of this patch was to abstract exported functions, since
> > arch/powerpc/ can support multiple platfroms in one binary.
> 
> So... the thread just ended without any consensus, ACK or whatever.
> 
> Choices I see:
> - have EXPORT_SYMBOL for clk.h functions in ie. lib/clock.c and have
>   every implemenation fill some global struct.
> - leave this patch as it is, abstraction only for arch/powerpc/.
> - or I can just forget about this, and leave it for the next sucker
>   who will want nicer clock handling in some driver

It seems like arm really wants this optimized to the last cycle
and no abstraction inbetween so we're probably stuck with the status
quo.   I'm pretty sure this will get too messy sooner and later and
people will clean the mess up, but due to the political issues I
don't think it's fair to put that burden on you just for submitting
the powerpc implementation.

So, please leave the patch as-is.

Re: Generic clk.h wrappers? [Was: Re: [PATCH 1/3] powerpc clk.h interface for platforms]

[David Brownell]

On Wednesday 01 August 2007, Christoph Hellwig wrote:
> On Wed, Aug 01, 2007 at 09:28:07AM +0200, Domen Puncer wrote:
> > > It doesn't make any assumption on struct clk, it's just a
> > > wrapper around functions from clk.h.
> > > Point of this patch was to abstract exported functions, since
> > > arch/powerpc/ can support multiple platfroms in one binary.
> > 
> > So... the thread just ended without any consensus, ACK or whatever.
> > 
> > Choices I see:
> > - have EXPORT_SYMBOL for clk.h functions in ie. lib/clock.c and have
> >   every implemenation fill some global struct.
> > - leave this patch as it is, abstraction only for arch/powerpc/.

That seems the best solution for now, I agree.


> > - or I can just forget about this, and leave it for the next sucker
> >   who will want nicer clock handling in some driver
> 
> It seems like arm really wants this optimized to the last cycle
> and no abstraction inbetween so we're probably stuck with the status
> quo.   I'm pretty sure this will get too messy sooner and later and
> people will clean the mess up, but due to the political issues I
> don't think it's fair to put that burden on you just for submitting
> the powerpc implementation.
> 
> So, please leave the patch as-is.
> 

Re: Generic clk.h wrappers? [Was: Re: [PATCH 1/3] powerpc clk.h interface for platforms]

[Russell King]

On Thu, Aug 02, 2007 at 04:32:13PM -0700, David Brownell wrote:
> On Wednesday 01 August 2007, Christoph Hellwig wrote:
> > On Wed, Aug 01, 2007 at 09:28:07AM +0200, Domen Puncer wrote:
> > > > It doesn't make any assumption on struct clk, it's just a
> > > > wrapper around functions from clk.h.
> > > > Point of this patch was to abstract exported functions, since
> > > > arch/powerpc/ can support multiple platfroms in one binary.
> > > 
> > > So... the thread just ended without any consensus, ACK or whatever.
> > > 
> > > Choices I see:
> > > - have EXPORT_SYMBOL for clk.h functions in ie. lib/clock.c and have
> > >   every implemenation fill some global struct.
> > > - leave this patch as it is, abstraction only for arch/powerpc/.
> 
> That seems the best solution for now, I agree.

I've not been avoiding replying further to this thread in spite, it's
just that I've not had any time what so ever to look at this further.
It's very low priority for me at the moment, so it's getting zero time,
and will continue to be in that state for at least the next month and
a bit.  Sorry.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Domen Puncer]

Hi!

Paul, what do you say about this?
Sylvain suggested it could also be integrated with define_machine
interface.

If it's OK, it would be cool if it got merged in next release cycle.


	Domen

On 11/07/07 11:32 +0200, Domen Puncer wrote:
> clk interface for arch/powerpc, platforms should fill
> clk_functions.
> 
> Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
> 
> ---
>  arch/powerpc/kernel/Makefile        |    3 -
>  arch/powerpc/kernel/clock.c         |   82 ++++++++++++++++++++++++++++++++++++
>  include/asm-powerpc/clk_interface.h |   20 ++++++++
>  3 files changed, 104 insertions(+), 1 deletion(-)
> 
> Index: work-powerpc.git/arch/powerpc/kernel/clock.c
> ===================================================================
> --- /dev/null
> +++ work-powerpc.git/arch/powerpc/kernel/clock.c
> @@ -0,0 +1,82 @@
> +/*
> + * Dummy clk implementations for powerpc.
> + * These need to be overridden in platform code.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <asm/clk_interface.h>
> +
> +struct clk_interface clk_functions;
> +
> +struct clk *clk_get(struct device *dev, const char *id)
> +{
> +	if (clk_functions.clk_get)
> +		return clk_functions.clk_get(dev, id);
> +	return ERR_PTR(-ENOSYS);
> +}
> +EXPORT_SYMBOL(clk_get);
> +
> +void clk_put(struct clk *clk)
> +{
> +	if (clk_functions.clk_put)
> +		clk_functions.clk_put(clk);
> +}
> +EXPORT_SYMBOL(clk_put);
> +
> +int clk_enable(struct clk *clk)
> +{
> +	if (clk_functions.clk_enable)
> +		return clk_functions.clk_enable(clk);
> +	return -ENOSYS;
> +}
> +EXPORT_SYMBOL(clk_enable);
> +
> +void clk_disable(struct clk *clk)
> +{
> +	if (clk_functions.clk_disable)
> +		clk_functions.clk_disable(clk);
> +}
> +EXPORT_SYMBOL(clk_disable);
> +
> +unsigned long clk_get_rate(struct clk *clk)
> +{
> +	if (clk_functions.clk_get_rate)
> +		return clk_functions.clk_get_rate(clk);
> +	return 0;
> +}
> +EXPORT_SYMBOL(clk_get_rate);
> +
> +long clk_round_rate(struct clk *clk, unsigned long rate)
> +{
> +	if (clk_functions.clk_round_rate)
> +		return clk_functions.clk_round_rate(clk, rate);
> +	return -ENOSYS;
> +}
> +EXPORT_SYMBOL(clk_round_rate);
> +
> +int clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> +	if (clk_functions.clk_set_rate)
> +		return clk_functions.clk_set_rate(clk, rate);
> +	return -ENOSYS;
> +}
> +EXPORT_SYMBOL(clk_set_rate);
> +
> +struct clk *clk_get_parent(struct clk *clk)
> +{
> +	if (clk_functions.clk_get_parent)
> +		return clk_functions.clk_get_parent(clk);
> +	return ERR_PTR(-ENOSYS);
> +}
> +EXPORT_SYMBOL(clk_get_parent);
> +
> +int clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> +	if (clk_functions.clk_set_parent)
> +		return clk_functions.clk_set_parent(clk, parent);
> +	return -ENOSYS;
> +}
> +EXPORT_SYMBOL(clk_set_parent);
> Index: work-powerpc.git/include/asm-powerpc/clk_interface.h
> ===================================================================
> --- /dev/null
> +++ work-powerpc.git/include/asm-powerpc/clk_interface.h
> @@ -0,0 +1,20 @@
> +#ifndef __ASM_POWERPC_CLK_INTERFACE_H
> +#define __ASM_POWERPC_CLK_INTERFACE_H
> +
> +#include <linux/clk.h>
> +
> +struct clk_interface {
> +	struct clk*	(*clk_get)	(struct device *dev, const char *id);
> +	int		(*clk_enable)	(struct clk *clk);
> +	void		(*clk_disable)	(struct clk *clk);
> +	unsigned long	(*clk_get_rate)	(struct clk *clk);
> +	void		(*clk_put)	(struct clk *clk);
> +	long		(*clk_round_rate) (struct clk *clk, unsigned long rate);
> +	int 		(*clk_set_rate)	(struct clk *clk, unsigned long rate);
> +	int		(*clk_set_parent) (struct clk *clk, struct clk *parent);
> +	struct clk*	(*clk_get_parent) (struct clk *clk);
> +};
> +
> +extern struct clk_interface clk_functions;
> +
> +#endif /* __ASM_POWERPC_CLK_INTERFACE_H */
> Index: work-powerpc.git/arch/powerpc/kernel/Makefile
> ===================================================================
> --- work-powerpc.git.orig/arch/powerpc/kernel/Makefile
> +++ work-powerpc.git/arch/powerpc/kernel/Makefile
> @@ -12,7 +12,8 @@ endif
>  
>  obj-y				:= semaphore.o cputable.o ptrace.o syscalls.o \
>  				   irq.o align.o signal_32.o pmc.o vdso.o \
> -				   init_task.o process.o systbl.o idle.o
> +				   init_task.o process.o systbl.o idle.o \
> +				   clock.o
>  obj-y				+= vdso32/
>  obj-$(CONFIG_PPC64)		+= setup_64.o binfmt_elf32.o sys_ppc32.o \
>  				   signal_64.o ptrace32.o \
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Paul Mackerras]

Domen Puncer writes:

> Paul, what do you say about this?
> Sylvain suggested it could also be integrated with define_machine
> interface.

As it stands, your patch adds the clk_* functions on all platforms.
What platforms would actually want to use them?

Paul.

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Domen Puncer]

On 19/09/07 13:47 +1000, Paul Mackerras wrote:
> Domen Puncer writes:
> 
> > Paul, what do you say about this?
> > Sylvain suggested it could also be integrated with define_machine
> > interface.
> 
> As it stands, your patch adds the clk_* functions on all platforms.
> What platforms would actually want to use them?

52xx
Reason for adding it to all was that EXPORT_SYMBOL's would clash if
one were to add clk support for another platform.


	Domen

> 
> Paul.

Re: [PATCH 1/3] powerpc clk.h interface for platforms

[Paul Mackerras]

Domen Puncer writes:

> 52xx
> Reason for adding it to all was that EXPORT_SYMBOL's would clash if
> one were to add clk support for another platform.

What I meant was, why aren't you using a config symbol so that we
don't build it on platforms that don't need any "clk" support at all?

Paul.

[PATCH 1/3 v2] powerpc clk.h interface for platforms

[Domen Puncer]

clk interface for arch/powerpc, platforms should fill
clk_functions.

Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
On 20/09/07 15:07 +1000, Paul Mackerras wrote:
> Domen Puncer writes:
> 
> > 52xx
> > Reason for adding it to all was that EXPORT_SYMBOL's would clash if
> > one were to add clk support for another platform.
> 
> What I meant was, why aren't you using a config symbol so that we
> don't build it on platforms that don't need any "clk" support at all?

Right, doh!

> 
> Paul.


 arch/powerpc/Kconfig                |    4 +
 arch/powerpc/kernel/Makefile        |    1 
 arch/powerpc/kernel/clock.c         |   82 ++++++++++++++++++++++++++++++++++++
 arch/powerpc/platforms/52xx/Kconfig |    1 
 include/asm-powerpc/clk_interface.h |   20 ++++++++
 5 files changed, 108 insertions(+)

Index: linux.git/arch/powerpc/kernel/clock.c
===================================================================
--- /dev/null
+++ linux.git/arch/powerpc/kernel/clock.c
@@ -0,0 +1,82 @@
+/*
+ * Dummy clk implementations for powerpc.
+ * These need to be overridden in platform code.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <asm/clk_interface.h>
+
+struct clk_interface clk_functions;
+
+struct clk *clk_get(struct device *dev, const char *id)
+{
+	if (clk_functions.clk_get)
+		return clk_functions.clk_get(dev, id);
+	return ERR_PTR(-ENOSYS);
+}
+EXPORT_SYMBOL(clk_get);
+
+void clk_put(struct clk *clk)
+{
+	if (clk_functions.clk_put)
+		clk_functions.clk_put(clk);
+}
+EXPORT_SYMBOL(clk_put);
+
+int clk_enable(struct clk *clk)
+{
+	if (clk_functions.clk_enable)
+		return clk_functions.clk_enable(clk);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_enable);
+
+void clk_disable(struct clk *clk)
+{
+	if (clk_functions.clk_disable)
+		clk_functions.clk_disable(clk);
+}
+EXPORT_SYMBOL(clk_disable);
+
+unsigned long clk_get_rate(struct clk *clk)
+{
+	if (clk_functions.clk_get_rate)
+		return clk_functions.clk_get_rate(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_get_rate);
+
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk_functions.clk_round_rate)
+		return clk_functions.clk_round_rate(clk, rate);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk_functions.clk_set_rate)
+		return clk_functions.clk_set_rate(clk, rate);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_set_rate);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+	if (clk_functions.clk_get_parent)
+		return clk_functions.clk_get_parent(clk);
+	return ERR_PTR(-ENOSYS);
+}
+EXPORT_SYMBOL(clk_get_parent);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	if (clk_functions.clk_set_parent)
+		return clk_functions.clk_set_parent(clk, parent);
+	return -ENOSYS;
+}
+EXPORT_SYMBOL(clk_set_parent);
Index: linux.git/include/asm-powerpc/clk_interface.h
===================================================================
--- /dev/null
+++ linux.git/include/asm-powerpc/clk_interface.h
@@ -0,0 +1,20 @@
+#ifndef __ASM_POWERPC_CLK_INTERFACE_H
+#define __ASM_POWERPC_CLK_INTERFACE_H
+
+#include <linux/clk.h>
+
+struct clk_interface {
+	struct clk*	(*clk_get)	(struct device *dev, const char *id);
+	int		(*clk_enable)	(struct clk *clk);
+	void		(*clk_disable)	(struct clk *clk);
+	unsigned long	(*clk_get_rate)	(struct clk *clk);
+	void		(*clk_put)	(struct clk *clk);
+	long		(*clk_round_rate) (struct clk *clk, unsigned long rate);
+	int 		(*clk_set_rate)	(struct clk *clk, unsigned long rate);
+	int		(*clk_set_parent) (struct clk *clk, struct clk *parent);
+	struct clk*	(*clk_get_parent) (struct clk *clk);
+};
+
+extern struct clk_interface clk_functions;
+
+#endif /* __ASM_POWERPC_CLK_INTERFACE_H */
Index: linux.git/arch/powerpc/kernel/Makefile
===================================================================
--- linux.git.orig/arch/powerpc/kernel/Makefile
+++ linux.git/arch/powerpc/kernel/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_PPC64)		+= vdso64/
 obj-$(CONFIG_ALTIVEC)		+= vecemu.o vector.o
 obj-$(CONFIG_PPC_970_NAP)	+= idle_power4.o
 obj-$(CONFIG_PPC_OF)		+= of_device.o of_platform.o prom_parse.o
+obj-$(CONFIG_PPC_CLOCK)		+= clock.o
 procfs-$(CONFIG_PPC64)		:= proc_ppc64.o
 obj-$(CONFIG_PROC_FS)		+= $(procfs-y)
 rtaspci-$(CONFIG_PPC64)-$(CONFIG_PCI)	:= rtas_pci.o
Index: linux.git/arch/powerpc/Kconfig
===================================================================
--- linux.git.orig/arch/powerpc/Kconfig
+++ linux.git/arch/powerpc/Kconfig
@@ -664,3 +664,7 @@ config KEYS_COMPAT
 	default y
 
 source "crypto/Kconfig"
+
+config PPC_CLOCK
+	bool
+	default n
Index: linux.git/arch/powerpc/platforms/52xx/Kconfig
===================================================================
--- linux.git.orig/arch/powerpc/platforms/52xx/Kconfig
+++ linux.git/arch/powerpc/platforms/52xx/Kconfig
@@ -1,6 +1,7 @@
 config PPC_MPC52xx
 	bool
 	select FSL_SOC
+	select PPC_CLOCK
 	default n
 
 config PPC_MPC5200