[PATCH 0/2] mpc52xx: stop drivers from accessing clock config directly

[PATCH 0/2] mpc52xx: stop drivers from accessing clock config directly

[Grant Likely]

Domen,

Here's a real solution to the problem.  I've somewhat tested this on
the lite5200b.  Can you give it a spin on efika and see if SPI still
works for you?

Thanks,
g.

--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.

[PATCH 1/2] mpc52xx: add cdm (clock module) helper function for PSCs

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

Device drivers should not access the CDM registers directly to modify
the clocking.  Instead, provide a helper function for setting the MCLK
value so that the registers can be properly protected from concurent
access.
---

 arch/powerpc/platforms/52xx/efika.c          |    2 
 arch/powerpc/platforms/52xx/lite5200.c       |    1 
 arch/powerpc/platforms/52xx/mpc52xx_common.c |  112 +++++++++++++++++++++++---
 include/asm-powerpc/mpc52xx.h                |    7 ++
 4 files changed, 107 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/platforms/52xx/efika.c b/arch/powerpc/platforms/52xx/efika.c
index a0da70c..0e3b1ac 100644
--- a/arch/powerpc/platforms/52xx/efika.c
+++ b/arch/powerpc/platforms/52xx/efika.c
@@ -180,6 +180,8 @@ static void __init efika_setup_arch(void)
 {
 	rtas_initialize();
 
+	mpc52xx_setup_clocks();
+
 	efika_pcisetup();
 
 #ifdef CONFIG_PM
diff --git a/arch/powerpc/platforms/52xx/lite5200.c b/arch/powerpc/platforms/52xx/lite5200.c
index 25d2bfa..7665e60 100644
--- a/arch/powerpc/platforms/52xx/lite5200.c
+++ b/arch/powerpc/platforms/52xx/lite5200.c
@@ -143,6 +143,7 @@ static void __init lite5200_setup_arch(void)
 	lite5200_fix_port_config();
 
 	/* Some mpc5200 & mpc5200b related configuration */
+	mpc5200_setup_clocks();
 	mpc5200_setup_xlb_arbiter();
 
 	/* Map wdt for mpc52xx_restart() */
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/platforms/52xx/mpc52xx_common.c
index 9850685..ced046b 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -14,6 +14,7 @@
 
 #include <linux/kernel.h>
 #include <linux/of_platform.h>
+#include <linux/spinlock.h>
 #include <asm/io.h>
 #include <asm/prom.h>
 #include <asm/mpc52xx.h>
@@ -26,6 +27,20 @@
  */
 static volatile struct mpc52xx_gpt *mpc52xx_wdt = NULL;
 
+/*
+ * Location of clock distibution module.  The device regs are mapped at
+ * board init time to eliminate runtime lookups.  All access to these
+ * registers is protected with the mpc52xx_cdm_lock spinlock
+ */
+static void __iomem *mpc52xx_cdm_regs = NULL;
+static spinlock_t mpc52xx_cdm_lock = SPIN_LOCK_UNLOCKED;
+
+/*
+ * Cached clock values
+ */
+static unsigned int mpc52xx_system_freq;
+static unsigned int mpc52xx_ipb_freq;
+
 static void __iomem *
 mpc52xx_map_node(struct device_node *ofn)
 {
@@ -74,26 +89,92 @@ EXPORT_SYMBOL(mpc52xx_find_and_map_path);
 unsigned int
 mpc52xx_find_ipb_freq(struct device_node *node)
 {
-	struct device_node *np;
-	const unsigned int *p_ipb_freq = NULL;
+	return mpc52xx_ipb_freq;
+}
+EXPORT_SYMBOL(mpc52xx_find_ipb_freq);
 
-	of_node_get(node);
-	while (node) {
-		p_ipb_freq = of_get_property(node, "bus-frequency", NULL);
-		if (p_ipb_freq)
-			break;
+/*
+ * Clock support for PSCs
+ */
+struct mpc52xx_cdm_psc_clk_params {
+	int div_reg;
+	int enable;
+};
 
-		np = of_get_parent(node);
-		of_node_put(node);
-		node = np;
-	}
-	if (node)
-		of_node_put(node);
+static struct mpc52xx_cdm_psc_clk_params mpc52xx_cdm_psc_clk_params[] = {
+	[0] = { .div_reg = MPC52xx_CDM_MCLKEN_DIV_PSC1_OFF, .enable = 0x20 },
+	[1] = { .div_reg = MPC52xx_CDM_MCLKEN_DIV_PSC2_OFF, .enable = 0x40 },
+	[2] = { .div_reg = MPC52xx_CDM_MCLKEN_DIV_PSC3_OFF, .enable = 0x80 },
+	[5] = { .div_reg = MPC52xx_CDM_MCLKEN_DIV_PSC6_OFF, .enable = 0x10 },
+};
+
+/**
+ * mpc52xx_cdm_set_psc_clk: Set input MCLK for a PSC
+ * @psc: id of PSC, based at 0
+ * @freq_hz: desired frequency
+ */
+int mpc52xx_cdm_set_psc_clk(int psc, u32 freq_hz)
+{
+	struct mpc52xx_cdm_psc_clk_params *params;
+	unsigned long flags;
+	u16 mclken_div;
+	u32 reg;
+
+	if (!mpc52xx_cdm_regs)
+		return -ENODEV;
 
-	return p_ipb_freq ? *p_ipb_freq : 0;
+	/* Calculate the parameters */
+	params = &mpc52xx_cdm_psc_clk_params[psc];
+	mclken_div = 0x8000 | (((mpc52xx_system_freq / freq_hz) - 1) & 0x1FF);
+
+	spin_lock_irqsave(&mpc52xx_cdm_lock, flags);
+
+	/* disable the clock before modifying frequency */
+	reg = in_be32(mpc52xx_cdm_regs + MPC52xx_CDM_CLK_ENABLES_OFF);
+	reg &= ~params->enable;
+
+	/* Set the new speed */
+	out_be16(mpc52xx_cdm_regs + params->div_reg, mclken_div);
+
+	/* Set the enable bit */
+	reg |= params->enable;
+	out_be32(mpc52xx_cdm_regs + MPC52xx_CDM_CLK_ENABLES_OFF, reg);
+
+	spin_unlock_irqrestore(&mpc52xx_cdm_lock, flags);
+
+	return 0;
 }
-EXPORT_SYMBOL(mpc52xx_find_ipb_freq);
+EXPORT_SYMBOL_GPL(mpc52xx_cdm_set_psc_clk);
 
+/**
+ * mpc5200_setup_clocks: called by platform code to setup clock frequencies
+ */
+void mpc5200_setup_clocks(void)
+{
+	struct device_node *node;
+	const unsigned int *prop = NULL;
+
+	node = of_find_compatible_node(NULL, NULL, "fsl,mpc5200");
+	if (!node)
+		node = of_find_compatible_node(NULL, NULL, "mpc5200");
+	if (!node) {
+		printk(KERN_ERR"mpc5200_setup_clocks: could not find soc node\n");
+		return;
+	}
+
+	prop = of_get_property(node, "system-frequency", NULL);
+	if (prop)
+		mpc52xx_system_freq = *prop;
+	
+	prop = of_get_property(node, "bus-frequency", NULL);
+	if (prop)
+		mpc52xx_ipb_freq = *prop;
+	of_node_put(node);
+
+	mpc52xx_cdm_regs = mpc52xx_find_and_map("fsl,mpc5200-cdm");
+	if (!mpc52xx_cdm_regs)
+		mpc52xx_cdm_regs = mpc52xx_find_and_map("mpc5200-cdm");
+}
 
 /*
  * Configure the XLB arbiter settings to match what Linux expects.
@@ -176,3 +257,4 @@ mpc52xx_restart(char *cmd)
 
 	while (1);
 }
+
diff --git a/include/asm-powerpc/mpc52xx.h b/include/asm-powerpc/mpc52xx.h
index fcb2ebb..08eb714 100644
--- a/include/asm-powerpc/mpc52xx.h
+++ b/include/asm-powerpc/mpc52xx.h
@@ -208,6 +208,7 @@ struct mpc52xx_cdm {
 	u16 fd_counters;	/* CDM + 0x12  reg4 byte2,3 */
 
 	u32 clk_enables;	/* CDM + 0x14  reg5 */
+#define MPC52xx_CDM_CLK_ENABLES_OFF	0x14
 
 	u8 osc_disable;		/* CDM + 0x18  reg6 byte0 */
 	u8 reserved0[3];	/* CDM + 0x19  reg6 byte1,2,3 */
@@ -228,15 +229,19 @@ struct mpc52xx_cdm {
 
 	u16 reserved4;		/* CDM + 0x28  reg10 byte0,1 */
 	u16 mclken_div_psc1;	/* CDM + 0x2a  reg10 byte2,3 */
+#define MPC52xx_CDM_MCLKEN_DIV_PSC1_OFF	0x2a
 
 	u16 reserved5;		/* CDM + 0x2c  reg11 byte0,1 */
 	u16 mclken_div_psc2;	/* CDM + 0x2e  reg11 byte2,3 */
+#define MPC52xx_CDM_MCLKEN_DIV_PSC2_OFF	0x2e
 
 	u16 reserved6;		/* CDM + 0x30  reg12 byte0,1 */
 	u16 mclken_div_psc3;	/* CDM + 0x32  reg12 byte2,3 */
+#define MPC52xx_CDM_MCLKEN_DIV_PSC3_OFF	0x32
 
 	u16 reserved7;		/* CDM + 0x34  reg13 byte0,1 */
 	u16 mclken_div_psc6;	/* CDM + 0x36  reg13 byte2,3 */
+#define MPC52xx_CDM_MCLKEN_DIV_PSC6_OFF	0x36
 };
 
 #endif /* __ASSEMBLY__ */
@@ -251,6 +256,8 @@ struct mpc52xx_cdm {
 extern void __iomem * mpc52xx_find_and_map(const char *);
 extern void __iomem * mpc52xx_find_and_map_path(const char *path);
 extern unsigned int mpc52xx_find_ipb_freq(struct device_node *node);
+extern int mpc52xx_cdm_set_psc_clk(int psc, u32 freq_hz);
+extern void mpc5200_setup_clocks(void);
 extern void mpc5200_setup_xlb_arbiter(void);
 extern void mpc52xx_declare_of_platform_devices(void);
 

[PATCH 2/2] mpc5200: psc-spi driver must not touch port_config or cdm registers

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

port_config and the cdm are the responsibility of firmware; and if
firmware doesn't set it up correctly, it should be fixed up by
platform code on a per-board basis because it's a property of the
board.

Drivers should never touch these registers.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/spi/mpc52xx_psc_spi.c |   84 ++++-------------------------------------
 1 files changed, 8 insertions(+), 76 deletions(-)

diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
index 7051e6c..44d1110 100644
--- a/drivers/spi/mpc52xx_psc_spi.c
+++ b/drivers/spi/mpc52xx_psc_spi.c
@@ -328,77 +328,15 @@ static void mpc52xx_psc_spi_cleanup(struct spi_device *spi)
 	kfree(spi->controller_state);
 }
 
-static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
+static int mpc52xx_psc_spi_config(int psc_id, struct mpc52xx_psc_spi *mps)
 {
-	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;
+	int rc;
 
-#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;
-		goto unmap_regs;
-	}
-
-	/* default sysclk is 512MHz */
-	mclken_div = 0x8000 |
-		(((mps->sysclk ? mps->sysclk : 512000000) / MCLK) & 0x1FF);
-
-	switch (psc_id) {
-	case 1:
-		ul = in_be32(&gpio->port_config);
-		ul &= 0xFFFFFFF8;
-		ul |= 0x00000006;
-		out_be32(&gpio->port_config, ul);
-		out_be16(&cdm->mclken_div_psc1, mclken_div);
-		ul = in_be32(&cdm->clk_enables);
-		ul |= 0x00000020;
-		out_be32(&cdm->clk_enables, ul);
-		break;
-	case 2:
-		ul = in_be32(&gpio->port_config);
-		ul &= 0xFFFFFF8F;
-		ul |= 0x00000060;
-		out_be32(&gpio->port_config, ul);
-		out_be16(&cdm->mclken_div_psc2, mclken_div);
-		ul = in_be32(&cdm->clk_enables);
-		ul |= 0x00000040;
-		out_be32(&cdm->clk_enables, ul);
-		break;
-	case 3:
-		ul = in_be32(&gpio->port_config);
-		ul &= 0xFFFFF0FF;
-		ul |= 0x00000600;
-		out_be32(&gpio->port_config, ul);
-		out_be16(&cdm->mclken_div_psc3, mclken_div);
-		ul = in_be32(&cdm->clk_enables);
-		ul |= 0x00000080;
-		out_be32(&cdm->clk_enables, ul);
-		break;
-	case 6:
-		ul = in_be32(&gpio->port_config);
-		ul &= 0xFF8FFFFF;
-		ul |= 0x00700000;
-		out_be32(&gpio->port_config, ul);
-		out_be16(&cdm->mclken_div_psc6, mclken_div);
-		ul = in_be32(&cdm->clk_enables);
-		ul |= 0x00000010;
-		out_be32(&cdm->clk_enables, ul);
-		break;
-	default:
-		ret = -EINVAL;
-		goto unmap_regs;
-	}
+	/* Setup a desirable MCLK */
+	rc = mpc52xx_cdm_set_psc_clk(psc_id, MCLK);
+	if (rc)
+		return rc;
 
 	/* Reset the PSC into a known state */
 	out_8(&psc->command, MPC52xx_PSC_RST_RX);
@@ -422,13 +360,7 @@ static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
 
 	mps->bits_per_word = 8;
 
-unmap_regs:
-	if (cdm)
-		iounmap(cdm);
-	if (gpio)
-		iounmap(gpio);
-
-	return ret;
+	return 0;
 }
 
 static irqreturn_t mpc52xx_psc_spi_isr(int irq, void *dev_id)
@@ -493,7 +425,7 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
 	if (ret)
 		goto free_master;
 
-	ret = mpc52xx_psc_spi_port_config(master->bus_num, mps);
+	ret = mpc52xx_psc_spi_config(master->bus_num-1, mps);
 	if (ret < 0)
 		goto free_irq;
 

Re: [PATCH 0/2] mpc52xx: stop drivers from accessing clock config directly

[Domen Puncer]

On 24/10/07 12:24 -0600, Grant Likely wrote:
> Domen,
> 
> Here's a real solution to the problem.  I've somewhat tested this on
> the lite5200b.  Can you give it a spin on efika and see if SPI still
> works for you?

My test case was lite5200b too, I don't think I ever tried SPI on
efika.
(Are even the right pins on irda connector, or is a necessary line
missing?)


	Domen

> 
> Thanks,
> g.
> 
> --
> Grant Likely, B.Sc. P.Eng.
> Secret Lab Technologies Ltd.

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

Re: [PATCH 0/2] mpc52xx: stop drivers from accessing clock config directly

[Grant Likely]

On 10/24/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> On 24/10/07 12:24 -0600, Grant Likely wrote:
> > Domen,
> >
> > Here's a real solution to the problem.  I've somewhat tested this on
> > the lite5200b.  Can you give it a spin on efika and see if SPI still
> > works for you?
>
> My test case was lite5200b too, I don't think I ever tried SPI on
> efika.
> (Are even the right pins on irda connector, or is a necessary line
> missing?)

Hmm, I guess that's right.  Can you at least make sure it still boots
on Efika?  Some of the clock detection stuff has changed so I want to
make sure it still boots.

Are you setup to do your SPI test easily on you lite5200b?  When I say
"somewhat" tested; I mean I probed the driver and it didn't crash.
:-)  I haven't tried to run traffic over it.

Can you check that on your system?  If not, can you email me what
setup/programs you used for testing?  I know very little about the SPI
infrastructure.

Thanks,
g.

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

Re: [PATCH 0/2] mpc52xx: stop drivers from accessing clock config directly

[Domen Puncer]

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

On 24/10/07 14:14 -0600, Grant Likely wrote:
> On 10/24/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> > On 24/10/07 12:24 -0600, Grant Likely wrote:
> > > Domen,
> > >
> > > Here's a real solution to the problem.  I've somewhat tested this on
> > > the lite5200b.  Can you give it a spin on efika and see if SPI still
> > > works for you?
> >
> > My test case was lite5200b too, I don't think I ever tried SPI on
> > efika.
> > (Are even the right pins on irda connector, or is a necessary line
> > missing?)
> 
> Hmm, I guess that's right.  Can you at least make sure it still boots
> on Efika?  Some of the clock detection stuff has changed so I want to
> make sure it still boots.

OK. I'll do that tomorrow.

> 
> Are you setup to do your SPI test easily on you lite5200b?  When I say
> "somewhat" tested; I mean I probed the driver and it didn't crash.
> :-)  I haven't tried to run traffic over it.

Sorry, lite5200b is resting these days. :-(

> 
> Can you check that on your system?  If not, can you email me what
> setup/programs you used for testing?  I know very little about the SPI
> infrastructure.

For userspace part I used something like: Documentation/spi/spidev
And for kernel the attached, to fill get binded to spidev driver.


	Domen

> 
> Thanks,
> 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

[-- Attachment #2: spidev_test_devices --]
[-- Type: text/plain, Size: 1502 bytes --]

---
 drivers/spi/Makefile           |    1 +
 drivers/spi/spi_test_devices.c |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

Index: work-powerpc.git/drivers/spi/Makefile
===================================================================
--- work-powerpc.git.orig/drivers/spi/Makefile
+++ work-powerpc.git/drivers/spi/Makefile
@@ -35,3 +35,4 @@ obj-$(CONFIG_SPI_SPIDEV)	+= spidev.o
 
 # SPI slave drivers (protocol for that link)
 # 	... add above this line ...
+obj-m				+= spi_test_devices.o
Index: work-powerpc.git/drivers/spi/spi_test_devices.c
===================================================================
--- /dev/null
+++ work-powerpc.git/drivers/spi/spi_test_devices.c
@@ -0,0 +1,38 @@
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/spi/spi.h>
+
+static struct spi_board_info spi_info[7];
+static struct spi_device *spidev[7];
+static int testdev_init(void)
+{
+	struct spi_board_info *info;
+	int i;
+
+	for (i=0; i<7; i++) {
+		struct spi_master *master;
+
+		info = &spi_info[i];
+		//info->max_speed_hz = 2*1000000;
+		info->max_speed_hz = 100000;
+		//info->max_speed_hz = 1*1000000;
+		strcpy(info->modalias, "spidev");
+
+		master = spi_busnum_to_master(i);
+		if (master)
+			spidev[i] = spi_new_device(master, info);
+	}
+	return 0;
+}
+
+static void testdev_exit(void)
+{
+	/* there is no _remove? */
+	/*for (i=0; i<7; i++) {
+	}*/
+}
+
+module_init(testdev_init);
+module_exit(testdev_exit);
+
+MODULE_LICENSE("GPL");

Re: [PATCH 1/2] mpc52xx: add cdm (clock module) helper function for PSCs

[Stephen Rothwell]

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

On Wed, 24 Oct 2007 12:24:26 -0600 Grant Likely <grant.likely@secretlab.ca> wrote:
>
> +static spinlock_t mpc52xx_cdm_lock = SPIN_LOCK_UNLOCKED;

static DEFINE_SPINLOCK(mpc52xx_cdm_lock);

-- 
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 0/2] mpc52xx: stop drivers from accessing clock config directly

[Domen Puncer]

On 24/10/07 14:14 -0600, Grant Likely wrote:
> On 10/24/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> > On 24/10/07 12:24 -0600, Grant Likely wrote:
> > > Domen,
> > >
> > > Here's a real solution to the problem.  I've somewhat tested this on
> > > the lite5200b.  Can you give it a spin on efika and see if SPI still
> > > works for you?
> >
> > My test case was lite5200b too, I don't think I ever tried SPI on
> > efika.
> > (Are even the right pins on irda connector, or is a necessary line
> > missing?)
> 
> Hmm, I guess that's right.  Can you at least make sure it still boots
> on Efika?  Some of the clock detection stuff has changed so I want to
> make sure it still boots.

OK, with the following patch it compiles and boots.
I don't have any psc configured as spi in device tree though.



---
 arch/powerpc/platforms/52xx/efika.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux.git/arch/powerpc/platforms/52xx/efika.c
===================================================================
--- linux.git.orig/arch/powerpc/platforms/52xx/efika.c
+++ linux.git/arch/powerpc/platforms/52xx/efika.c
@@ -180,7 +180,7 @@ static void __init efika_setup_arch(void
 {
 	rtas_initialize();
 
-	mpc52xx_setup_clocks();
+	mpc5200_setup_clocks();
 
 	efika_pcisetup();