i2c-ibm_iic driver

This patch allows the i2c-ibm_iic driver to be built either as an ocp driver or an of_platform driver. This allows it to run under the powerpc arch but maintains backward compatibility with the ppc arch.

Quoted 5 lines · Sean MacLennan

This patch allows the i2c-ibm_iic driver to be built either as an ocp driver or an of_platform driver. This allows it to run under the powerpc arch but maintains backward compatibility with the ppc arch.

The original patch had a "bug" in that it matched both device type and compatibility. Current practice is to remove the i2c device types from the device trees. So I have removed the device match from the match table.

Cheers,
Sean

Signed-off-by Sean MacLennan <smaclennan-Qtffpm9i2AVWk0Htik3J/[email protected]>

---

drivers/i2c/busses/Kconfig +0 -1

index c466c6c..e9e1493 100644
@@ -241,7 +241,6 @@ config I2C_PIIX4
 
 config I2C_IBM_IIC
 	tristate "IBM PPC 4xx on-chip I2C interface"
-	depends on IBM_OCP
 	help
 	  Say Y here if you want to use IIC peripheral found on 
 	  embedded IBM PPC 4xx based systems. 

drivers/i2c/busses/i2c-ibm_iic.c +115 -26

index 9b43ff7..ea1c769 100644
@@ -6,7 +6,10 @@
  * Copyright (c) 2003, 2004 Zultys Technologies.
  * Eugene Surovegin <[email protected]> or <[email protected]>
  *
- * Based on original work by 
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <smaclennan-Qtffpm9i2AVWk0Htik3J/[email protected]>
+ *
+ * Based on original work by
  * 	Ian DaSilva  <[email protected]>
  *      Armin Kuster <[email protected]>
  * 	Matt Porter  <[email protected]>

@@ -39,12 +42,17 @@
 #include <asm/io.h>
 #include <linux/i2c.h>
 #include <linux/i2c-id.h>
+
+#ifdef CONFIG_IBM_OCP
 #include <asm/ocp.h>
 #include <asm/ibm4xx.h>
+#else
+#include <linux/of_platform.h>
+#endif
 
 #include "i2c-ibm_iic.h"
 
-#define DRIVER_VERSION "2.1"
+#define DRIVER_VERSION "2.2"
 
 MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION);
 MODULE_LICENSE("GPL");

@@ -650,13 +658,14 @@ static inline u8 iic_clckdiv(unsigned int opb)
 	opb /= 1000000;
 
 	if (opb < 20 || opb > 150){
-		printk(KERN_CRIT "ibm-iic: invalid OPB clock frequency %u MHz\n",
+		printk(KERN_WARNING "ibm-iic: invalid OPB clock frequency %u MHz\n",
 			opb);
 		opb = opb < 20 ? 20 : 150;
 	}
 	return (u8)((opb + 9) / 10 - 1);
 }
 
+#ifdef CONFIG_IBM_OCP
 /*
  * Register single IIC interface
  */

@@ -672,14 +681,14 @@ static int __devinit iic_probe(struct ocp_device *ocp){
 			ocp->def->index);
 
 	if (!(dev = kzalloc(sizeof(*dev), GFP_KERNEL))) {
-		printk(KERN_CRIT "ibm-iic%d: failed to allocate device data\n",
+		printk(KERN_ERR "ibm-iic%d: failed to allocate device data\n",
 			ocp->def->index);
 		return -ENOMEM;
 	}
 
 	dev->idx = ocp->def->index;
 	ocp_set_drvdata(ocp, dev);
-	
+
 	if (!request_mem_region(ocp->def->paddr, sizeof(struct iic_regs),
 				"ibm_iic")) {
 		ret = -EBUSY;

@@ -687,12 +696,12 @@ static int __devinit iic_probe(struct ocp_device *ocp){
 	}
 
 	if (!(dev->vaddr = ioremap(ocp->def->paddr, sizeof(struct iic_regs)))){
-		printk(KERN_CRIT "ibm-iic%d: failed to ioremap device registers\n",
+		printk(KERN_ERR "ibm-iic%d: failed to ioremap device registers\n",
 			dev->idx);
 		ret = -ENXIO;
 		goto fail2;
 	}
-	
+
 	init_waitqueue_head(&dev->wq);
 
 	dev->irq = iic_force_poll ? -1 : ocp->def->irq;

@@ -702,29 +711,29 @@ static int __devinit iic_probe(struct ocp_device *ocp){
 		 */
 		iic_interrupt_mode(dev, 0);
 		if (request_irq(dev->irq, iic_handler, 0, "IBM IIC", dev)){
-			printk(KERN_ERR "ibm-iic%d: request_irq %d failed\n", 
+			printk(KERN_ERR "ibm-iic%d: request_irq %d failed\n",
 				dev->idx, dev->irq);
-			/* Fallback to the polling mode */	
+			/* Fallback to the polling mode */
 			dev->irq = -1;
 		}
 	}
-	
+
 	if (dev->irq < 0)
-		printk(KERN_WARNING "ibm-iic%d: using polling mode\n", 
+		printk(KERN_WARNING "ibm-iic%d: using polling mode\n",
 			dev->idx);
-		
+
 	/* Board specific settings */
 	dev->fast_mode = iic_force_fast ? 1 : (iic_data ? iic_data->fast_mode : 0);
-	
-	/* clckdiv is the same for *all* IIC interfaces, 
+
+	/* clckdiv is the same for *all* IIC interfaces,
 	 * but I'd rather make a copy than introduce another global. --ebs
 	 */
 	dev->clckdiv = iic_clckdiv(ocp_sys_info.opb_bus_freq);
 	DBG("%d: clckdiv = %d\n", dev->idx, dev->clckdiv);
-	
+
 	/* Initialize IIC interface */
 	iic_dev_init(dev);
-	
+
 	/* Register it with i2c layer */
 	adap = &dev->adap;
 	adap->dev.parent = &ocp->dev;

@@ -746,28 +755,28 @@ static int __devinit iic_probe(struct ocp_device *ocp){
 	adap->nr = dev->idx >= 0 ? dev->idx : 0;
 
 	if ((ret = i2c_add_numbered_adapter(adap)) < 0) {
-		printk(KERN_CRIT "ibm-iic%d: failed to register i2c adapter\n",
+		printk(KERN_ERR "ibm-iic%d: failed to register i2c adapter\n",
 			dev->idx);
 		goto fail;
 	}
-	
+
 	printk(KERN_INFO "ibm-iic%d: using %s mode\n", dev->idx,
 		dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
 
 	return 0;
 
-fail:	
+fail:
 	if (dev->irq >= 0){
 		iic_interrupt_mode(dev, 0);
 		free_irq(dev->irq, dev);
-	}	
+	}
 
 	iounmap(dev->vaddr);
-fail2:	
+fail2:
 	release_mem_region(ocp->def->paddr, sizeof(struct iic_regs));
 fail1:
 	ocp_set_drvdata(ocp, NULL);
-	kfree(dev);	
+	kfree(dev);
 	return ret;
 }
 

@@ -779,17 +788,17 @@ static void __devexit iic_remove(struct ocp_device *ocp)
 	struct ibm_iic_private* dev = (struct ibm_iic_private*)ocp_get_drvdata(ocp);
 	BUG_ON(dev == NULL);
 	if (i2c_del_adapter(&dev->adap)){
-		printk(KERN_CRIT "ibm-iic%d: failed to delete i2c adapter :(\n",
+		printk(KERN_ERR "ibm-iic%d: failed to delete i2c adapter :(\n",
 			dev->idx);
 		/* That's *very* bad, just shutdown IRQ ... */
 		if (dev->irq >= 0){
-		    iic_interrupt_mode(dev, 0);	
+		    iic_interrupt_mode(dev, 0);
 		    free_irq(dev->irq, dev);
 		    dev->irq = -1;
 		}
 	} else {
 		if (dev->irq >= 0){
-		    iic_interrupt_mode(dev, 0);	
+		    iic_interrupt_mode(dev, 0);
 		    free_irq(dev->irq, dev);
 		}
 		iounmap(dev->vaddr);

@@ -798,7 +807,7 @@ static void __devexit iic_remove(struct ocp_device *ocp)
 	}
 }
 
-static struct ocp_device_id ibm_iic_ids[] __devinitdata = 
+static struct ocp_device_id ibm_iic_ids[] __devinitdata =
 {
 	{ .vendor = OCP_VENDOR_IBM, .function = OCP_FUNC_IIC },
 	{ .vendor = OCP_VENDOR_INVALID }

@@ -831,3 +840,189 @@ static void __exit iic_exit(void)
 
 module_init(iic_init);
 module_exit(iic_exit);
+#else
+/*
+ * Register single IIC interface
+ */
+static int __devinit iic_probe(struct of_device *ofdev,
+			       const struct of_device_id *match)
+{
+	static int index = 0;
+	struct device_node *np = ofdev->node;
+	struct ibm_iic_private* dev;
+	struct i2c_adapter* adap;
+	const u32 *indexp, *freq;
+	int ret;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		printk(KERN_ERR "ibm-iic: failed to allocate device data\n");
+		return -ENOMEM;
+	}
+
+	/* This assumes we don't mix index and non-index entries. */
+	indexp = of_get_property(np, "index", NULL);
+	dev->idx = indexp ? *indexp : index++;
+
+	dev_set_drvdata(&ofdev->dev, dev);
+
+	dev->vaddr = of_iomap(np, 0);
+	if (dev->vaddr == NULL) {
+		printk(KERN_ERR "ibm-iic%d: failed to ioremap device registers\n",
+			dev->idx);
+		ret = -ENXIO;
+		goto fail1;
+	}
+
+	init_waitqueue_head(&dev->wq);
+
+	if (iic_force_poll)
+		dev->irq = NO_IRQ;
+	else {
+		dev->irq = irq_of_parse_and_map(np, 0);
+		if (dev->irq == NO_IRQ)
+			printk(KERN_ERR __FILE__ ": irq_of_parse_and_map failed\n");
+		else {
+			/* Disable interrupts until we finish initialization,
+			   assumes level-sensitive IRQ setup...
+			*/
+			iic_interrupt_mode(dev, 0);
+			if (request_irq(dev->irq, iic_handler, 0, "IBM IIC", dev)){
+				printk(KERN_ERR "ibm-iic%d: request_irq %d failed\n",
+				       dev->idx, dev->irq);
+				/* Fallback to the polling mode */
+				dev->irq = NO_IRQ;
+			}
+		}
+	}
+
+	if (dev->irq == NO_IRQ)
+		printk(KERN_WARNING "ibm-iic%d: using polling mode\n",
+			   dev->idx);
+
+	/* Board specific settings */
+	if (iic_force_fast || of_get_property(np, "fast-mode", NULL))
+		dev->fast_mode = 1;
+	else
+		dev->fast_mode = 0;
+
+	/* clckdiv is the same for *all* IIC interfaces, but I'd rather
+	 * make a copy than introduce another global. --ebs
+	 */
+	freq = of_get_property(np, "clock-frequency", NULL);
+	if (freq == NULL) {
+		freq = of_get_property(np->parent, "clock-frequency", NULL);
+		if (freq == NULL) {
+			printk(KERN_ERR "ibm-iic%d: Unable to get bus frequency\n",
+			       dev->idx);
+			ret = -EBUSY;
+			goto fail;
+		}
+	}
+

drivers/i2c/busses/i2c-ibm_iic.c +64 -0

+	adap->nr = dev->idx;
+
+	ret = i2c_add_numbered_adapter(adap);
+	if (ret  < 0) {
+		printk(KERN_ERR "ibm-iic%d: failed to register i2c adapter\n",
+			dev->idx);
+		goto fail;
+	}
+
+	printk(KERN_INFO "ibm-iic%d: using %s mode\n", dev->idx,
+		dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
+
+	return 0;
+
+fail:
+	if (dev->irq != NO_IRQ){
+		iic_interrupt_mode(dev, 0);
+		free_irq(dev->irq, dev);
+	}
+
+	iounmap(dev->vaddr);
+fail1:
+	dev_set_drvdata(&ofdev->dev, NULL);
+	kfree(dev);
+	return ret;
+}
+
+/*
+ * Cleanup initialized IIC interface
+ */
+static int __devexit iic_remove(struct of_device *ofdev)
+{
+	struct ibm_iic_private* dev = dev_get_drvdata(&ofdev->dev);
+
+	BUG_ON(dev == NULL);
+	if (i2c_del_adapter(&dev->adap)){
+		printk(KERN_ERR "ibm-iic%d: failed to delete i2c adapter :(\n",
+			dev->idx);
+		/* That's *very* bad, just shutdown IRQ ... */
+		if (dev->irq != NO_IRQ){
+		    iic_interrupt_mode(dev, 0);
+		    free_irq(dev->irq, dev);
+		    dev->irq = NO_IRQ;
+		}
+	} else {
+		if (dev->irq != NO_IRQ){
+		    iic_interrupt_mode(dev, 0);
+		    free_irq(dev->irq, dev);
+		}
+		iounmap(dev->vaddr);
+		kfree(dev);
+	}
+
+	return 0;
+}
+
+
+static const struct of_device_id ibm_iic_match[] =
+{
+	{ .compatible = "ibm,iic-405ex", },
+	{ .compatible = "ibm,iic-405gp", },
+	{ .compatible = "ibm,iic-440gp", },
+	{ .compatible = "ibm,iic-440gpx", },
+	{ .compatible = "ibm,iic-440grx", },

drivers/i2c/busses/i2c-ibm_iic.c +4 -0

+
+static int __init ibm_iic_init(void)
+{
+	printk(KERN_INFO "IBM IIC driver v" DRIVER_VERSION "\n");

Please split your patch into logical parts:
* Whitespace and coding-style cleanups
* Other cleanups (e.g. changing the log levels)
* Add OF support

Quoted 5 lines · Jean Delvare

Please split your patch into logical parts:
* Whitespace and coding-style cleanups
* Other cleanups (e.g. changing the log levels)
* Add OF support

Here is the first patch with everything except the OF support. Really all I did was change the log levels based on feedback from linxppc-dev.

Cheers,
Sean

Signed-off-by Sean MacLennan <[email protected]>

Here is the of platform patch. I removed the retries and removed the spaces used for spacing.

Cheers,
Sean

Signed-off-by Sean MacLennan <[email protected]>

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +3 -0

@@ -6,6 +6,9 @@
  * Copyright (c) 2003, 2004 Zultys Technologies.
  * Eugene Surovegin <[email protected]> or <[email protected]>
  *
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <[email protected]>
+ *
  * Based on original work by
  * 	Ian DaSilva  <[email protected]>
  *      Armin Kuster <[email protected]>

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +5 -1

 #include <asm/ocp.h>
 #include <asm/ibm4xx.h>
+#else
+#include <linux/of_platform.h>
+#endif
 
 #include "i2c-ibm_iic.h"
 
-#define DRIVER_VERSION "2.1"
+#define DRIVER_VERSION "2.2"
 
 MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION);
 MODULE_LICENSE("GPL");

@@ -657,6 +665,7 @@
 	return (u8)((opb + 9) / 10 - 1);
 }
 
+#ifdef CONFIG_IBM_OCP
 /*
  * Register single IIC interface
  */

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +32 -0

+
+	dev_set_drvdata(&ofdev->dev, dev);
+
+	dev->vaddr = of_iomap(np, 0);
+	if (dev->vaddr == NULL) {
+		printk(KERN_ERR "ibm-iic%d: failed to ioremap device registers\n",
+			dev->idx);
+		ret = -ENXIO;
+		goto fail1;
+	}
+
+	init_waitqueue_head(&dev->wq);
+
+	if (iic_force_poll)
+		dev->irq = NO_IRQ;
+	else {
+		dev->irq = irq_of_parse_and_map(np, 0);
+		if (dev->irq == NO_IRQ)
+			printk(KERN_ERR __FILE__ ": irq_of_parse_and_map failed\n");
+		else {
+			/* Disable interrupts until we finish initialization,
+			   assumes level-sensitive IRQ setup...
+			*/
+			iic_interrupt_mode(dev, 0);
+			if (request_irq(dev->irq, iic_handler, 0, "IBM IIC", dev)){
+				printk(KERN_ERR "ibm-iic%d: request_irq %d failed\n",
+				       dev->idx, dev->irq);
+				/* Fallback to the polling mode */
+				dev->irq = NO_IRQ;
+			}
+		}
+	}

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +15 -0

+
+	/* clckdiv is the same for *all* IIC interfaces, but I'd rather
+	 * make a copy than introduce another global. --ebs
+	 */
+	freq = of_get_property(np, "clock-frequency", NULL);
+	if (freq == NULL) {
+		freq = of_get_property(np->parent, "clock-frequency", NULL);
+		if (freq == NULL) {
+			printk(KERN_ERR "ibm-iic%d: Unable to get bus frequency\n",
+			       dev->idx);
+			ret = -EBUSY;
+			goto fail;
+		}
+	}
+

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +11 -0

+
+	ret = i2c_add_numbered_adapter(adap);
+	if (ret  < 0) {
+		printk(KERN_ERR "ibm-iic%d: failed to register i2c adapter\n",
+			dev->idx);
+		goto fail;
+	}
+
+	printk(KERN_INFO "ibm-iic%d: using %s mode\n", dev->idx,
+		dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
+

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +13 -0

+		kfree(dev);
+	}
+
+	return 0;
+}
+
+
+static const struct of_device_id ibm_iic_match[] =
+{
+	{ .compatible = "ibm,iic-405ex", },
+	{ .compatible = "ibm,iic-405gp", },
+	{ .compatible = "ibm,iic-440gp", },
+	{ .compatible = "ibm,iic-440gpx", },

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +4 -0

+};
+
+static int __init ibm_iic_init(void)
+{

Hi Sean,

Quoted 5 lines · Sean MacLennan

Here is the of platform patch. I removed the retries and removed the spaces used for spacing.

Cheers,
Sean

Quoted 2 lines

Signed-off-by Sean MacLennan <[email protected]>

First of all: please run scripts/checkpatch.pl on your patch and fix the reported errors. It tells me:
total: 10 errors, 5 warnings, 222 lines checked
which is definitely too much.

Quoted 5 lines

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +5 -0

+		return -ENOMEM;
+	}
+
+	/* This assumes we don't mix index and non-index entries. */
+	indexp = of_get_property(np, "index", NULL);

i2c-ibm_iic.c line 864

Quoted 2 lines

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +1 -0

+	dev->idx = indexp ? *indexp : index++;

I don't like this static index thing much. Can't you just make the "index" OF property mandatory? Mixing ways to number things can become very confusing. In particular as you are using dev->idx later to call i2c_add_numbered_adapter(), the caller is really supposed to know what they are doing with the bus numbers.

i2c-ibm_iic.c line 939

Quoted 3 lines

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +2 -0

+	adap->timeout = 1;
+	adap->nr = dev->idx;

Looks to me like the block above has much in common with the original probe function, maybe it would be worth sharing the code to make future maintenance easier?

Note that I cannot test the code so I am relying on the linuxppc-dev folks to test it

Quoted 9 lines · Jean Delvare

Hi Sean,

Quoted 5 lines · Sean MacLennan

Here is the of platform patch. I removed the retries and removed the spaces used for spacing.

Cheers,
Sean

Quoted 8 lines

Quoted 2 lines

Signed-off-by Sean MacLennan <[email protected]>

First of all: please run scripts/checkpatch.pl on your patch and fix the reported errors. It tells me:
total: 10 errors, 5 warnings, 222 lines checked
which is definitely too much.

Many of the errors/warnings are from the cut and paste of the original code. I realize this doesn't justify my new code, but does beg the question; should I also cleanup the original code? And how much? I am tempted to cleanup all but the #ifdef CONFIG_IBM_OCP part.

A few comments, everything else I agree with.

Quoted 5 lines

Quoted 5 lines

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +5 -0

+		return -ENOMEM;
+	}
+
+	/* This assumes we don't mix index and non-index entries. */
+	indexp = of_get_property(np, "index", NULL);

Quoted 9 lines

i2c-ibm_iic.c line 864

Quoted 2 lines

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +1 -0

+	dev->idx = indexp ? *indexp : index++;

I don't like this static index thing much. Can't you just make the "index" OF property mandatory? Mixing ways to number things can become very confusing. In particular as you are using dev->idx later to call i2c_add_numbered_adapter(), the caller is really supposed to know what they are doing with the bus numbers.

I also don't really like mixing the two methods, hence the comment. I did this so existing dts files, which don't currently have an index, would work as is. Maybe it would be better to add the index?

Without my patch, or one like it, you cannot currently use the IBM IIC in the arch/powerpc branch, so maybe now *is* the time to enforce an index?

So if nobody responds with a good reason not to, I will change the code to require the index to be set. Less testing for me ;)

Quoted 8 lines

i2c-ibm_iic.c line 939

Quoted 3 lines

i2c-ibm_iic.c renamed from old-i2c-ibm_iic.c +2 -0

+	adap->timeout = 1;
+	adap->nr = dev->idx;

Looks to me like the block above has much in common with the original probe function, maybe it would be worth sharing the code to make future maintenance easier?

It is my understanding the the arch/ppc branch is going away in the middle of the year. So I kept the of platform code separate and clean expecting the CONFIG_IBM_OCP block to be removed in the near future. So future maintenance should not be an issue.

Quoted 2 lines

Note that I cannot test the code so I am relying on the linuxppc-dev folks to test it

I can test the of platform code and the common code. I am not setup to test the OCP code, which is why I am loath to change it. I use the IBM IIC for access to an ad7414 thermal chip (currently not in the kernel, I am working on that too) and for reading an eeprom.

An updated version of the patch. This one should answer all of Jean's concerns.

Cheers,
Sean

Signed-off-by Sean MacLennan <[email protected]>

---

drivers/i2c/busses/i2c-ibm_iic.c renamed from drivers/i2c/busses/orig-i2c-ibm_iic.c +43 -1

@@ -6,6 +6,9 @@
  * Copyright (c) 2003, 2004 Zultys Technologies.
  * Eugene Surovegin <[email protected]> or <[email protected]>
  *
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <[email protected]>
+ *
  * Based on original work by
  * 	Ian DaSilva  <[email protected]>
  *      Armin Kuster <[email protected]>

@@ -39,12 +42,17 @@
 #include <asm/io.h>
 #include <linux/i2c.h>
 #include <linux/i2c-id.h>
+
+#ifdef CONFIG_IBM_OCP
 #include <asm/ocp.h>
 #include <asm/ibm4xx.h>
+#else
+#include <linux/of_platform.h>
+#endif
 
 #include "i2c-ibm_iic.h"
 
-#define DRIVER_VERSION "2.1"
+#define DRIVER_VERSION "2.2"
 
 MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION);
 MODULE_LICENSE("GPL");

@@ -657,6 +665,7 @@
 	return (u8)((opb + 9) / 10 - 1);
 }
 
+#ifdef CONFIG_IBM_OCP
 /*
  * Register single IIC interface
  */

@@ -828,5 +837,182 @@
 	ocp_unregister_driver(&ibm_iic_driver);
 }
 
+#else  /* !CONFIG_IBM_OCP */
+
+static int __devinit iic_request_irq(struct of_device *ofdev,
+				     struct ibm_iic_private *dev)
+{
+	struct device_node *np = ofdev->node;
+	int irq;
+
+	if (iic_force_poll)
+		return NO_IRQ;
+
+	irq = irq_of_parse_and_map(np, 0);
+	if (irq == NO_IRQ) {
+		dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n");
+		return NO_IRQ;
+	}
+
+	/* Disable interrupts until we finish initialization, assumes
+	 *  level-sensitive IRQ setup...
+	 */
+	iic_interrupt_mode(dev, 0);
+	if (request_irq(irq, iic_handler, 0, "IBM IIC", dev)) {
+		dev_err(&ofdev->dev, "request_irq %d failed\n", irq);
+		/* Fallback to the polling mode */
+		return NO_IRQ;
+	}
+
+	return irq;
+}
+
+/*
+ * Register single IIC interface
+ */

drivers/i2c/busses/i2c-ibm_iic.c renamed from drivers/i2c/busses/orig-i2c-ibm_iic.c +17 -0

+		goto error_cleanup;
+	}
+	dev->idx = *indexp;
+
+	dev->vaddr = of_iomap(np, 0);
+	if (dev->vaddr == NULL) {
+		dev_err(&ofdev->dev, "failed to iomap device\n");
+		ret = -ENXIO;
+		goto error_cleanup;
+	}
+
+	init_waitqueue_head(&dev->wq);
+
+	dev->irq = iic_request_irq(ofdev, dev);
+	if (dev->irq == NO_IRQ)
+		dev_warn(&ofdev->dev, "using polling mode\n");
+

drivers/i2c/busses/i2c-ibm_iic.c renamed from drivers/i2c/busses/orig-i2c-ibm_iic.c +18 -0

+			goto error_cleanup;
+		}
+	}
+
+	dev->clckdiv = iic_clckdiv(*freq);
+	dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv);
+
+	/* Initialize IIC interface */
+	iic_dev_init(dev);
+
+	/* Register it with i2c layer */
+	adap = &dev->adap;
+	adap->dev.parent = &ofdev->dev;
+	strlcpy(adap->name, "IBM IIC", sizeof(adap->name));
+	i2c_set_adapdata(adap, dev);
+	adap->id = I2C_HW_OCP;
+	adap->class = I2C_CLASS_HWMON;
+	adap->algo = &iic_algo;

drivers/i2c/busses/i2c-ibm_iic.c renamed from drivers/i2c/busses/orig-i2c-ibm_iic.c +67 -0

+		dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
+
+	return 0;
+
+error_cleanup:
+	if (dev->irq != NO_IRQ) {
+		iic_interrupt_mode(dev, 0);
+		free_irq(dev->irq, dev);
+	}
+
+	if (dev->vaddr)
+		iounmap(dev->vaddr);
+
+	dev_set_drvdata(&ofdev->dev, NULL);
+	kfree(dev);
+	return ret;
+}
+
+/*
+ * Cleanup initialized IIC interface
+ */
+static int __devexit iic_remove(struct of_device *ofdev)
+{
+	struct ibm_iic_private *dev = dev_get_drvdata(&ofdev->dev);
+
+	dev_set_drvdata(&ofdev->dev, NULL);
+
+	i2c_del_adapter(&dev->adap);
+
+	if (dev->irq != NO_IRQ) {
+		iic_interrupt_mode(dev, 0);
+		free_irq(dev->irq, dev);
+	}
+
+	iounmap(dev->vaddr);
+	kfree(dev);
+
+	return 0;
+}
+
+static const struct of_device_id ibm_iic_match[] = {
+	{ .compatible = "ibm,iic-405ex", },
+	{ .compatible = "ibm,iic-405gp", },
+	{ .compatible = "ibm,iic-440gp", },
+	{ .compatible = "ibm,iic-440gpx", },
+	{ .compatible = "ibm,iic-440grx", },
+	{}
+};
+
+static struct of_platform_driver ibm_iic_driver = {
+	.name	= "ibm-iic",
+	.match_table = ibm_iic_match,
+	.probe	= iic_probe,
+	.remove	= __devexit_p(iic_remove),
+};
+
+static int __init iic_init(void)
+{
+	return of_register_platform_driver(&ibm_iic_driver);
+}
+
+static void __exit iic_exit(void)
+{
+	of_unregister_platform_driver(&ibm_iic_driver);
+}
+#endif /* CONFIG_IBM_OCP */
+
 module_init(iic_init);
 module_exit(iic_exit);

drivers/i2c/busses/Kconfig +0 -0

index b61f56b..44c0984 100644

Quoted 6 lines

drivers/i2c/busses/Kconfig +0 -0

index b61f56b..44c0984 100644
@@ -244,7 +244,6 @@ config I2C_PIIX4
 

drivers/i2c/busses/Kconfig config I2C_PIIX4 · line 249

Quoted 6 lines

drivers/i2c/busses/Kconfig +0 -1

 config I2C_IBM_IIC
 	tristate "IBM PPC 4xx on-chip I2C interface"
-	depends on IBM_OCP
 	help
 	  Say Y here if you want to use IIC peripheral found on
 	  embedded IBM PPC 4xx based systems.

With this Kconfig change, "make menuconfig" lets me select the i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think that you want to restrict the build to PPC machines somehow, or at least make sure that either IBM_OCP or OF support is present.

Quoted 5 lines · Jean Delvare

With this Kconfig change, "make menuconfig" lets me select the i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think that you want to restrict the build to PPC machines somehow, or at least make sure that either IBM_OCP or OF support is present.

How about this:

-       depends on IBM_OCP
+       depends on 4xx

Review

Quoted 12 lines · Stefan Roese

Quoted 5 lines · Jean Delvare

With this Kconfig change, "make menuconfig" lets me select the i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think that you want to restrict the build to PPC machines somehow, or at least make sure that either IBM_OCP or OF support is present.

How about this:

(quoted diff) +1 -1

-       depends on IBM_OCP
+       depends on 4xx

I think we should allow it to be built on other platforms as well, as long as they have of_platform_device support.

The Axon south bridge used on IBMs QS21 blade probably has an ibm_iic, even though it's managed by the firmware and we probably don't want to use it at this time, someone could use the same chip in a new design and actually do that.

In general, I also like to make it possible to enable drivers just for the benefit of compile testing, even for stuff that you can't find in any existing HW configuration, so as long as it builds on a platform, I think we shouldn't forbid it:

Review

Quoted 2 lines

(quoted diff) +1 -1

-       depends on IBM_OCP
+       depends on IBM_OCP || PPC_MERGE

Quoted 2 lines

Quoted 2 lines

-       depends on IBM_OCP
+       depends on IBM_OCP || PPC_MERGE

not PPC_OF?  or even OF (give the sparc guys the opportunity to build it for us :-))?

Quoted 11 lines · Stefan Roese

Quoted 5 lines · Jean Delvare

With this Kconfig change, "make menuconfig" lets me select the i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think that you want to restrict the build to PPC machines somehow, or at least make sure that either IBM_OCP or OF support is present.

How about this:

-       depends on IBM_OCP
+       depends on 4xx

I think we should allow it to be built on other platforms as well, as long as they have of_platform_device support.

The Axon south bridge used on IBMs QS21 blade probably has an ibm_iic, even though it's managed by the firmware and we probably don't want to use it at this time, someone could use the same chip in a new design and actually do that.

In general, I also like to make it possible to enable drivers just for the benefit of compile testing, even for stuff that you can't find in any existing HW configuration, so as long as it builds on a platform, I think we shouldn't forbid it:

(quoted diff) +1 -1

-       depends on IBM_OCP
+       depends on IBM_OCP || PPC_MERGE

Signed-off-by Sean MacLennan <[email protected]>

drivers/i2c/busses/i2c-ibm_iic.c renamed from drivers/i2c/busses/orig-i2c-ibm_iic.c +3 -0

@@ -6,6 +6,9 @@
  * Copyright (c) 2003, 2004 Zultys Technologies.
  * Eugene Surovegin <[email protected]> or <[email protected]>
  *
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <[email protected]>
+ *
  * Based on original work by
  * 	Ian DaSilva  <[email protected]>
  *      Armin Kuster <[email protected]>
(...)