[PATCH 0/2] clk for mpc52xx

[PATCH 0/2] clk for mpc52xx

[Domen Puncer]

Hello!

Now that the generic part for powerpc went in, it's time to actually
use it.

Split into:
1 - define psc_mclk clocks for mpc52xx
2 - use those clocks in psc_spi driver


	Domen

-- 
Domen Puncer | Research & Development
.............................................................................................
Telargo d.o.o. | Zagrebška cesta 20 | 2000 Maribor | Slovenia
.............................................................................................
www.telargo.com

[PATCH 1/2] clk for mpc52xx: platform part

[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: linux.git/arch/powerpc/platforms/52xx/Makefile
===================================================================
--- linux.git.orig/arch/powerpc/platforms/52xx/Makefile
+++ linux.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: linux.git/arch/powerpc/platforms/52xx/mpc52xx_clock.c
===================================================================
--- /dev/null
+++ linux.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: linux.git/arch/powerpc/platforms/52xx/mpc52xx_common.c
===================================================================
--- linux.git.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ linux.git/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -101,6 +101,9 @@ mpc5200_setup_xlb_arbiter(void)
 	 */
 	out_be32(&xlb->config, in_be32(&xlb->config) | MPC52xx_XLB_CFG_PLDIS);
 
+	/* setup clk system */
+	mpc52xx_clock_init();
+
 	iounmap(xlb);
 }
 
Index: linux.git/include/asm-powerpc/mpc52xx.h
===================================================================
--- linux.git.orig/include/asm-powerpc/mpc52xx.h
+++ linux.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 2/2] clk for mpc52xx: use psc_mclk's in spi driver

[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 files changed, 57 insertions(+), 7 deletions(-)

Index: linux.git/drivers/spi/mpc52xx_psc_spi.c
===================================================================
--- linux.git.orig/drivers/spi/mpc52xx_psc_spi.c
+++ linux.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;
 
@@ -330,20 +366,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;
@@ -399,6 +432,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);
@@ -422,11 +456,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;
 }
@@ -511,11 +547,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);
@@ -532,6 +579,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/2] clk for mpc52xx: platform part

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]

On Sun, 14 Oct 2007 10:09:23 +0200 Domen Puncer <domen.puncer@telargo.com> wrote:
>
> +++ linux.git/arch/powerpc/platforms/52xx/mpc52xx_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>

You should include <linux/of.h> to use struct device_node and its
accessors.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/2] clk for mpc52xx: use psc_mclk's in spi driver

[Grant Likely]

On 10/14/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> Use clocks subsystem in spi driver.

I don't understand the advantage of this approach.  Is the current code broken?

I agree that abstraction is good; but in this case it seems these two
patches add a lot of code for a very simple calculation.  Also, there
is exactly 2 chips that use these devices, the mpc5200 and the
mpc5200b, and they are both wired up in exactly the same way.  I'm
inclined to believe that splitting of reading of the CDM into a
separate driver (or at least using the clk infrastructure) is over the
edge of diminishing returns.  However, I could be convinced that
having a utility function for setting the PSC clock rate is a useful
thing, but until arch/ppc goes away, you should support it in both
arch/ppc and arch/powerpc.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH 2/2] clk for mpc52xx: use psc_mclk's in spi driver

[Domen Puncer]

On 14/10/07 16:42 -0600, Grant Likely wrote:
> On 10/14/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> > Use clocks subsystem in spi driver.
> 
> I don't understand the advantage of this approach.  Is the current code broken?

Actually the calculations are broken. But ok, fix doesn't need to be like this.

And it wasn't my idea to use clk.h :-)
http://patchwork.ozlabs.org/linuxppc-embedded/patch?id=11186

> 
> I agree that abstraction is good; but in this case it seems these two
> patches add a lot of code for a very simple calculation.  Also, there
> is exactly 2 chips that use these devices, the mpc5200 and the
> mpc5200b, and they are both wired up in exactly the same way.  I'm
> inclined to believe that splitting of reading of the CDM into a
> separate driver (or at least using the clk infrastructure) is over the
> edge of diminishing returns.  However, I could be convinced that
> having a utility function for setting the PSC clock rate is a useful
> thing, but until arch/ppc goes away, you should support it in both
> arch/ppc and arch/powerpc.
> 
> Cheers,
> g.
> 
> -- 
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> grant.likely@secretlab.ca
> (403) 399-0195

-- 
Domen Puncer | Research & Development
.............................................................................................
Telargo d.o.o. | Zagrebška cesta 20 | 2000 Maribor | Slovenia
.............................................................................................
www.telargo.com