Stephen Rothwell wrote: > Hi Sean, > > On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <[EMAIL PROTECTED]> 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 <[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"); @@ -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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev