Skip to contentSkip to conversation

clk for mpc52xx

PATCH In review Domen Puncer 3 participants 6 messages Oct 14, 2007 – Oct 15, 2007

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover clk for mpc52xx

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

Signature · 5 lines
Domen Puncer | Research & Development
.............................................................................................
Telargo d.o.o. | Zagrebška cesta 20 | 2000 Maribor | Slovenia
.............................................................................................
www.telargo.com
#1 lore raw

PATCH 1/2 clk for mpc52xx: platform part

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

---
 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(-)
arch/powerpc/platforms/52xx/Makefile +1 -1
Index: 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
 
arch/powerpc/platforms/52xx/mpc52xx_clock.c new file +352 -0

352 lines not on this page · raw message

arch/powerpc/platforms/52xx/mpc52xx_common.c +3 -0
Index: 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);
 }
 
include/asm-powerpc/mpc52xx.h +2 -0
Index: 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__ */
 
#2 lore raw

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

arch/powerpc/platforms/52xx/mpc52xx_clock.c line 16

Quoted 13 lines · On Sun, 14 Oct 2007 10:09:23 +0200 Domen Puncer <[email protected]>…

+++ linux.git/arch/powerpc/platforms/52xx/mpc52xx_clock.c

arch/powerpc/platforms/52xx/mpc52xx_clock.c new file +11 -0
+#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.

Signature · 3 lines
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/
#3 lore raw

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

In reply to Domen Puncer

Use clocks subsystem in spi driver.

---
 drivers/spi/mpc52xx_psc_spi.c |   64 +++++++++++++++++++++++++++++++++++++-----
 1 files changed, 57 insertions(+), 7 deletions(-)
drivers/spi/mpc52xx_psc_spi.c +57 -7
Index: 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);
#4 lore raw

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

Quoted 1 line · Domen Puncer

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.

Signature · 4 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[email protected]
(403) 399-0195
#5 lore raw

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

Quoted 4 lines · Grant Likely
Quoted 1 line · Domen Puncer

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

Quoted 20 lines

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.

Signature · 4 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[email protected]
(403) 399-0195
Signature · 5 lines
Domen Puncer | Research & Development
.............................................................................................
Telargo d.o.o. | Zagrebška cesta 20 | 2000 Maribor | Slovenia
.............................................................................................
www.telargo.com