[PATCH] i2c-ibm_iic driver - new patch
Sean MacLennan
smaclennan at pikatech.com
Tue Jan 8 16:56:27 EST 2008
Stephen Rothwell wrote:
> Hi Sean,
>
> On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <smaclennan at pikatech.com> wrote:
>
>
> Please don't post patches as attachments.
>
Ok.
>
>> +static int __devinit iic_probe(struct of_device *ofdev,
>> + const struct of_device_id *match)
>>
>
> Indenting could be better.
>
Sorry. Since this kernel is in a "work" directory and not in
/usr/src/linux* my rules for deciding on the tab size didn't work :( I
tried to correct the tabbing.
> Please split the assignments from the tests. Here and elsewhere.
>
>
I made the changes in my code. I am trying to leave the original code as
much as possible.
>> + printk(KERN_CRIT "ibm-iic: failed to allocate device data\n");
>>
>
> I am not sure that these messages are necessary and, even if so, not KERN_CRIT.
>
>
What would you recommend then? KERN_ERR? These are cut and paste from
the original driver, so I left them alone. I will try KERN_ERR.
>> + }
>> + } else {
>> + if (dev->irq != NO_IRQ){
>> + iic_interrupt_mode(dev, 0);
>> + free_irq(dev->irq, dev);
>> + }
>> + iounmap(dev->vaddr);
>> + kfree(dev);
>>
>
> Should these last two be after the below brace?
>
>
I'm not really qualified to answer, but I will anyway ;) I assume the
original author is basically saying if he cannot delete the adapter, it
is unsafe to free the memory since the i2c code may still use it. If I
have read that right, then I agree.
Cheers,
Sean
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index c466c6c..e9e1493 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -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.
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c
b/drivers/i2c/busses/i2c-ibm_iic.c
index 9b43ff7..d218a3b 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -6,6 +6,9 @@
* Copyright (c) 2003, 2004 Zultys Technologies.
* Eugene Surovegin <eugene.surovegin at zultys.com> or <ebs at ebshome.net>
*
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <smaclennan at pikatech.com>
+ *
* Based on original work by
* Ian DaSilva <idasilva at mvista.com>
* Armin Kuster <akuster at mvista.com>
@@ -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,7 +681,7 @@ 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;
}
@@ -687,7 +696,7 @@ 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;
@@ -746,7 +755,7 @@ 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;
}
@@ -779,7 +788,7 @@ 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){
@@ -831,3 +840,186 @@ 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 if ((dev->irq = irq_of_parse_and_map(np, 0)) == 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;
+ }
+ }
+
+ dev->clckdiv = iic_clckdiv(*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 = &ofdev->dev;
+ strcpy(adap->name, "IBM IIC");
+ i2c_set_adapdata(adap, dev);
+ adap->id = I2C_HW_OCP;
+ adap->class = I2C_CLASS_HWMON;
+ adap->algo = &iic_algo;
+ adap->client_register = NULL;
+ adap->client_unregister = NULL;
+ adap->timeout = 1;
+ adap->retries = 1;
+ 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[] =
+{
+ { .type = "i2c", .compatible = "ibm,iic-405ex", },
+ { .type = "i2c", .compatible = "ibm,iic-405gp", },
+ { .type = "i2c", .compatible = "ibm,iic-440gp", },
+ { .type = "i2c", .compatible = "ibm,iic-440gpx", },
+ { .type = "i2c", .compatible = "ibm,iic-440grx", },
+ {}
+};
+
+static struct of_platform_driver ibm_iic_driver =
+{
+ .name = "ibm-iic",
+ .match_table = ibm_iic_match,
+ .probe = iic_probe,
+ .remove = iic_remove,
+};
+
+static int __init ibm_iic_init(void)
+{
+ printk(KERN_INFO "IBM IIC driver v" DRIVER_VERSION "\n");
+ return of_register_platform_driver(&ibm_iic_driver);
+}
+module_init(ibm_iic_init);
+
+static void __exit ibm_iic_exit(void)
+{
+ of_unregister_platform_driver(&ibm_iic_driver);
+}
+module_exit(ibm_iic_exit);
+#endif
More information about the Linuxppc-dev
mailing list