Re: [PATCH][RFC]Updated MPC I2C driver

Re: [PATCH][RFC]Updated MPC I2C driver

[Matthew McClintock]

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

Hi Adrian,

	The driver works well on both an mpc8540ads and a sandpoint here.
However, I just learned yesterday that i2c 32-bit read/writes on the
85xx will not always be guaranteed to work. So there does in fact need
to be an abstraction there for 8-bit/32-bit read/writes. I wrote up a
patch that does just that and it is attached, any suggestions or
different ways to implement it I would be glad to take a go at it if
your busy. I tested the patch on our mpc8540ads and sandpoint here, and
it seems to work well. One note: the FS_I2C_32BIT needs to be set in the
flags. Let me know what you think.

Regards,
Matthew

On Thu, 2004-07-01 at 18:19, Adrian Cox wrote:
> This version should support MPC8540 as well as MPC8245 and MPC107. I
> don't have MPC85xx hardware, so I'd like feedback from people who do.
> To use it on MPC107/824x requires the OCP patch that went by on the list
> on Tuesday. The support for MPC85xx is already in the kernel.
> If nobody has any complaints I'm ready to send it to Greg K-H.
> - Adrian Cox
> Humboldt Solutions Ltd.
--
Matthew McClintock <mcclintock@freescale.com>

[-- Attachment #2: mpc-i2c-byte-word.patch --]
[-- Type: text/x-patch, Size: 4820 bytes --]

diff -purN linuxppc-2.5-i2c-bak/drivers/i2c/busses/i2c-mpc.c linuxppc-2.5-i2c/drivers/i2c/busses/i2c-mpc.c
--- linuxppc-2.5-i2c-bak/drivers/i2c/busses/i2c-mpc.c	2004-07-01 14:16:22.000000000 -0500
+++ linuxppc-2.5-i2c/drivers/i2c/busses/i2c-mpc.c	2004-07-01 14:47:39.000000000 -0500
@@ -72,18 +72,40 @@ struct mpc_i2c {
 	struct i2c_adapter adap;
 };

+static u32 mpc_i2c_read(struct mpc_i2c *i2c, char *addr)
+{
+	struct ocp_fs_i2c_data *i2c_data = i2c->ocpdef->additions;
+
+	if (i2c_data && (i2c_data->flags & FS_I2C_32BIT)) {
+		return readl(addr);
+	} else {
+		return readb(addr);
+	}
+}
+
+static void mpc_i2c_write(struct mpc_i2c *i2c, u32 val, char *addr)
+{
+	struct ocp_fs_i2c_data *i2c_data = i2c->ocpdef->additions;
+
+	if (i2c_data && (i2c_data->flags & FS_I2C_32BIT)) {
+		writel(val, addr);
+	} else {
+		writeb(val & 0xff, addr);
+	}
+}
+
 static __inline__ void writeccr(struct mpc_i2c *i2c, u32 x)
 {
-	writel(x, i2c->base + MPC_I2C_CR);
+	mpc_i2c_write(i2c, x, i2c->base + MPC_I2C_CR);
 }

 static irqreturn_t mpc_i2c_isr(int irq, void *dev_id, struct pt_regs * regs)
 {
 	struct mpc_i2c *i2c = dev_id;
-	if (readl(i2c->base + MPC_I2C_SR) & CSR_MIF) {
+	if (mpc_i2c_read(i2c, i2c->base + MPC_I2C_SR) & CSR_MIF) {
 		/* Read again to allow register to stabilise */
-		i2c->interrupt = readl(i2c->base + MPC_I2C_SR);
-		writel(0, i2c->base + MPC_I2C_SR);
+		i2c->interrupt = mpc_i2c_read(i2c, i2c->base + MPC_I2C_SR);
+		mpc_i2c_write(i2c, 0, i2c->base + MPC_I2C_SR);
 		wake_up_interruptible(&i2c->queue);
 	}
 	return IRQ_HANDLED;
@@ -97,7 +119,7 @@ static int i2c_wait(struct mpc_i2c *i2c,
 	int result = 0;

 	if (i2c->ocpdef->irq == OCP_IRQ_NA) {
-		while(! (readl(i2c->base + MPC_I2C_SR) & CSR_MIF)) {
+		while(! (mpc_i2c_read(i2c, i2c->base + MPC_I2C_SR) & CSR_MIF)) {
 			schedule();
 			if (orig_jiffies + timeout < jiffies) {
 				DPRINTK("I2C: timeout\n");
@@ -105,8 +127,8 @@ static int i2c_wait(struct mpc_i2c *i2c,
 				break;
 			}
 		}
-		x = readl(i2c->base + MPC_I2C_SR);
-		writel(0, i2c->base + MPC_I2C_SR);
+		x = mpc_i2c_read(i2c, i2c->base + MPC_I2C_SR);
+		mpc_i2c_write(i2c, 0, i2c->base + MPC_I2C_SR);
 	} else {
 		add_wait_queue(&i2c->queue, &wait);
 		while ( ! (i2c->interrupt & CSR_MIF)) {
@@ -158,12 +180,12 @@ static void mpc_i2c_start(struct mpc_i2c
 	struct ocp_fs_i2c_data *i2c_data = i2c->ocpdef->additions;
         /* Set clock and filters */
 	if (i2c_data && (i2c_data->flags & FS_I2C_SEPARATE_DFSRR)) {
-		writel(0x31, i2c->base + MPC_I2C_FDR);
-		writel(0x10, i2c->base + MPC_I2C_DFSRR);
+		mpc_i2c_write(i2c, 0x31, i2c->base + MPC_I2C_FDR);
+		mpc_i2c_write(i2c, 0x10, i2c->base + MPC_I2C_DFSRR);
 	} else
-		writel(0x1031, i2c->base + MPC_I2C_FDR);
+		mpc_i2c_write(i2c, 0x1031, i2c->base + MPC_I2C_FDR);
         /* Clear arbitration */
-        writel(0, i2c->base + MPC_I2C_SR);
+        mpc_i2c_write(i2c, 0, i2c->base + MPC_I2C_SR);
        	/* Start with MEN */
 	writeccr(i2c, CCR_MEN);
 }
@@ -187,14 +209,14 @@ static int mpc_write(struct mpc_i2c *i2c
 	writeccr(i2c, CCR_MIEN | CCR_MEN |
 		      CCR_MSTA | CCR_MTX | flags);
 	/* Write target byte */
-	writel((target << 1), i2c->base + MPC_I2C_DR);
+	mpc_i2c_write(i2c, (target << 1), i2c->base + MPC_I2C_DR);

 	if (i2c_wait(i2c, timeout, 1) < 0)
 	    return -1;

         for(i = 0; i < length; i++) {
 	        /* Write data byte */
-	        writel(data[i], i2c->base + MPC_I2C_DR);
+	        mpc_i2c_write(i2c, data[i], i2c->base + MPC_I2C_DR);

 	        if (i2c_wait(i2c, timeout, 1) < 0)
 	            return -1;
@@ -217,7 +239,7 @@ static int mpc_read(struct mpc_i2c *i2c,
 	writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA |
 		      CCR_MTX | flags);
 	/* Write target address byte - this time with the read flag set */
-	writel((target << 1) | 1, i2c->base + MPC_I2C_DR);
+	mpc_i2c_write(i2c, (target << 1) | 1, i2c->base + MPC_I2C_DR);

 	if (i2c_wait(i2c, timeout, 0) < 0)
 	    return -1;
@@ -228,7 +250,7 @@ static int mpc_read(struct mpc_i2c *i2c,
 	else
 	    writeccr(i2c, CCR_MIEN| CCR_MEN | CCR_MSTA);
     	/* Dummy read */
-	readl(i2c->base + MPC_I2C_DR);
+	mpc_i2c_read(i2c, i2c->base + MPC_I2C_DR);

 	for(i = 0; i < length; i++) {
 		if (i2c_wait(i2c, timeout, 0) < 0)
@@ -242,7 +264,7 @@ static int mpc_read(struct mpc_i2c *i2c,
 		if (i == length - 1)
 			writeccr(i2c, CCR_MIEN | CCR_MEN |
 				      CCR_TXAK);
-		data[i] = readl(i2c->base + MPC_I2C_DR);
+		data[i] = mpc_i2c_read(i2c, i2c->base + MPC_I2C_DR);
 	}

 	return length;
@@ -260,7 +282,7 @@ static int mpc_xfer(struct i2c_adapter *
         mpc_i2c_start(i2c);

 	/* Allow bus up to 1s to become not busy */
-	while(readl(i2c->base + MPC_I2C_SR) & CSR_MBB) {
+	while(mpc_i2c_read(i2c, i2c->base + MPC_I2C_SR) & CSR_MBB) {
                 if (signal_pending(current))
 		{
 			DPRINTK("I2C: Interrupted\n");

Re: [PATCH][RFC]Updated MPC I2C driver

[Matthew McClintock]

The blocks look very similar, you might want to look at the follow up
patch I just sent that abstracts the read/writes. It might be as simple
as abstracting another read/write function for your particular platform.

Regards,
Matthew

On Thu, 2004-07-01 at 19:20, Sylvain Munaut wrote:
> Adrian Cox wrote:
>
>  > This version should support MPC8540 as well as MPC8245 and MPC107.
>  > I don't have MPC85xx hardware, so I'd like feedback from people who
>  > do.
>  >
>  > To use it on MPC107/824x requires the OCP patch that went by on the
>  > list on Tuesday. The support for MPC85xx is already in the kernel.
>  >
>  > If nobody has any complaints I'm ready to send it to Greg K-H.
>
>
> Interesting, it really looks like the MPC5200 I2C controller.
> Unfortunatly :
>  - On the MPC5200, all the status/controls bits are in the MSBs ( so
> the consts would need to be << 24 )
>  - And there is a interrupt control register I don't see in your code.
> It's shared by the two I2C controllers onboard.
>
> I'll try to see if I can make it work on the lite5200 board.
>
> Sylvain Munaut
--
Matthew McClintock <mcclintock@freescale.com>


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

[PATCH][RFC]Updated MPC I2C driver

[Adrian Cox]

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

This version should support MPC8540 as well as MPC8245 and MPC107. I
don't have MPC85xx hardware, so I'd like feedback from people who do.

To use it on MPC107/824x requires the OCP patch that went by on the list
on Tuesday. The support for MPC85xx is already in the kernel.

If nobody has any complaints I'm ready to send it to Greg K-H.

- Adrian Cox
Humboldt Solutions Ltd.


[-- Attachment #2: mpc-ic-2.patch --]
[-- Type: text/x-patch, Size: 12511 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/07/01 17:53:05+01:00 adrian@humboldt.co.uk
#   Add I2C driver for Motorola I2C
#
# drivers/i2c/busses/i2c-mpc.c
#   2004/07/01 17:52:55+01:00 adrian@humboldt.co.uk +410 -0
#
# drivers/i2c/busses/i2c-mpc.c
#   2004/07/01 17:52:55+01:00 adrian@humboldt.co.uk +0 -0
#   BitKeeper file /home/adrian/kernels/sa107-2.6/for-upstream/drivers/i2c/busses/i2c-mpc.c
#
# drivers/i2c/busses/Makefile
#   2004/07/01 17:52:55+01:00 adrian@humboldt.co.uk +1 -0
#   Add I2C driver for Motorola I2C
#
# drivers/i2c/busses/Kconfig
#   2004/07/01 17:52:54+01:00 adrian@humboldt.co.uk +11 -0
#   Add I2C driver for Motorola I2C
#
diff -Nru a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
--- a/drivers/i2c/busses/Kconfig	Thu Jul  1 17:54:57 2004
+++ b/drivers/i2c/busses/Kconfig	Thu Jul  1 17:54:57 2004
@@ -396,4 +396,15 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-voodoo3.

+config I2C_MPC
+	tristate "MPC107/824x/85xx"
+	depends on I2C && FSL_OCP
+	help
+	  If you say yes to this option, support will be included for the
+	  built-in I2C interface on the MPC107/Tsi107/MPC8240/MPC8245 and
+	  MPC85xx family processors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-mpc.
+
 endmenu
diff -Nru a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
--- a/drivers/i2c/busses/Makefile	Thu Jul  1 17:54:57 2004
+++ b/drivers/i2c/busses/Makefile	Thu Jul  1 17:54:57 2004
@@ -32,6 +32,7 @@
 obj-$(CONFIG_I2C_VOODOO3)	+= i2c-voodoo3.o
 obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
 obj-$(CONFIG_SCx200_I2C)	+= scx200_i2c.o
+obj-$(CONFIG_I2C_MPC)	+= i2c-mpc.o

 ifeq ($(CONFIG_I2C_DEBUG_BUS),y)
 EXTRA_CFLAGS += -DDEBUG
diff -Nru a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
--- /dev/null	Wed Dec 31 16:00:00 1969
+++ b/drivers/i2c/busses/i2c-mpc.c	Thu Jul  1 17:54:57 2004
@@ -0,0 +1,410 @@
+/*
+ * (C) Copyright 2003-2004
+ * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
+
+ * This is a combined i2c adapter and algorithm driver for the
+ * MPC107/Tsi107 PowerPC northbridge and processors that include
+ * the same I2C unit (8240, 8245, 85xx).
+ *
+ * Release 0.5
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <linux/config.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <asm/io.h>
+#include <asm/ocp.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+
+#ifdef CONFIG_I2C_DEBUG_BUS
+#define DPRINTK(args...) printk(KERN_DEBUG __FILE__": " args)
+#else
+#define DPRINTK(args...)
+#endif
+
+
+#define MPC_I2C_ADDR  0x00
+#define MPC_I2C_FDR 	0x04
+#define MPC_I2C_CR	0x08
+#define MPC_I2C_SR	0x0c
+#define MPC_I2C_DR	0x10
+#define MPC_I2C_DFSRR 0x14
+#define MPC_I2C_REGION 0x20
+
+#define CCR_MEN  0x80
+#define CCR_MIEN 0x40
+#define CCR_MSTA 0x20
+#define CCR_MTX  0x10
+#define CCR_TXAK 0x08
+#define CCR_RSTA 0x04
+
+#define CSR_MCF  0x80
+#define CSR_MAAS 0x40
+#define CSR_MBB  0x20
+#define CSR_MAL  0x10
+#define CSR_SRW  0x04
+#define CSR_MIF  0x02
+#define CSR_RXAK 0x01
+
+struct mpc_i2c {
+	char *base;
+	struct ocp_def *ocpdef;
+	u32 interrupt;
+	wait_queue_head_t queue;
+	struct i2c_adapter adap;
+};
+
+static __inline__ void writeccr(struct mpc_i2c *i2c, u32 x)
+{
+	writel(x, i2c->base + MPC_I2C_CR);
+}
+
+static irqreturn_t mpc_i2c_isr(int irq, void *dev_id, struct pt_regs * regs)
+{
+	struct mpc_i2c *i2c = dev_id;
+	if (readl(i2c->base + MPC_I2C_SR) & CSR_MIF) {
+		/* Read again to allow register to stabilise */
+		i2c->interrupt = readl(i2c->base + MPC_I2C_SR);
+		writel(0, i2c->base + MPC_I2C_SR);
+		wake_up_interruptible(&i2c->queue);
+	}
+	return IRQ_HANDLED;
+}
+
+static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
+{
+	DECLARE_WAITQUEUE(wait, current);
+        unsigned long orig_jiffies = jiffies;
+        u32 x;
+	int result = 0;
+
+	if (i2c->ocpdef->irq == OCP_IRQ_NA) {
+		while(! (readl(i2c->base + MPC_I2C_SR) & CSR_MIF)) {
+			schedule();
+			if (orig_jiffies + timeout < jiffies) {
+				DPRINTK("I2C: timeout\n");
+				result = -EIO;
+				break;
+			}
+		}
+		x = readl(i2c->base + MPC_I2C_SR);
+		writel(0, i2c->base + MPC_I2C_SR);
+	} else {
+		add_wait_queue(&i2c->queue, &wait);
+		while ( ! (i2c->interrupt & CSR_MIF)) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			if (signal_pending(current)) {
+				DPRINTK("I2C: Interrupted\n");
+				result = -EINTR;
+				break;
+			}
+			if (orig_jiffies + timeout < jiffies) {
+				DPRINTK("I2C: timeout\n");
+				result = -EIO;
+				break;
+			}
+			schedule_timeout(timeout);
+		}
+		current->state = TASK_RUNNING;
+		remove_wait_queue(&i2c->queue, &wait);
+		x = i2c->interrupt;
+		i2c->interrupt = 0;
+	}
+
+	if (result < -0)
+	    return result;
+
+	if (! (x & CSR_MCF))
+	{
+	    DPRINTK("I2C: unfinished\n");
+	    return -EIO;
+	}
+
+        if (x & CSR_MAL)
+	{
+	    	DPRINTK("I2C: MAL\n");
+                return -EIO;
+	}
+
+        if (writing && (x & CSR_RXAK)) {
+                DPRINTK("I2C: No RXAK\n");
+                /* generate stop */
+                writeccr(i2c, CCR_MEN);
+                return -EIO;
+        }
+        return 0;
+}
+
+static void mpc_i2c_start(struct mpc_i2c *i2c)
+{
+	struct ocp_fs_i2c_data *i2c_data = i2c->ocpdef->additions;
+        /* Set clock and filters */
+	if (i2c_data && (i2c_data->flags & FS_I2C_SEPARATE_DFSRR)) {
+		writel(0x31, i2c->base + MPC_I2C_FDR);
+		writel(0x10, i2c->base + MPC_I2C_DFSRR);
+	} else
+		writel(0x1031, i2c->base + MPC_I2C_FDR);
+        /* Clear arbitration */
+        writel(0, i2c->base + MPC_I2C_SR);
+       	/* Start with MEN */
+	writeccr(i2c, CCR_MEN);
+}
+
+static void mpc_i2c_stop(struct mpc_i2c *i2c)
+{
+	writeccr(i2c, CCR_MEN);
+}
+
+static int mpc_write(struct mpc_i2c *i2c, int target,
+			const u8 *data, int length, int restart)
+{
+        int i;
+        unsigned timeout = HZ;
+    	u32 flags = restart ? CCR_RSTA : 0;
+
+	/* Start with MEN */
+	if (! restart)
+		writeccr(i2c, CCR_MEN);
+	/* Start as master */
+	writeccr(i2c, CCR_MIEN | CCR_MEN |
+		      CCR_MSTA | CCR_MTX | flags);
+	/* Write target byte */
+	writel((target << 1), i2c->base + MPC_I2C_DR);
+
+	if (i2c_wait(i2c, timeout, 1) < 0)
+	    return -1;
+
+        for(i = 0; i < length; i++) {
+	        /* Write data byte */
+	        writel(data[i], i2c->base + MPC_I2C_DR);
+
+	        if (i2c_wait(i2c, timeout, 1) < 0)
+	            return -1;
+        }
+
+        return 0;
+}
+
+static int mpc_read(struct mpc_i2c *i2c, int target,
+		       u8 *data, int length, int restart)
+{
+    	unsigned timeout = HZ;
+    	int i;
+    	u32 flags = restart ? CCR_RSTA : 0;
+
+	/* Start with MEN */
+	if (! restart)
+		writeccr(i2c, CCR_MEN);
+	/* Switch to read - restart */
+	writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA |
+		      CCR_MTX | flags);
+	/* Write target address byte - this time with the read flag set */
+	writel((target << 1) | 1, i2c->base + MPC_I2C_DR);
+
+	if (i2c_wait(i2c, timeout, 0) < 0)
+	    return -1;
+
+	if (length == 1)
+	    writeccr(i2c, CCR_MIEN| CCR_MEN |
+			  CCR_MSTA | CCR_TXAK);
+	else
+	    writeccr(i2c, CCR_MIEN| CCR_MEN | CCR_MSTA);
+    	/* Dummy read */
+	readl(i2c->base + MPC_I2C_DR);
+
+	for(i = 0; i < length; i++) {
+		if (i2c_wait(i2c, timeout, 0) < 0)
+			return -1;
+
+		/* Generate txack on next to last byte */
+		if (i == length - 2)
+			writeccr(i2c, CCR_MIEN | CCR_MEN |
+				      CCR_MSTA | CCR_TXAK);
+		/* Generate stop on last byte */
+		if (i == length - 1)
+			writeccr(i2c, CCR_MIEN | CCR_MEN |
+				      CCR_TXAK);
+		data[i] = readl(i2c->base + MPC_I2C_DR);
+	}
+
+	return length;
+}
+
+static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
+	                   int num)
+{
+	struct i2c_msg *pmsg;
+	int i;
+	int ret=0;
+        unsigned long orig_jiffies = jiffies;
+	struct mpc_i2c *i2c = i2c_get_adapdata(adap);
+
+        mpc_i2c_start(i2c);
+
+	/* Allow bus up to 1s to become not busy */
+	while(readl(i2c->base + MPC_I2C_SR) & CSR_MBB) {
+                if (signal_pending(current))
+		{
+			DPRINTK("I2C: Interrupted\n");
+                        return -EINTR;
+		}
+                if (orig_jiffies + HZ < jiffies)
+		{
+		    	printk("I2C: timeout\n");
+                        return -EIO;
+		}
+                schedule();
+        }
+
+	for (i = 0;ret >= 0 && i < num; i++) {
+		pmsg = &msgs[i];
+		DPRINTK("Doing %s %d bytes to 0x%02x - %d of %d messages\n",
+		     pmsg->flags & I2C_M_RD ? "read" : "write",
+                     pmsg->len, pmsg->addr, i + 1, num);
+		if (pmsg->flags & I2C_M_RD)
+                        ret = mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+                else
+                        ret = mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+        }
+        mpc_i2c_stop(i2c);
+        return (ret < 0) ? ret : num;
+}
+
+static u32 mpc_functionality(struct i2c_adapter *adap)
+{
+        return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm mpc_algo = {
+	.name = "MPC algorithm",
+	.id = I2C_ALGO_MPC107,
+	.master_xfer = mpc_xfer,
+	.functionality = mpc_functionality,
+};
+
+static struct i2c_adapter mpc_ops = {
+	.owner = THIS_MODULE,
+	.name = "MPC adapter",
+	.id = I2C_ALGO_MPC107 | I2C_HW_MPC107,
+	.algo = &mpc_algo,
+	.class = I2C_CLASS_HWMON,
+	.timeout = 1,
+	.retries = 1
+};
+
+static int __devinit mpc_i2c_probe(struct ocp_device *ocp)
+{
+	int result = 0;
+	struct mpc_i2c *i2c;
+
+	if (!(i2c = kmalloc(sizeof(*i2c), GFP_KERNEL))){
+		return -ENOMEM;
+	}
+	i2c->ocpdef = ocp->def;
+	init_waitqueue_head(&i2c->queue);
+
+	if (! request_mem_region(ocp->def->paddr, MPC_I2C_REGION,
+		                 "i2c-mpc")) {
+		printk(KERN_ERR "i2c-mpc - resource unavailable\n");
+		return -ENODEV;
+	}
+
+	i2c->base = ioremap(ocp->def->paddr, MPC_I2C_REGION);
+
+	if (! i2c->base) {
+		printk(KERN_ERR "i2c-mpc - failed to map controller\n");
+		result = -ENOMEM;
+		goto fail_map;
+	}
+
+	if (ocp->def->irq != OCP_IRQ_NA)
+		if ((result = request_irq(ocp->def->irq, mpc_i2c_isr,
+					  0, "i2c-mpc", i2c)) < 0) {
+			printk(KERN_ERR "i2c-mpc - failed to attach interrupt\n");
+			goto fail_irq;
+		}
+
+	i2c->adap = mpc_ops;
+	i2c_set_adapdata(&i2c->adap, i2c);
+	if ((result = i2c_add_adapter(&i2c->adap)) < 0) {
+		printk(KERN_ERR "i2c-mpc - failed to add adapter\n");
+		goto fail_add;
+	}
+	ocp_set_drvdata(ocp, i2c);
+	return result;
+
+fail_add:
+	if (ocp->def->irq != OCP_IRQ_NA)
+		free_irq(ocp->def->irq, 0);
+fail_irq:
+	iounmap(i2c->base);
+fail_map:
+	release_mem_region(ocp->def->paddr, MPC_I2C_REGION);
+	kfree(i2c);
+	return result;
+}
+static void __devexit mpc_i2c_remove(struct ocp_device *ocp)
+{
+	struct mpc_i2c *i2c = ocp_get_drvdata(ocp);
+	ocp_set_drvdata(ocp, NULL);
+	i2c_del_adapter(&i2c->adap);
+
+	if (ocp->def->irq != OCP_IRQ_NA)
+		free_irq(i2c->ocpdef->irq, i2c);
+	iounmap(i2c->base);
+	release_mem_region(i2c->ocpdef->paddr, MPC_I2C_REGION);
+	kfree(i2c);
+}
+
+static struct ocp_device_id mpc_iic_ids[] __devinitdata =
+{
+	{ .vendor = OCP_VENDOR_FREESCALE, .function = OCP_FUNC_IIC },
+	{ .vendor = OCP_VENDOR_INVALID }
+};
+
+MODULE_DEVICE_TABLE(ocp, mpc_iic_ids);
+
+static struct ocp_driver mpc_iic_driver =
+{
+	.name 		= "iic",
+	.id_table	= mpc_iic_ids,
+	.probe		= mpc_i2c_probe,
+	.remove		= __devexit_p(mpc_i2c_remove)
+};
+
+static int __init iic_init(void)
+{
+	return ocp_register_driver(&mpc_iic_driver);
+}
+
+static void __exit iic_exit(void)
+{
+	ocp_unregister_driver(&mpc_iic_driver);
+}
+
+module_init(iic_init);
+module_exit(iic_exit);
+
+
+MODULE_AUTHOR("Adrian Cox <adrian@humboldt.co.uk>");
+MODULE_DESCRIPTION("I2C-Bus adapter for MPC107 bridge and MPC824x/85xx processors");
+MODULE_LICENSE("GPL");

Re: [PATCH][RFC]Updated MPC I2C driver

[Eugene Surovegin]

On Thu, Jul 01, 2004 at 07:19:15PM +0100, Adrian Cox wrote:

[snip]

> +static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
> +{
> +	DECLARE_WAITQUEUE(wait, current);
> +        unsigned long orig_jiffies = jiffies;
> +        u32 x;
> +	int result = 0;
> +
> +	if (i2c->ocpdef->irq == OCP_IRQ_NA) {
> +		while(! (readl(i2c->base + MPC_I2C_SR) & CSR_MIF)) {
> +			schedule();
> +			if (orig_jiffies + timeout < jiffies) {

This doesn't look right. Please, use time_after()/time_before() here.

--
Eugene

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH][RFC]Updated MPC I2C driver

[Sylvain Munaut]

Adrian Cox wrote:

 > This version should support MPC8540 as well as MPC8245 and MPC107.
 > I don't have MPC85xx hardware, so I'd like feedback from people who
 > do.
 >
 > To use it on MPC107/824x requires the OCP patch that went by on the
 > list on Tuesday. The support for MPC85xx is already in the kernel.
 >
 > If nobody has any complaints I'm ready to send it to Greg K-H.


Interesting, it really looks like the MPC5200 I2C controller.
Unfortunatly :
 - On the MPC5200, all the status/controls bits are in the MSBs ( so
the consts would need to be << 24 )
 - And there is a interrupt control register I don't see in your code.
It's shared by the two I2C controllers onboard.

I'll try to see if I can make it work on the lite5200 board.

Sylvain Munaut


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH][RFC]Updated MPC I2C driver

[Adrian Cox]

On Thu, 2004-07-01 at 20:20, Sylvain Munaut wrote:

> Interesting, it really looks like the MPC5200 I2C controller.
> Unfortunatly :
>  - On the MPC5200, all the status/controls bits are in the MSBs ( so
> the consts would need to be << 24 )

Are you sure about that? The MPC107 registers are little-endian PCI
registers, accessed with readl/writel, but it looks to me that the
MPC5200 registers are big-endian.

>  - And there is a interrupt control register I don't see in your code.
> It's shared by the two I2C controllers onboard.

That could easily be added as a special case.

> I'll try to see if I can make it work on the lite5200 board.

Please do try. It would be good if one driver covered all these chips.

I'm going to try byte reads and writes on the MPC107 as my next
experiment. I think that may avoid the need for separate access
functions.

- Adrian Cox


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH][RFC]Updated MPC I2C driver

[Sylvain Munaut]

Matthew McClintock wrote:

 > The blocks look very similar, you might want to look at the follow
 > up patch I just sent that abstracts the read/writes. It might be as
 > simple as abstracting another read/write function for your
 > particular platform.
 >

Yes
I'd say they are identical in fact. I kind of forgot that readl/writel
does a endianness correction. So in fact this driver should work for
the MPC5200.

My only concern is about interrupts.
MPC5200 has two I2C port:
 * One is MBAR + 0x3D00
 * Second is MBAR + 0x3D40

At MBAR + 0x3D20 there is an interrupt control register for the I2C
ports ( shared, 4bits for the first port, 4bits for the second).
 It controls for what reason an I2C interrupt should be fired. I don't
know if there is a similar register on the other, I'll look up the
datasheets.


Sylvain Munaut


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH][RFC]Updated MPC I2C driver

[Adrian Cox]

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

On Thu, 2004-07-01 at 15:59, Matthew McClintock wrote:
> 	The driver works well on both an mpc8540ads and a sandpoint here.
> However, I just learned yesterday that i2c 32-bit read/writes on the
> 85xx will not always be guaranteed to work.

It seems that the MPC107 is happy with 8 bit accesses to the registers,
so here's another version of the patch. This also changes the time
calculations to use time_after().

This still leaves the interrupt handling register of the MPC5200
unsolved. I suggest introducing another flag for the MPC5200, and adding
a small piece of extra setup code.

- Adrian Cox
Humboldt Solutions Ltd.


[-- Attachment #2: mpc-i2c-3.patch --]
[-- Type: text/x-patch, Size: 13104 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/07/01 22:10:01+01:00 adrian@humboldt.co.uk
#   Switch to time_after for jiffies calculations
#
# drivers/i2c/busses/i2c-mpc.c
#   2004/07/01 22:09:50+01:00 adrian@humboldt.co.uk +3 -3
#   Switch to time_after for jiffies calculations
#
# ChangeSet
#   2004/07/01 22:04:45+01:00 adrian@humboldt.co.uk
#   Switch from readl/writel to readb/writeb for MPC85xx compatibility.
#
# drivers/i2c/busses/i2c-mpc.c
#   2004/07/01 22:04:32+01:00 adrian@humboldt.co.uk +17 -17
#   Switch from readl/writel to readb/writeb for MPC85xx compatibility.
#
# ChangeSet
#   2004/07/01 17:53:05+01:00 adrian@humboldt.co.uk
#   Add I2C driver for Motorola I2C
#
# drivers/i2c/busses/i2c-mpc.c
#   2004/07/01 17:52:55+01:00 adrian@humboldt.co.uk +410 -0
#
# drivers/i2c/busses/i2c-mpc.c
#   2004/07/01 17:52:55+01:00 adrian@humboldt.co.uk +0 -0
#   BitKeeper file /home/adrian/kernels/sa107-2.6/for-upstream/drivers/i2c/busses/i2c-mpc.c
#
# drivers/i2c/busses/Makefile
#   2004/07/01 17:52:55+01:00 adrian@humboldt.co.uk +1 -0
#   Add I2C driver for Motorola I2C
#
# drivers/i2c/busses/Kconfig
#   2004/07/01 17:52:54+01:00 adrian@humboldt.co.uk +11 -0
#   Add I2C driver for Motorola I2C
#
diff -Nru a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
--- a/drivers/i2c/busses/Kconfig	Thu Jul  1 22:20:58 2004
+++ b/drivers/i2c/busses/Kconfig	Thu Jul  1 22:20:58 2004
@@ -396,4 +396,15 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-voodoo3.

+config I2C_MPC
+	tristate "MPC107/824x/85xx"
+	depends on I2C && FSL_OCP
+	help
+	  If you say yes to this option, support will be included for the
+	  built-in I2C interface on the MPC107/Tsi107/MPC8240/MPC8245 and
+	  MPC85xx family processors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-mpc.
+
 endmenu
diff -Nru a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
--- a/drivers/i2c/busses/Makefile	Thu Jul  1 22:20:58 2004
+++ b/drivers/i2c/busses/Makefile	Thu Jul  1 22:20:58 2004
@@ -32,6 +32,7 @@
 obj-$(CONFIG_I2C_VOODOO3)	+= i2c-voodoo3.o
 obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
 obj-$(CONFIG_SCx200_I2C)	+= scx200_i2c.o
+obj-$(CONFIG_I2C_MPC)	+= i2c-mpc.o

 ifeq ($(CONFIG_I2C_DEBUG_BUS),y)
 EXTRA_CFLAGS += -DDEBUG
diff -Nru a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
--- /dev/null	Wed Dec 31 16:00:00 1969
+++ b/drivers/i2c/busses/i2c-mpc.c	Thu Jul  1 22:20:58 2004
@@ -0,0 +1,410 @@
+/*
+ * (C) Copyright 2003-2004
+ * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
+
+ * This is a combined i2c adapter and algorithm driver for the
+ * MPC107/Tsi107 PowerPC northbridge and processors that include
+ * the same I2C unit (8240, 8245, 85xx).
+ *
+ * Release 0.6
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <linux/config.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <asm/io.h>
+#include <asm/ocp.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+
+#ifdef CONFIG_I2C_DEBUG_BUS
+#define DPRINTK(args...) printk(KERN_DEBUG __FILE__": " args)
+#else
+#define DPRINTK(args...)
+#endif
+
+
+#define MPC_I2C_ADDR  0x00
+#define MPC_I2C_FDR 	0x04
+#define MPC_I2C_CR	0x08
+#define MPC_I2C_SR	0x0c
+#define MPC_I2C_DR	0x10
+#define MPC_I2C_DFSRR 0x14
+#define MPC_I2C_REGION 0x20
+
+#define CCR_MEN  0x80
+#define CCR_MIEN 0x40
+#define CCR_MSTA 0x20
+#define CCR_MTX  0x10
+#define CCR_TXAK 0x08
+#define CCR_RSTA 0x04
+
+#define CSR_MCF  0x80
+#define CSR_MAAS 0x40
+#define CSR_MBB  0x20
+#define CSR_MAL  0x10
+#define CSR_SRW  0x04
+#define CSR_MIF  0x02
+#define CSR_RXAK 0x01
+
+struct mpc_i2c {
+	char *base;
+	struct ocp_def *ocpdef;
+	u32 interrupt;
+	wait_queue_head_t queue;
+	struct i2c_adapter adap;
+};
+
+static __inline__ void writeccr(struct mpc_i2c *i2c, u32 x)
+{
+	writeb(x, i2c->base + MPC_I2C_CR);
+}
+
+static irqreturn_t mpc_i2c_isr(int irq, void *dev_id, struct pt_regs * regs)
+{
+	struct mpc_i2c *i2c = dev_id;
+	if (readb(i2c->base + MPC_I2C_SR) & CSR_MIF) {
+		/* Read again to allow register to stabilise */
+		i2c->interrupt = readb(i2c->base + MPC_I2C_SR);
+		writeb(0, i2c->base + MPC_I2C_SR);
+		wake_up_interruptible(&i2c->queue);
+	}
+	return IRQ_HANDLED;
+}
+
+static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
+{
+	DECLARE_WAITQUEUE(wait, current);
+        unsigned long orig_jiffies = jiffies;
+        u32 x;
+	int result = 0;
+
+	if (i2c->ocpdef->irq == OCP_IRQ_NA) {
+		while(! (readb(i2c->base + MPC_I2C_SR) & CSR_MIF)) {
+			schedule();
+			if (time_after(jiffies, orig_jiffies + timeout)) {
+				DPRINTK("I2C: timeout\n");
+				result = -EIO;
+				break;
+			}
+		}
+		x = readb(i2c->base + MPC_I2C_SR);
+		writeb(0, i2c->base + MPC_I2C_SR);
+	} else {
+		add_wait_queue(&i2c->queue, &wait);
+		while ( ! (i2c->interrupt & CSR_MIF)) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			if (signal_pending(current)) {
+				DPRINTK("I2C: Interrupted\n");
+				result = -EINTR;
+				break;
+			}
+			if (time_after(jiffies, orig_jiffies + timeout)) {
+				DPRINTK("I2C: timeout\n");
+				result = -EIO;
+				break;
+			}
+			schedule_timeout(timeout);
+		}
+		current->state = TASK_RUNNING;
+		remove_wait_queue(&i2c->queue, &wait);
+		x = i2c->interrupt;
+		i2c->interrupt = 0;
+	}
+
+	if (result < -0)
+	    return result;
+
+	if (! (x & CSR_MCF))
+	{
+	    DPRINTK("I2C: unfinished\n");
+	    return -EIO;
+	}
+
+        if (x & CSR_MAL)
+	{
+	    	DPRINTK("I2C: MAL\n");
+                return -EIO;
+	}
+
+        if (writing && (x & CSR_RXAK)) {
+                DPRINTK("I2C: No RXAK\n");
+                /* generate stop */
+                writeccr(i2c, CCR_MEN);
+                return -EIO;
+        }
+        return 0;
+}
+
+static void mpc_i2c_start(struct mpc_i2c *i2c)
+{
+	struct ocp_fs_i2c_data *i2c_data = i2c->ocpdef->additions;
+        /* Set clock and filters */
+	if (i2c_data && (i2c_data->flags & FS_I2C_SEPARATE_DFSRR)) {
+		writeb(0x31, i2c->base + MPC_I2C_FDR);
+		writeb(0x10, i2c->base + MPC_I2C_DFSRR);
+	} else
+		writel(0x1031, i2c->base + MPC_I2C_FDR);
+        /* Clear arbitration */
+        writeb(0, i2c->base + MPC_I2C_SR);
+       	/* Start with MEN */
+	writeccr(i2c, CCR_MEN);
+}
+
+static void mpc_i2c_stop(struct mpc_i2c *i2c)
+{
+	writeccr(i2c, CCR_MEN);
+}
+
+static int mpc_write(struct mpc_i2c *i2c, int target,
+			const u8 *data, int length, int restart)
+{
+        int i;
+        unsigned timeout = HZ;
+    	u32 flags = restart ? CCR_RSTA : 0;
+
+	/* Start with MEN */
+	if (! restart)
+		writeccr(i2c, CCR_MEN);
+	/* Start as master */
+	writeccr(i2c, CCR_MIEN | CCR_MEN |
+		      CCR_MSTA | CCR_MTX | flags);
+	/* Write target byte */
+	writeb((target << 1), i2c->base + MPC_I2C_DR);
+
+	if (i2c_wait(i2c, timeout, 1) < 0)
+	    return -1;
+
+        for(i = 0; i < length; i++) {
+	        /* Write data byte */
+	        writeb(data[i], i2c->base + MPC_I2C_DR);
+
+	        if (i2c_wait(i2c, timeout, 1) < 0)
+	            return -1;
+        }
+
+        return 0;
+}
+
+static int mpc_read(struct mpc_i2c *i2c, int target,
+		       u8 *data, int length, int restart)
+{
+    	unsigned timeout = HZ;
+    	int i;
+    	u32 flags = restart ? CCR_RSTA : 0;
+
+	/* Start with MEN */
+	if (! restart)
+		writeccr(i2c, CCR_MEN);
+	/* Switch to read - restart */
+	writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA |
+		      CCR_MTX | flags);
+	/* Write target address byte - this time with the read flag set */
+	writeb((target << 1) | 1, i2c->base + MPC_I2C_DR);
+
+	if (i2c_wait(i2c, timeout, 0) < 0)
+	    return -1;
+
+	if (length == 1)
+	    writeccr(i2c, CCR_MIEN| CCR_MEN |
+			  CCR_MSTA | CCR_TXAK);
+	else
+	    writeccr(i2c, CCR_MIEN| CCR_MEN | CCR_MSTA);
+    	/* Dummy read */
+	readb(i2c->base + MPC_I2C_DR);
+
+	for(i = 0; i < length; i++) {
+		if (i2c_wait(i2c, timeout, 0) < 0)
+			return -1;
+
+		/* Generate txack on next to last byte */
+		if (i == length - 2)
+			writeccr(i2c, CCR_MIEN | CCR_MEN |
+				      CCR_MSTA | CCR_TXAK);
+		/* Generate stop on last byte */
+		if (i == length - 1)
+			writeccr(i2c, CCR_MIEN | CCR_MEN |
+				      CCR_TXAK);
+		data[i] = readb(i2c->base + MPC_I2C_DR);
+	}
+
+	return length;
+}
+
+static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
+	                   int num)
+{
+	struct i2c_msg *pmsg;
+	int i;
+	int ret=0;
+        unsigned long orig_jiffies = jiffies;
+	struct mpc_i2c *i2c = i2c_get_adapdata(adap);
+
+        mpc_i2c_start(i2c);
+
+	/* Allow bus up to 1s to become not busy */
+	while(readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
+                if (signal_pending(current))
+		{
+			DPRINTK("I2C: Interrupted\n");
+                        return -EINTR;
+		}
+                if (time_after(jiffies, orig_jiffies + HZ))
+		{
+		    	printk("I2C: timeout\n");
+                        return -EIO;
+		}
+                schedule();
+        }
+
+	for (i = 0;ret >= 0 && i < num; i++) {
+		pmsg = &msgs[i];
+		DPRINTK("Doing %s %d bytes to 0x%02x - %d of %d messages\n",
+		     pmsg->flags & I2C_M_RD ? "read" : "write",
+                     pmsg->len, pmsg->addr, i + 1, num);
+		if (pmsg->flags & I2C_M_RD)
+                        ret = mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+                else
+                        ret = mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+        }
+        mpc_i2c_stop(i2c);
+        return (ret < 0) ? ret : num;
+}
+
+static u32 mpc_functionality(struct i2c_adapter *adap)
+{
+        return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm mpc_algo = {
+	.name = "MPC algorithm",
+	.id = I2C_ALGO_MPC107,
+	.master_xfer = mpc_xfer,
+	.functionality = mpc_functionality,
+};
+
+static struct i2c_adapter mpc_ops = {
+	.owner = THIS_MODULE,
+	.name = "MPC adapter",
+	.id = I2C_ALGO_MPC107 | I2C_HW_MPC107,
+	.algo = &mpc_algo,
+	.class = I2C_CLASS_HWMON,
+	.timeout = 1,
+	.retries = 1
+};
+
+static int __devinit mpc_i2c_probe(struct ocp_device *ocp)
+{
+	int result = 0;
+	struct mpc_i2c *i2c;
+
+	if (!(i2c = kmalloc(sizeof(*i2c), GFP_KERNEL))){
+		return -ENOMEM;
+	}
+	i2c->ocpdef = ocp->def;
+	init_waitqueue_head(&i2c->queue);
+
+	if (! request_mem_region(ocp->def->paddr, MPC_I2C_REGION,
+		                 "i2c-mpc")) {
+		printk(KERN_ERR "i2c-mpc - resource unavailable\n");
+		return -ENODEV;
+	}
+
+	i2c->base = ioremap(ocp->def->paddr, MPC_I2C_REGION);
+
+	if (! i2c->base) {
+		printk(KERN_ERR "i2c-mpc - failed to map controller\n");
+		result = -ENOMEM;
+		goto fail_map;
+	}
+
+	if (ocp->def->irq != OCP_IRQ_NA)
+		if ((result = request_irq(ocp->def->irq, mpc_i2c_isr,
+					  0, "i2c-mpc", i2c)) < 0) {
+			printk(KERN_ERR "i2c-mpc - failed to attach interrupt\n");
+			goto fail_irq;
+		}
+
+	i2c->adap = mpc_ops;
+	i2c_set_adapdata(&i2c->adap, i2c);
+	if ((result = i2c_add_adapter(&i2c->adap)) < 0) {
+		printk(KERN_ERR "i2c-mpc - failed to add adapter\n");
+		goto fail_add;
+	}
+	ocp_set_drvdata(ocp, i2c);
+	return result;
+
+fail_add:
+	if (ocp->def->irq != OCP_IRQ_NA)
+		free_irq(ocp->def->irq, 0);
+fail_irq:
+	iounmap(i2c->base);
+fail_map:
+	release_mem_region(ocp->def->paddr, MPC_I2C_REGION);
+	kfree(i2c);
+	return result;
+}
+static void __devexit mpc_i2c_remove(struct ocp_device *ocp)
+{
+	struct mpc_i2c *i2c = ocp_get_drvdata(ocp);
+	ocp_set_drvdata(ocp, NULL);
+	i2c_del_adapter(&i2c->adap);
+
+	if (ocp->def->irq != OCP_IRQ_NA)
+		free_irq(i2c->ocpdef->irq, i2c);
+	iounmap(i2c->base);
+	release_mem_region(i2c->ocpdef->paddr, MPC_I2C_REGION);
+	kfree(i2c);
+}
+
+static struct ocp_device_id mpc_iic_ids[] __devinitdata =
+{
+	{ .vendor = OCP_VENDOR_FREESCALE, .function = OCP_FUNC_IIC },
+	{ .vendor = OCP_VENDOR_INVALID }
+};
+
+MODULE_DEVICE_TABLE(ocp, mpc_iic_ids);
+
+static struct ocp_driver mpc_iic_driver =
+{
+	.name 		= "iic",
+	.id_table	= mpc_iic_ids,
+	.probe		= mpc_i2c_probe,
+	.remove		= __devexit_p(mpc_i2c_remove)
+};
+
+static int __init iic_init(void)
+{
+	return ocp_register_driver(&mpc_iic_driver);
+}
+
+static void __exit iic_exit(void)
+{
+	ocp_unregister_driver(&mpc_iic_driver);
+}
+
+module_init(iic_init);
+module_exit(iic_exit);
+
+
+MODULE_AUTHOR("Adrian Cox <adrian@humboldt.co.uk>");
+MODULE_DESCRIPTION("I2C-Bus adapter for MPC107 bridge and MPC824x/85xx processors");
+MODULE_LICENSE("GPL");

Re: [PATCH][RFC]Updated MPC I2C driver

[Sylvain Munaut]

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

Adrian Cox wrote:

>It seems that the MPC107 is happy with 8 bit accesses to the registers,
>so here's another version of the patch. This also changes the time
>calculations to use time_after().
>
>
The MPC5200 seems happy too ;) There is a eeprom on the board and it's
detected and I can access it. If I find a soldering iron, I'll hook up
another I2C device to test other devices.

Some gotchas though :

 - The FDR computation is completly different. Here is the code I
attached the code I used in my I2C driver. Basically, it takes as
argument the desired I2C clock rate ( was a module parameter ) and the
internal bus frequency and then compute the FDR.
 - During the scan, the first bus is scanned without devices and the
error when debug is active is I2C_MAL. From the moment the eeprom is
detected, the error code become I2C_NORXACK for the address wihout
device. Included dmesg

drivers/i2c/busses/i2c-mpc.c: Doing write 0 bytes to 0x57 - 1 of 1 messages
drivers/i2c/busses/i2c-mpc.c: I2C: MAL
i2c_adapter i2c-1: found normal i2c_range entry for adapter 1, addr 0050
i2c_adapter i2c-1: master_xfer: with 1 msgs.
drivers/i2c/busses/i2c-mpc.c: Doing write 0 bytes to 0x50 - 1 of 1 messages
i2c_adapter i2c-1: master_xfer: with 1 msgs.
drivers/i2c/busses/i2c-mpc.c: Doing write 0 bytes to 0x50 - 1 of 1 messages
i2c_adapter i2c-1: client [eeprom] registered to adapter
registering 1-0050
i2c_adapter i2c-1: found normal i2c_range entry for adapter 1, addr 0051
i2c_adapter i2c-1: master_xfer: with 1 msgs.
drivers/i2c/busses/i2c-mpc.c: Doing write 0 bytes to 0x51 - 1 of 1 messages
drivers/i2c/busses/i2c-mpc.c: I2C: No RXAK
i2c_adapter i2c-1: found normal i2c_range entry for adapter 1, addr 0052
i2c_adapter i2c-1: master_xfer: with 1 msgs.

 - The interrupt bit. BTW, on what event should the interrupt be fired ?
(I did my test without any interrupts )

>This still leaves the interrupt handling register of the MPC5200
>unsolved. I suggest introducing another flag for the MPC5200, and adding
>a small piece of extra setup code.
>
>
Yes, something like FS_I2C_IS_MPC52xx


Another remark : The register setup are done at every _start. Couldn't
they be done once for all during init ( just after the probe, call a
init_hardware functions ).



Sylvain Munaut

[-- Attachment #2: fdr_mpc5200.c --]
[-- Type: text/x-csrc, Size: 864 bytes --]

static int
mpc52xx_i2c_get_best_fdr(int ipb_freq, int i2c_speed)
{
	/* Consts */
	const struct { int scl2tap; int tap2tap; } x2taps[] = {
		{   4,   1 },
		{   4,   2 },
		{   6,   4 },
		{   6,   8 },
		{  14,  16 },
		{  30,  32 },
		{  62,  64 },
		{ 126, 128 },
	};
	const int scl_taps[] = { 9, 10, 12, 15, 5, 6, 7, 8 };

	int best_i, best_j, i, j;
	int scl;
	int best_diff = 0x7fffffff, diff;
	int fdr;

	for ( i=7 ; i>=0 ; i-- ) {
		for ( j=0 ; j<8 ; j++ ) {
			scl = 2 * (x2taps[i].scl2tap + ((scl_taps[j] - 1) * x2taps[i].tap2tap) + 2);
			/* We only want frequency BELOW or EQUAL to */
			/* the target frequency */
			diff = i2c_speed * scl - ipb_freq;
			if ( (diff > 0) && (diff < best_diff) ) {
				best_diff = diff;
				best_i = i;
				best_j = j;
			}
		}
	}

	fdr = ((best_i << 2) | (best_j & 0x03) | ((best_j & 0x04) << 5)) & 0x3f;
	return fdr;
}

Re: [PATCH][RFC]Updated MPC I2C driver

[Adrian Cox]

On Thu, 2004-07-01 at 23:32, Sylvain Munaut wrote:

>  - The FDR computation is completly different. Here is the code I
> attached the code I used in my I2C driver. Basically, it takes as
> argument the desired I2C clock rate ( was a module parameter ) and the
> internal bus frequency and then compute the FDR.

The 5200 seems to have a different FDR implementation. I could include
your calculation code if you can tell me where the value of ipb_freq
should come from.

I don't currently do a calculation on the 107, but pick a slow value for
safety. I'd need more platform specific knowledge to do the calculation,
as the 107/824x scale from the SDRAM clock, and the 85xx scales from the
CCB clock.

>  - During the scan, the first bus is scanned without devices and the
> error when debug is active is I2C_MAL. From the moment the eeprom is
> detected, the error code become I2C_NORXACK for the address wihout
> device. Included dmesg

Are you sure that it changes from MAL to NORXACK after seeing a device?
It looks to me that it returns MAL only on the bus without devices,
probably due to lack of pull-up resistors.

>  - The interrupt bit. BTW, on what event should the interrupt be fired ?
> (I did my test without any interrupts )

To use my driver, just set IE1 and IE2. The other flags are for a
different approach using DMA to load the TX and RX registers. That would
give a lower CPU overhead, but would also require a separate driver.

In fact, the reset value of this register is correct. The easiest fix
may be for platform code to ensure that the register has the correct
value, in case boot firmware changed it.

> Another remark : The register setup are done at every _start. Couldn't
> they be done once for all during init ( just after the probe, call a
> init_hardware functions ).

Yes, the frequency settings could be done once on startup. I might try
that.

- Adrian Cox
Humboldt Solutions Ltd.


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH][RFC]Updated MPC I2C driver

[Sylvain Munaut]

Hi

Adrian Cox wrote:

>The 5200 seems to have a different FDR implementation. I could include
>your calculation code if you can tell me where the value of ipb_freq
>should come from.
>
>
That would be nice.
The ipb_freq value is given by the boot loader or computed at run-time
if the boot loader don't give it.

#include <asm/ppcboot.h>
extern bd_t __res;
u32 ipbfreq = __res.bi_ipbfreq;

But this field will only exists when :
 - CONFIG_PPC_MPC52xx symbol is defined.
 - When the MPC52xx patch is applied to the kernel

Also, there is no DFSRR register on the 5200.

>I don't currently do a calculation on the 107, but pick a slow value for
>safety. I'd need more platform specific knowledge to do the calculation,
>as the 107/824x scale from the SDRAM clock, and the 85xx scales from the
>CCB clock.
>
>
Well yes, I guess that where the I2C modules takes it's clock from is
dependent of each platform.

>Are you sure that it changes from MAL to NORXACK after seeing a device?
>It looks to me that it returns MAL only on the bus without devices,
>probably due to lack of pull-up resistors.
>
>
Yes, indeed, looking at the schematics indicates that it's not even I2C
and it's disabled by the port multiplexing config. I've suppressed that
entry in my ocp definition.

>> - The interrupt bit. BTW, on what event should the interrupt be fired ?
>>(I did my test without any interrupts )
>>
>>
>
>To use my driver, just set IE1 and IE2. The other flags are for a
>different approach using DMA to load the TX and RX registers. That would
>give a lower CPU overhead, but would also require a separate driver.
>
>
>
Are you sure ? If I don't set the BNBE (Bus Not Busy Enable) bit, I just
get timeouts.

>In fact, the reset value of this register is correct. The easiest fix
>may be for platform code to ensure that the register has the correct
>value, in case boot firmware changed it.
>
>
Yes sure, that's the easiest way. It's just that I'd like to avoid it.
Especially when it's content is dependent on if the user has choosed to
use irq or not.
But It's sure is a pity that the register is shared between the two I2C
... Because even with a flag, the driver should be passed the address of
this register, and what bits to use.

>>Another remark : The register setup are done at every _start. Couldn't
>>they be done once for all during init ( just after the probe, call a
>>init_hardware functions ).
>>
>>
>
>Yes, the frequency settings could be done once on startup. I might try
>that.
>
>
Thanks.
Computing the FDR each time is a little overkill ;)


Sylvain Munaut

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH][RFC]Updated MPC I2C driver

[Adrian Cox]

It looks to me that supporting the 5200 will require a lot of small
changes.

On Fri, 2004-07-02 at 12:01, Sylvain Munaut wrote:

> #include <asm/ppcboot.h>
> extern bd_t __res;
> u32 ipbfreq = __res.bi_ipbfreq;
>
> But this field will only exists when :
>  - CONFIG_PPC_MPC52xx symbol is defined.
>  - When the MPC52xx patch is applied to the kernel

Maybe we should define two more fields in the ocp_fs_i2c_data structure:
one for base clock, and one for i2c clock. Then platform code could fill
in the clocks as necessary.

> Also, there is no DFSRR register on the 5200.

I noticed that. I don't think anybody ever used anything but the default
value on the other chips.

> >To use my driver, just set IE1 and IE2. The other flags are for a
> >different approach using DMA to load the TX and RX registers. That would
> >give a lower CPU overhead, but would also require a separate driver.

> Are you sure ? If I don't set the BNBE (Bus Not Busy Enable) bit, I just
> get timeouts.

>From the manual it looks as if setting BNBE might cause extra
interrupts, which the driver has no way to handle.  Could you try
enabling the interrupts, and see if this happens?

> Yes sure, that's the easiest way. It's just that I'd like to avoid it.
> Especially when it's content is dependent on if the user has choosed to
> use irq or not.
> But It's sure is a pity that the register is shared between the two I2C
> ... Because even with a flag, the driver should be passed the address of
> this register, and what bits to use.

How about putting a function pointer for platform interrupt enabling and
disabling into the ocp_fs_i2c_data?

- Adrian Cox
Humboldt Solutions Ltd.


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH][RFC]Updated MPC I2C driver

[Sylvain Munaut]

Hi

Adrian Cox wrote:

>On Fri, 2004-07-02 at 12:01, Sylvain Munaut wrote:
>
>
>
>>#include <asm/ppcboot.h>
>>extern bd_t __res;
>>u32 ipbfreq = __res.bi_ipbfreq;
>>
>>But this field will only exists when :
>> - CONFIG_PPC_MPC52xx symbol is defined.
>> - When the MPC52xx patch is applied to the kernel
>>
>>
>
>Maybe we should define two more fields in the ocp_fs_i2c_data structure:
>one for base clock, and one for i2c clock. Then platform code could fill
>in the clocks as necessary.
>
>
Yes, that's a good idea. It would need a flag to tell which clock
computation to take though.
Maybe instead of passing the baseclock, giving a pointer to a u32 that
contains the clock would be more appropriate. Since all board have
probably an int somewhere holding that value, you can put de definition
in the ocp_def without any further modifications.

>>Also, there is no DFSRR register on the 5200.
>>
>>
>
>I noticed that. I don't think anybody ever used anything but the default
>value on the other chips.
>
>
Yes. I've seen that the DFSRR is not as the same place. So maybe we
would need a two bits flag
like
#define FS_I2C_NO_DFSRR 0x00
#define FS_I2C_PACKED_DFSRR  0x02
#define FS_I2C_SEPARATE_DFSRR 0x04


>>Are you sure ? If I don't set the BNBE (Bus Not Busy Enable) bit, I just
>>get timeouts.
>>
>>
>
>>From the manual it looks as if setting BNBE might cause extra
>interrupts, which the driver has no way to handle.  Could you try
>enabling the interrupts, and see if this happens?
>
>
>
>>Yes sure, that's the easiest way. It's just that I'd like to avoid it.
>>Especially when it's content is dependent on if the user has choosed to
>>use irq or not.
>>But It's sure is a pity that the register is shared between the two I2C
>>... Because even with a flag, the driver should be passed the address of
>>this register, and what bits to use.
>>
>>
>
>How about putting a function pointer for platform interrupt enabling and
>disabling into the ocp_fs_i2c_data?
>
>
Well, you're right setting the BNBE is not right, it just hang because
interrupts are fired and not handled.
Now, here is the interesting part :
If I set the ocp_def to use both I2C controller, defining the I2C1
before I2C2, it works fine. Now if I just invert the entry, it does not.
Interrupts for I2C2 are never fired.
If I only put I2C1, it seems to work ( no timout). With only I2C2,
interrupts are never fired ...

I have no clue why ! The best option is just to set IRQ to OCP_IRQ_NA,
and it works fine, whatever the value of the shared interrupt registers is.

>It looks to me that supporting the 5200 will require a lot of small
>changes.
>
>
Well, for the interrupt yes that a quirk. The solution of dropping
interrupt completly for it is the best option for now.
For the DFSRR register, the solution mentionned above is clean, the 3
chips requires different implementation anyway.
For the clock thing, I think it's good to be able to set the I2C clock.


Sylvain Munaut


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/