Patchwork drivers/mtd/devices/spear_smi.c: use devm_ functions consistently

login
register
mail settings
Submitter Artem Bityutskiy
Date Aug. 24, 2012, 2:43 p.m.
Message ID <1345819407.2848.319.camel@sauron.fi.intel.com>
Download mbox | patch
Permalink /patch/179856/
State New
Headers show

Comments

Artem Bityutskiy - Aug. 24, 2012, 2:43 p.m.
On Fri, 2012-08-24 at 15:14 +0200, Stefan Roese wrote:
> On 08/24/2012 01:35 PM, Artem Bityutskiy wrote:
> >> @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev)
> >>  		ret = mtd_device_unregister(&flash->mtd);
> >>  		if (ret)
> >>  			dev_err(&pdev->dev, "error removing mtd\n");
> >> -
> >> -		iounmap(flash->base_addr);
> >> -		kfree(flash);
> >>  	}
> >>  
> >>  	irq = platform_get_irq(pdev, 0);
> >> -	free_irq(irq, dev);
> > 
> > I guess 'platform_get_irq()' should be killed as well? Stefan, this is
> > strange code - we get irq, without checking for error, and then free it?
> > What is the rationale?
> 
> Yes, this seems bogus. platform_get_irq() definitely should be removed
> from spear_smi_remove().

OK, thanks, I've massaged Julia's patch and pushed the following (also
attached) to l2-mtd.git, compile-tested only.

From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Sat, 4 Aug 2012 22:36:38 +0200
Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently

Use devm_kzalloc for all calls to kzalloc and not just the first.  Use devm
functions for other allocations as well.

Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to
where its result is passed to devm_request_and_ioremap to make the lack of
need for a NULL test more evident.

The semantic match that finds the inconsistency is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
@@

*devm_kzalloc(...)
...
*kzalloc(...)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/devices/spear_smi.c |   83 +++++++++------------------------------
 1 file changed, 18 insertions(+), 65 deletions(-)

Patch

From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Sat, 4 Aug 2012 22:36:38 +0200
Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently

Use devm_kzalloc for all calls to kzalloc and not just the first.  Use devm
functions for other allocations as well.

Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to
where its result is passed to devm_request_and_ioremap to make the lack of
need for a NULL test more evident.

The semantic match that finds the inconsistency is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
@@

*devm_kzalloc(...)
...
*kzalloc(...)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---

 drivers/mtd/devices/spear_smi.c |   83 +++++++++------------------------------
 1 file changed, 18 insertions(+), 65 deletions(-)

diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c

index 421bc65..dcc3c95 100644

--- a/drivers/mtd/devices/spear_smi.c

+++ b/drivers/mtd/devices/spear_smi.c

@@ -827,7 +827,7 @@  static int spear_smi_setup_banks(struct platform_device *pdev,

 	if (!flash_info)
 		return -ENODEV;
 
-	flash = kzalloc(sizeof(*flash), GFP_ATOMIC);

+	flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC);

 	if (!flash)
 		return -ENOMEM;
 	flash->bank = bank;
@@ -838,15 +838,13 @@  static int spear_smi_setup_banks(struct platform_device *pdev,

 	flash_index = spear_smi_probe_flash(dev, bank);
 	if (flash_index < 0) {
 		dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank);
-		ret = flash_index;

-		goto err_probe;

+		return flash_index;

 	}
 	/* map the memory for nor flash chip */
-	flash->base_addr = ioremap(flash_info->mem_base, flash_info->size);

-	if (!flash->base_addr) {

-		ret = -EIO;

-		goto err_probe;

-	}

+	flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base,

+					flash_info->size);

+	if (!flash->base_addr)

+		return -EIO;

 
 	dev->flash[bank] = flash;
 	flash->mtd.priv = dev;
@@ -888,17 +886,10 @@  static int spear_smi_setup_banks(struct platform_device *pdev,

 					count);
 	if (ret) {
 		dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret);
-		goto err_map;

+		return ret;

 	}
 
 	return 0;
-

-err_map:

-	iounmap(flash->base_addr);

-

-err_probe:

-	kfree(flash);

-	return ret;

 }
 
 /**
@@ -942,13 +933,6 @@  static int __devinit spear_smi_probe(struct platform_device *pdev)

 		}
 	}
 
-	smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);

-	if (!smi_base) {

-		ret = -ENODEV;

-		dev_err(&pdev->dev, "invalid smi base address\n");

-		goto err;

-	}

-

 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		ret = -ENODEV;
@@ -956,26 +940,20 @@  static int __devinit spear_smi_probe(struct platform_device *pdev)

 		goto err;
 	}
 
-	dev = kzalloc(sizeof(*dev), GFP_ATOMIC);

+	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC);

 	if (!dev) {
 		ret = -ENOMEM;
 		dev_err(&pdev->dev, "mem alloc fail\n");
 		goto err;
 	}
 
-	smi_base = request_mem_region(smi_base->start, resource_size(smi_base),

-			pdev->name);

-	if (!smi_base) {

-		ret = -EBUSY;

-		dev_err(&pdev->dev, "request mem region fail\n");

-		goto err_mem;

-	}

+	smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);

 
-	dev->io_base = ioremap(smi_base->start, resource_size(smi_base));

+	dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base);

 	if (!dev->io_base) {
 		ret = -EIO;
-		dev_err(&pdev->dev, "ioremap fail\n");

-		goto err_ioremap;

+		dev_err(&pdev->dev, "devm_request_and_ioremap fail\n");

+		goto err;

 	}
 
 	dev->pdev = pdev;
@@ -991,17 +969,18 @@  static int __devinit spear_smi_probe(struct platform_device *pdev)

 		dev->num_flashes = MAX_NUM_FLASH_CHIP;
 	}
 
-	dev->clk = clk_get(&pdev->dev, NULL);

+	dev->clk = devm_clk_get(&pdev->dev, NULL);

 	if (IS_ERR(dev->clk)) {
 		ret = PTR_ERR(dev->clk);
-		goto err_clk;

+		goto err;

 	}
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret)
-		goto err_clk_prepare_enable;

+		goto err;

 
-	ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev);

+	ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0,

+			       pdev->name, dev);

 	if (ret) {
 		dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n");
 		goto err_irq;
@@ -1024,18 +1003,9 @@  static int __devinit spear_smi_probe(struct platform_device *pdev)

 	return 0;
 
 err_bank_setup:
-	free_irq(irq, dev);

 	platform_set_drvdata(pdev, NULL);
 err_irq:
 	clk_disable_unprepare(dev->clk);
-err_clk_prepare_enable:

-	clk_put(dev->clk);

-err_clk:

-	iounmap(dev->io_base);

-err_ioremap:

-	release_mem_region(smi_base->start, resource_size(smi_base));

-err_mem:

-	kfree(dev);

 err:
 	return ret;
 }
@@ -1049,11 +1019,8 @@  err:

 static int __devexit spear_smi_remove(struct platform_device *pdev)
 {
 	struct spear_smi *dev;
-	struct spear_smi_plat_data *pdata;

 	struct spear_snor_flash *flash;
-	struct resource *smi_base;

-	int ret;

-	int i, irq;

+	int ret, i;

 
 	dev = platform_get_drvdata(pdev);
 	if (!dev) {
@@ -1061,8 +1028,6 @@  static int __devexit spear_smi_remove(struct platform_device *pdev)

 		return -ENODEV;
 	}
 
-	pdata = dev_get_platdata(&pdev->dev);

-

 	/* clean up for all nor flash */
 	for (i = 0; i < dev->num_flashes; i++) {
 		flash = dev->flash[i];
@@ -1073,21 +1038,9 @@  static int __devexit spear_smi_remove(struct platform_device *pdev)

 		ret = mtd_device_unregister(&flash->mtd);
 		if (ret)
 			dev_err(&pdev->dev, "error removing mtd\n");
-

-		iounmap(flash->base_addr);

-		kfree(flash);

 	}
 
-	irq = platform_get_irq(pdev, 0);

-	free_irq(irq, dev);

-

 	clk_disable_unprepare(dev->clk);
-	clk_put(dev->clk);

-	iounmap(dev->io_base);

-	kfree(dev);

-

-	smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);

-	release_mem_region(smi_base->start, resource_size(smi_base));

 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
-- 

1.7.10.4