Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one

["Jon Smirl" <jonsmirl@gmail.com>]

On 6/11/08, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Tue, Jun 10, 2008 at 10:40:45PM -0400, Jon Smirl wrote:
>  > Convert i2c-mpc from a platform driver into an of_platform driver.
>  > This patch is much smaller since Jochen already added
>  > of_find_i2c_driver(). Versions of this have been posted before.
>  >
>  > Signed-ff-by: Jon Smirl <jonsmirl@gmail.com>
>
>
> Typo: Signed-off... (I'm curious, do such typos enforce resending the
>  patch?)


I just cut and pasted this version to get the comments. Next pass I
will send it using stgit which will add the right signed-off line and
fix the wrapping.


>
>  Tested-by: Wolfram Sang <w.sang@pengutronix.de>
>
>
>  >
>  > --
>  >
>  >  drivers/i2c/busses/i2c-mpc.c |  105 +++++++++++++++++++++++++-----------------
>  >  1 files changed, 63 insertions(+), 42 deletions(-)
>  >
>  >
>  > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
>  > index a076129..76d8091 100644
>  > --- a/drivers/i2c/busses/i2c-mpc.c
>  > +++ b/drivers/i2c/busses/i2c-mpc.c
>  > @@ -18,6 +18,8 @@
>  >  #include <linux/sched.h>
>  >  #include <linux/init.h>
>  >  #include <linux/platform_device.h>
>  > +#include <linux/of_platform.h>
>  > +#include <linux/of_i2c.h>
>  >
>  >  #include <asm/io.h>
>  >  #include <linux/fsl_devices.h>
>  > @@ -25,13 +27,13 @@
>  >  #include <linux/interrupt.h>
>  >  #include <linux/delay.h>
>  >
>  > -#define MPC_I2C_ADDR  0x00
>  > +#define DRV_NAME "mpc-i2c"
>  > +
>  >  #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
>  > @@ -315,102 +317,121 @@ static struct i2c_adapter mpc_ops = {
>  >       .timeout = 1,
>  >  };
>  >
>  > -static int fsl_i2c_probe(struct platform_device *pdev)
>  > +static int fsl_i2c_probe(struct of_device *op, const struct
>  > of_device_id *match)
>
>
> The above two lines have to be concatenated, otherwise the patch will
>  not apply. Also, __devinit?
>
>
>  >  {
>  >       int result = 0;
>  >       struct mpc_i2c *i2c;
>  > -     struct fsl_i2c_platform_data *pdata;
>  > -     struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  > -
>  > -     pdata = (struct fsl_i2c_platform_data *) pdev->dev.platform_data;
>  >
>  >       i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
>  >       if (!i2c)
>  >               return -ENOMEM;
>  >
>  > -     i2c->irq = platform_get_irq(pdev, 0);
>  > -     if (i2c->irq < 0)
>  > -             i2c->irq = NO_IRQ; /* Use polling */
>  > +     if (of_get_property(op->node, "dfsrr", NULL))
>  > +             i2c->flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
>  >
>  > -     i2c->flags = pdata->device_flags;
>  > -     init_waitqueue_head(&i2c->queue);
>  > +     if (of_device_is_compatible(op->node, "mpc5200-i2c"))
>  > +             i2c->flags |= FSL_I2C_DEV_CLOCK_5200;
>  >
>  > -     i2c->base = ioremap((phys_addr_t)r->start, MPC_I2C_REGION);
>  > +     init_waitqueue_head(&i2c->queue);
>  >
>  > +     i2c->base = of_iomap(op->node, 0);
>  >       if (!i2c->base) {
>  >               printk(KERN_ERR "i2c-mpc - failed to map controller\n");
>  >               result = -ENOMEM;
>  >               goto fail_map;
>  >       }
>  >
>  > -     if (i2c->irq != NO_IRQ)
>  > -             if ((result = request_irq(i2c->irq, mpc_i2c_isr,
>  > -                                       IRQF_SHARED, "i2c-mpc", i2c)) < 0) {
>  > -                     printk(KERN_ERR
>  > -                            "i2c-mpc - failed to attach interrupt\n");
>  > -                     goto fail_irq;
>  > -             }
>  > +     i2c->irq = irq_of_parse_and_map(op->node, 0);
>  > +     if (i2c->irq == NO_IRQ) {
>  > +             result = -ENXIO;
>
>
> Minor thing, but I wonder if -EINVAL is more appropriate?
>
>
>  > +             goto fail_irq;
>  > +     }
>  > +
>  > +     result = request_irq(i2c->irq, mpc_i2c_isr,
>  > +                                             IRQF_SHARED, "i2c-mpc", i2c);
>  > +     if (result < 0) {
>  > +             printk(KERN_ERR "i2c-mpc - failed to attach interrupt\n");
>  > +             goto fail_request;
>  > +     }
>  >
>  >       mpc_i2c_setclock(i2c);
>  > -     platform_set_drvdata(pdev, i2c);
>  > +
>  > +     dev_set_drvdata(&op->dev, i2c);
>  >
>  >       i2c->adap = mpc_ops;
>  > -     i2c->adap.nr = pdev->id;
>  >       i2c_set_adapdata(&i2c->adap, i2c);
>  > -     i2c->adap.dev.parent = &pdev->dev;
>  > -     if ((result = i2c_add_numbered_adapter(&i2c->adap)) < 0) {
>  > +     i2c->adap.dev.parent = &op->dev;
>  > +
>  > +     result = i2c_add_adapter(&i2c->adap);
>  > +     if (result < 0) {
>  >               printk(KERN_ERR "i2c-mpc - failed to add adapter\n");
>  >               goto fail_add;
>  >       }
>  > +     of_register_i2c_devices(&i2c->adap, op->node);
>  >
>  >       return result;
>  >
>  > -      fail_add:
>  > -     if (i2c->irq != NO_IRQ)
>  > -             free_irq(i2c->irq, i2c);
>  > -      fail_irq:
>  > + fail_add:
>  > +     dev_set_drvdata(&op->dev, NULL);
>  > +     free_irq(i2c->irq, i2c);
>  > + fail_request:
>  > +     irq_dispose_mapping(i2c->irq);
>  > + fail_irq:
>  >       iounmap(i2c->base);
>  > -      fail_map:
>  > + fail_map:
>  >       kfree(i2c);
>  >       return result;
>  >  };
>  >
>  > -static int fsl_i2c_remove(struct platform_device *pdev)
>  > +static int fsl_i2c_remove(struct of_device *op)
>
>
> __devexit?
>
>
>  >  {
>  > -     struct mpc_i2c *i2c = platform_get_drvdata(pdev);
>  > +     struct mpc_i2c *i2c = dev_get_drvdata(&op->dev);
>  >
>  >       i2c_del_adapter(&i2c->adap);
>  > -     platform_set_drvdata(pdev, NULL);
>  > +     dev_set_drvdata(&op->dev, NULL);
>  >
>  >       if (i2c->irq != NO_IRQ)
>  >               free_irq(i2c->irq, i2c);
>  >
>  > +     irq_dispose_mapping(i2c->irq);
>  >       iounmap(i2c->base);
>  >       kfree(i2c);
>  >       return 0;
>  >  };
>  >
>  > -/* work with hotplug and coldplug */
>  > -MODULE_ALIAS("platform:fsl-i2c");
>  > +static const struct of_device_id mpc_i2c_of_match[] = {
>  > +     {
>  > +             .compatible     = "fsl-i2c",
>  > +     },
>  > +     {},
>  > +};
>  > +MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
>  > +
>  >
>  >  /* Structure for a device driver */
>  > -static struct platform_driver fsl_i2c_driver = {
>  > -     .probe = fsl_i2c_probe,
>  > -     .remove = fsl_i2c_remove,
>  > -     .driver = {
>  > -             .owner = THIS_MODULE,
>  > -             .name = "fsl-i2c",
>  > +static struct of_platform_driver mpc_i2c_driver = {
>  > +     .match_table    = mpc_i2c_of_match,
>  > +     .probe          = fsl_i2c_probe,
>  > +     .remove         = __devexit_p(fsl_i2c_remove),
>  > +     .driver         = {
>  > +             .owner  = THIS_MODULE,
>  > +             .name   = DRV_NAME,
>  >       },
>  >  };
>  >
>  >  static int __init fsl_i2c_init(void)
>  >  {
>  > -     return platform_driver_register(&fsl_i2c_driver);
>  > +     int rv;
>  > +
>  > +     rv = of_register_platform_driver(&mpc_i2c_driver);
>  > +     if (rv)
>  > +             printk(KERN_ERR DRV_NAME " of_register_platform_driver failed (%i)\n", rv);
>  > +     return rv;
>  >  }
>  >
>  >  static void __exit fsl_i2c_exit(void)
>  >  {
>  > -     platform_driver_unregister(&fsl_i2c_driver);
>  > +     of_unregister_platform_driver(&mpc_i2c_driver);
>  >  }
>  >
>  >  module_init(fsl_i2c_init);
>  >
>
> > _______________________________________________
>  > i2c mailing list
>  > i2c@lm-sensors.org
>  > http://lists.lm-sensors.org/mailman/listinfo/i2c
>
>  Kind regards,
>
>    Wolfram
>
>
>  --
>   Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
>   Pengutronix - Linux Solutions for Science and Industry
>
> -----BEGIN PGP SIGNATURE-----
>  Version: GnuPG v1.4.6 (GNU/Linux)
>
>  iD8DBQFIT/awD27XaX1/VRsRAoPrAJ0eWru6TJizVxfksnhS3KjZ7WHhmACcCbXJ
>  YsAUtKZSxlJAV5r/G22BlaE=
>  =YX24
>  -----END PGP SIGNATURE-----
>
>
>


-- 
Jon Smirl
jonsmirl@gmail.com