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

[Wolfram Sang <w.sang@pengutronix.de>]

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

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?)

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]