linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] drop frees of devm_ alloc'd data
@ 2012-09-01 16:33 Julia Lawall
  2012-09-01 16:33 ` [PATCH 1/4] drivers/spi/spi-sh-hspi.c: " Julia Lawall
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Julia Lawall @ 2012-09-01 16:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: kernel-janitors

Drop frees of devm_ alloc'd data.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] drivers/spi/spi-sh-hspi.c: drop frees of devm_ alloc'd data
  2012-09-01 16:33 [PATCH 0/4] drop frees of devm_ alloc'd data Julia Lawall
@ 2012-09-01 16:33 ` Julia Lawall
  2012-09-05 23:43   ` Mark Brown
  2012-09-01 16:33 ` [PATCH 2/4] drivers/tty/serial/sirfsoc_uart.c: " Julia Lawall
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2012-09-01 16:33 UTC (permalink / raw)
  To: Grant Likely, broonie; +Cc: kernel-janitors, spi-devel-general, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

devm free functions should not have to be explicitly used.

A semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/spi/spi-sh-hspi.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sh-hspi.c b/drivers/spi/spi-sh-hspi.c
index 934138c..796c077 100644
--- a/drivers/spi/spi-sh-hspi.c
+++ b/drivers/spi/spi-sh-hspi.c
@@ -283,7 +283,7 @@ static int __devinit hspi_probe(struct platform_device *pdev)
 	ret = spi_register_master(master);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "spi_register_master error.\n");
-		goto error2;
+		goto error1;
 	}
 
 	pm_runtime_enable(&pdev->dev);
@@ -292,8 +292,6 @@ static int __devinit hspi_probe(struct platform_device *pdev)
 
 	return 0;
 
- error2:
-	devm_iounmap(hspi->dev, hspi->addr);
  error1:
 	clk_put(clk);
  error0:
@@ -310,7 +308,6 @@ static int __devexit hspi_remove(struct platform_device *pdev)
 
 	clk_put(hspi->clk);
 	spi_unregister_master(hspi->master);
-	devm_iounmap(hspi->dev, hspi->addr);
 
 	return 0;
 }


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] drivers/tty/serial/sirfsoc_uart.c: drop frees of devm_ alloc'd data
  2012-09-01 16:33 [PATCH 0/4] drop frees of devm_ alloc'd data Julia Lawall
  2012-09-01 16:33 ` [PATCH 1/4] drivers/spi/spi-sh-hspi.c: " Julia Lawall
@ 2012-09-01 16:33 ` Julia Lawall
  2012-09-01 16:33 ` [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups Julia Lawall
  2012-09-01 16:33 ` [PATCH 3/4] drivers/mtd/maps/autcpu12-nvram.c: drop frees of devm_ alloc'd data Julia Lawall
  3 siblings, 0 replies; 11+ messages in thread
From: Julia Lawall @ 2012-09-01 16:33 UTC (permalink / raw)
  To: Alan Cox; +Cc: kernel-janitors, Greg Kroah-Hartman, linux-serial, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

devm free functions should not have to be explicitly used.

A semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 5b3eda2..a9e2bd1 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -668,7 +668,7 @@ int sirfsoc_uart_probe(struct platform_device *pdev)
 	if (res == NULL) {
 		dev_err(&pdev->dev, "Insufficient resources.\n");
 		ret = -EFAULT;
-		goto irq_err;
+		goto err;
 	}
 	port->irq = res->start;
 
@@ -676,7 +676,7 @@ int sirfsoc_uart_probe(struct platform_device *pdev)
 		sirfport->p = pinctrl_get_select_default(&pdev->dev);
 		ret = IS_ERR(sirfport->p);
 		if (ret)
-			goto pin_err;
+			goto err;
 	}
 
 	port->ops = &sirfsoc_uart_ops;
@@ -695,9 +695,6 @@ port_err:
 	platform_set_drvdata(pdev, NULL);
 	if (sirfport->hw_flow_ctrl)
 		pinctrl_put(sirfport->p);
-pin_err:
-irq_err:
-	devm_iounmap(&pdev->dev, port->membase);
 err:
 	return ret;
 }
@@ -709,7 +706,6 @@ static int sirfsoc_uart_remove(struct platform_device *pdev)
 	platform_set_drvdata(pdev, NULL);
 	if (sirfport->hw_flow_ctrl)
 		pinctrl_put(sirfport->p);
-	devm_iounmap(&pdev->dev, port->membase);
 	uart_remove_one_port(&sirfsoc_uart_drv, port);
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
  2012-09-01 16:33 [PATCH 0/4] drop frees of devm_ alloc'd data Julia Lawall
  2012-09-01 16:33 ` [PATCH 1/4] drivers/spi/spi-sh-hspi.c: " Julia Lawall
  2012-09-01 16:33 ` [PATCH 2/4] drivers/tty/serial/sirfsoc_uart.c: " Julia Lawall
@ 2012-09-01 16:33 ` Julia Lawall
  2012-09-04  8:42   ` Artem Bityutskiy
  2012-09-01 16:33 ` [PATCH 3/4] drivers/mtd/maps/autcpu12-nvram.c: drop frees of devm_ alloc'd data Julia Lawall
  3 siblings, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2012-09-01 16:33 UTC (permalink / raw)
  To: David Woodhouse; +Cc: kernel-janitors, linux-mtd, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

devm free functions should not have to be explicitly used.

The only thing left that is useful in the function mpc5121_nfc_free is the
call to clk_disable, which is moved to the call sites.

This function also incorrectly called iounmap on devm_ioremap allocated
data.

Use devm_request_and_ioremap in place of devm_request_mem_region and
devm_ioremap.

Use devm_clk_get.

A semantic match that finds the first problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/mtd/nand/mpc5121_nfc.c |   35 +++++------------------------------
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
index c259c24..45183ba 100644
--- a/drivers/mtd/nand/mpc5121_nfc.c
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -632,21 +632,6 @@ out:
 	return ret;
 }
 
-/* Free driver resources */
-static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
-{
-	struct nand_chip *chip = mtd->priv;
-	struct mpc5121_nfc_prv *prv = chip->priv;
-
-	if (prv->clk) {
-		clk_disable(prv->clk);
-		clk_put(prv->clk);
-	}
-
-	if (prv->csreg)
-		iounmap(prv->csreg);
-}
-
 static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 {
 	struct device_node *rootnode, *dn = op->dev.of_node;
@@ -713,12 +698,7 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 	regs_paddr = res.start;
 	regs_size = resource_size(&res);
 
-	if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
-		dev_err(dev, "Error requesting memory region!\n");
-		return -EBUSY;
-	}
-
-	prv->regs = devm_ioremap(dev, regs_paddr, regs_size);
+	prv->regs = devm_request_and_ioremap(dev, &res);
 	if (!prv->regs) {
 		dev_err(dev, "Error mapping memory region!\n");
 		return -ENOMEM;
@@ -752,11 +732,10 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 	of_node_put(rootnode);
 
 	/* Enable NFC clock */
-	prv->clk = clk_get(dev, "nfc_clk");
+	prv->clk = devm_clk_get(dev, "nfc_clk");
 	if (IS_ERR(prv->clk)) {
 		dev_err(dev, "Unable to acquire NFC clock!\n");
-		retval = PTR_ERR(prv->clk);
-		goto error;
+		return PTR_ERR(prv->clk);
 	}
 
 	clk_enable(prv->clk);
@@ -803,7 +782,6 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 	/* Detect NAND chips */
 	if (nand_scan(mtd, be32_to_cpup(chips_no))) {
 		dev_err(dev, "NAND Flash not found !\n");
-		devm_free_irq(dev, prv->irq, mtd);
 		retval = -ENXIO;
 		goto error;
 	}
@@ -828,7 +806,6 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 
 	default:
 		dev_err(dev, "Unsupported NAND flash!\n");
-		devm_free_irq(dev, prv->irq, mtd);
 		retval = -ENXIO;
 		goto error;
 	}
@@ -839,13 +816,12 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 	retval = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
 	if (retval) {
 		dev_err(dev, "Error adding MTD device!\n");
-		devm_free_irq(dev, prv->irq, mtd);
 		goto error;
 	}
 
 	return 0;
 error:
-	mpc5121_nfc_free(dev, mtd);
+	clk_disable(prv->clk);
 	return retval;
 }
 
@@ -857,8 +833,7 @@ static int __devexit mpc5121_nfc_remove(struct platform_device *op)
 	struct mpc5121_nfc_prv *prv = chip->priv;
 
 	nand_release(mtd);
-	devm_free_irq(dev, prv->irq, mtd);
-	mpc5121_nfc_free(dev, mtd);
+	clk_disable(prv->clk);
 
 	return 0;
 }


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] drivers/mtd/maps/autcpu12-nvram.c: drop frees of devm_ alloc'd data
  2012-09-01 16:33 [PATCH 0/4] drop frees of devm_ alloc'd data Julia Lawall
                   ` (2 preceding siblings ...)
  2012-09-01 16:33 ` [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups Julia Lawall
@ 2012-09-01 16:33 ` Julia Lawall
  2012-09-04  7:50   ` Artem Bityutskiy
  3 siblings, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2012-09-01 16:33 UTC (permalink / raw)
  To: David Woodhouse; +Cc: kernel-janitors, linux-mtd, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

devm free functions should not have to be explicitly used.

A semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/mtd/maps/autcpu12-nvram.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/maps/autcpu12-nvram.c b/drivers/mtd/maps/autcpu12-nvram.c
index ef420d9..76fb594 100644
--- a/drivers/mtd/maps/autcpu12-nvram.c
+++ b/drivers/mtd/maps/autcpu12-nvram.c
@@ -38,7 +38,6 @@ static int __devinit autcpu12_nvram_probe(struct platform_device *pdev)
 	map_word tmp, save0, save1;
 	struct resource *res;
 	struct autcpu12_nvram_priv *priv;
-	int err;
 
 	priv = devm_kzalloc(&pdev->dev,
 			    sizeof(struct autcpu12_nvram_priv), GFP_KERNEL);
@@ -50,8 +49,7 @@ static int __devinit autcpu12_nvram_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "failed to get memory resource\n");
-		err = -ENOENT;
-		goto out;
+		return -ENOENT;
 	}
 
 	priv->map.bankwidth	= 4;
@@ -61,8 +59,7 @@ static int __devinit autcpu12_nvram_probe(struct platform_device *pdev)
 	strcpy((char *)priv->map.name, res->name);
 	if (!priv->map.virt) {
 		dev_err(&pdev->dev, "failed to remap mem resource\n");
-		err = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 
 	simple_map_init(&priv->map);
@@ -90,8 +87,7 @@ static int __devinit autcpu12_nvram_probe(struct platform_device *pdev)
 	priv->mtd = do_map_probe("map_ram", &priv->map);
 	if (!priv->mtd) {
 		dev_err(&pdev->dev, "probing failed\n");
-		err = -ENXIO;
-		goto out;
+		return -ENXIO;
 	}
 
 	priv->mtd->owner	= THIS_MODULE;
@@ -106,12 +102,7 @@ static int __devinit autcpu12_nvram_probe(struct platform_device *pdev)
 
 	map_destroy(priv->mtd);
 	dev_err(&pdev->dev, "NV-RAM device addition failed\n");
-	err = -ENOMEM;
-
-out:
-	devm_kfree(&pdev->dev, priv);
-
-	return err;
+	return -ENOMEM;
 }
 
 static int __devexit autcpu12_nvram_remove(struct platform_device *pdev)
@@ -120,7 +111,6 @@ static int __devexit autcpu12_nvram_remove(struct platform_device *pdev)
 
 	mtd_device_unregister(priv->mtd);
 	map_destroy(priv->mtd);
-	devm_kfree(&pdev->dev, priv);
 
 	return 0;
 }


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] drivers/mtd/maps/autcpu12-nvram.c: drop frees of devm_ alloc'd data
  2012-09-01 16:33 ` [PATCH 3/4] drivers/mtd/maps/autcpu12-nvram.c: drop frees of devm_ alloc'd data Julia Lawall
@ 2012-09-04  7:50   ` Artem Bityutskiy
  0 siblings, 0 replies; 11+ messages in thread
From: Artem Bityutskiy @ 2012-09-04  7:50 UTC (permalink / raw)
  To: Julia Lawall; +Cc: David Woodhouse, kernel-janitors, linux-mtd, linux-kernel

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

On Sat, 2012-09-01 at 18:33 +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> devm free functions should not have to be explicitly used.
> 
> A semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)

Pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
  2012-09-01 16:33 ` [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups Julia Lawall
@ 2012-09-04  8:42   ` Artem Bityutskiy
  2012-09-04  8:53     ` Lars-Peter Clausen
  0 siblings, 1 reply; 11+ messages in thread
From: Artem Bityutskiy @ 2012-09-04  8:42 UTC (permalink / raw)
  To: Julia Lawall, linuxppc-dev
  Cc: David Woodhouse, kernel-janitors, linux-mtd, linux-kernel

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

Aiaiai! :-) [1] [2]

I've build-tested this using aiaiai and it reports that this change breaks the build:

dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < ~/tmp/julia2.mbox 
Tested the patch(es) on top of the following commits:
ba64756 Quick fixes - applied by aiaiai
651c6fa JFFS2: don't fail on bitflips in OOB
e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
ea9d312 mtd: cmdlinepart: minor cleanups

--------------------------------------------------------------------------------
Failed to build the following commit for configuration "powerpc-mpc512x_defconfig" (architecture powerpc)":

0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

...
drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set but not used [-Wunused-but-set-variable]
drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set but not used [-Wunused-but-set-variable]
drivers/built-in.o: In function `mpc5121_nfc_probe':
mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
make[1]: *** [vmlinux] Error 1

--------------------------------------------------------------------------------

I do not really know why, but it seems that clock framework is not supported for powerpc. CCing the PPC mailing list. Preserved the context below for the PPC people.

So, not taking this patch.

References:

1. http://git.infradead.org/users/dedekind/aiaiai.git
2. http://git.infradead.org/users/dedekind/maintaining.git

On Sat, 2012-09-01 at 18:33 +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> devm free functions should not have to be explicitly used.
> 
> The only thing left that is useful in the function mpc5121_nfc_free is the
> call to clk_disable, which is moved to the call sites.
> 
> This function also incorrectly called iounmap on devm_ioremap allocated
> data.
> 
> Use devm_request_and_ioremap in place of devm_request_mem_region and
> devm_ioremap.
> 
> Use devm_clk_get.
> 
> A semantic match that finds the first problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> @@
> 
> (
> * devm_kfree(...);
> |
> * devm_free_irq(...);
> |
> * devm_iounmap(...);
> |
> * devm_release_region(...);
> |
> * devm_release_mem_region(...);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
>  drivers/mtd/nand/mpc5121_nfc.c |   35 +++++------------------------------
>  1 file changed, 5 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
> index c259c24..45183ba 100644
> --- a/drivers/mtd/nand/mpc5121_nfc.c
> +++ b/drivers/mtd/nand/mpc5121_nfc.c
> @@ -632,21 +632,6 @@ out:
>  	return ret;
>  }
>  
> -/* Free driver resources */
> -static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
> -{
> -	struct nand_chip *chip = mtd->priv;
> -	struct mpc5121_nfc_prv *prv = chip->priv;
> -
> -	if (prv->clk) {
> -		clk_disable(prv->clk);
> -		clk_put(prv->clk);
> -	}
> -
> -	if (prv->csreg)
> -		iounmap(prv->csreg);
> -}
> -
>  static int __devinit mpc5121_nfc_probe(struct platform_device *op)
>  {
>  	struct device_node *rootnode, *dn = op->dev.of_node;
> @@ -713,12 +698,7 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
>  	regs_paddr = res.start;
>  	regs_size = resource_size(&res);
>  
> -	if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
> -		dev_err(dev, "Error requesting memory region!\n");
> -		return -EBUSY;
> -	}
> -
> -	prv->regs = devm_ioremap(dev, regs_paddr, regs_size);
> +	prv->regs = devm_request_and_ioremap(dev, &res);
>  	if (!prv->regs) {
>  		dev_err(dev, "Error mapping memory region!\n");
>  		return -ENOMEM;
> @@ -752,11 +732,10 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
>  	of_node_put(rootnode);
>  
>  	/* Enable NFC clock */
> -	prv->clk = clk_get(dev, "nfc_clk");
> +	prv->clk = devm_clk_get(dev, "nfc_clk");
>  	if (IS_ERR(prv->clk)) {
>  		dev_err(dev, "Unable to acquire NFC clock!\n");
> -		retval = PTR_ERR(prv->clk);
> -		goto error;
> +		return PTR_ERR(prv->clk);
>  	}
>  
>  	clk_enable(prv->clk);
> @@ -803,7 +782,6 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
>  	/* Detect NAND chips */
>  	if (nand_scan(mtd, be32_to_cpup(chips_no))) {
>  		dev_err(dev, "NAND Flash not found !\n");
> -		devm_free_irq(dev, prv->irq, mtd);
>  		retval = -ENXIO;
>  		goto error;
>  	}
> @@ -828,7 +806,6 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
>  
>  	default:
>  		dev_err(dev, "Unsupported NAND flash!\n");
> -		devm_free_irq(dev, prv->irq, mtd);
>  		retval = -ENXIO;
>  		goto error;
>  	}
> @@ -839,13 +816,12 @@ static int __devinit mpc5121_nfc_probe(struct platform_device *op)
>  	retval = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
>  	if (retval) {
>  		dev_err(dev, "Error adding MTD device!\n");
> -		devm_free_irq(dev, prv->irq, mtd);
>  		goto error;
>  	}
>  
>  	return 0;
>  error:
> -	mpc5121_nfc_free(dev, mtd);
> +	clk_disable(prv->clk);
>  	return retval;
>  }
>  
> @@ -857,8 +833,7 @@ static int __devexit mpc5121_nfc_remove(struct platform_device *op)
>  	struct mpc5121_nfc_prv *prv = chip->priv;
>  
>  	nand_release(mtd);
> -	devm_free_irq(dev, prv->irq, mtd);
> -	mpc5121_nfc_free(dev, mtd);
> +	clk_disable(prv->clk);
>  
>  	return 0;
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
  2012-09-04  8:42   ` Artem Bityutskiy
@ 2012-09-04  8:53     ` Lars-Peter Clausen
  2012-09-04  9:44       ` Julia Lawall
  0 siblings, 1 reply; 11+ messages in thread
From: Lars-Peter Clausen @ 2012-09-04  8:53 UTC (permalink / raw)
  To: dedekind1
  Cc: Julia Lawall, linuxppc-dev, David Woodhouse, kernel-janitors,
	linux-mtd, linux-kernel

On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
> Aiaiai! :-) [1] [2]
> 
> I've build-tested this using aiaiai and it reports that this change breaks the build:
> 
> dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < ~/tmp/julia2.mbox 
> Tested the patch(es) on top of the following commits:
> ba64756 Quick fixes - applied by aiaiai
> 651c6fa JFFS2: don't fail on bitflips in OOB
> e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
> ea9d312 mtd: cmdlinepart: minor cleanups
> 
> --------------------------------------------------------------------------------
> Failed to build the following commit for configuration "powerpc-mpc512x_defconfig" (architecture powerpc)":
> 
> 0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
> 
> ...
> drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
> drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set but not used [-Wunused-but-set-variable]
> drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set but not used [-Wunused-but-set-variable]
> drivers/built-in.o: In function `mpc5121_nfc_probe':
> mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
> make[1]: *** [vmlinux] Error 1
> 
> --------------------------------------------------------------------------------
> 
> I do not really know why, but it seems that clock framework is not supported for powerpc. CCing the PPC mailing list. Preserved the context below for the PPC people.
> 

I've been bitten by the same issue recently, also cause by one of these
cocci devm patches. devm_clk_get is only available if the generic
clk_get/clk_put implementation is used. Not all architectures do this and
some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
is merely a wrapper around clk_get/clk_put there is no reason why it should
depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
but it is on a different machine right now, will try to submit it later today.

- Lars


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
  2012-09-04  8:53     ` Lars-Peter Clausen
@ 2012-09-04  9:44       ` Julia Lawall
  2012-09-04  9:54         ` Artem Bityutskiy
  0 siblings, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2012-09-04  9:44 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: dedekind1, Julia Lawall, linuxppc-dev, David Woodhouse,
	kernel-janitors, linux-mtd, linux-kernel

On Tue, 4 Sep 2012, Lars-Peter Clausen wrote:

> On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
> > Aiaiai! :-) [1] [2]
> >
> > I've build-tested this using aiaiai and it reports that this change breaks the build:
> >
> > dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < ~/tmp/julia2.mbox
> > Tested the patch(es) on top of the following commits:
> > ba64756 Quick fixes - applied by aiaiai
> > 651c6fa JFFS2: don't fail on bitflips in OOB
> > e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
> > ea9d312 mtd: cmdlinepart: minor cleanups
> >
> > --------------------------------------------------------------------------------
> > Failed to build the following commit for configuration "powerpc-mpc512x_defconfig" (architecture powerpc)":
> >
> > 0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
> >
> > ...
> > drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
> > drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set but not used [-Wunused-but-set-variable]
> > drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set but not used [-Wunused-but-set-variable]
> > drivers/built-in.o: In function `mpc5121_nfc_probe':
> > mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
> > make[1]: *** [vmlinux] Error 1
> >
> > --------------------------------------------------------------------------------
> >
> > I do not really know why, but it seems that clock framework is not supported for powerpc. CCing the PPC mailing list. Preserved the context below for the PPC people.
> >
>
> I've been bitten by the same issue recently, also cause by one of these
> cocci devm patches. devm_clk_get is only available if the generic
> clk_get/clk_put implementation is used. Not all architectures do this and
> some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
> is merely a wrapper around clk_get/clk_put there is no reason why it should
> depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
> available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
> but it is on a different machine right now, will try to submit it later today.

Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
all architectures, and I have no way of compiling code for these
architectures...  But I wonder why it is not, since devm-ness doesn't seem
to have anything to do with architecture-specific details?  It would be
really nice to have it for all architectures, because the clock functions
are just as (or at least almost as) common as kzalloc, ioremap, etc.

thanks,
julia

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
  2012-09-04  9:44       ` Julia Lawall
@ 2012-09-04  9:54         ` Artem Bityutskiy
  0 siblings, 0 replies; 11+ messages in thread
From: Artem Bityutskiy @ 2012-09-04  9:54 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Lars-Peter Clausen, linuxppc-dev, David Woodhouse,
	kernel-janitors, linux-mtd, linux-kernel

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

On Tue, 2012-09-04 at 11:44 +0200, Julia Lawall wrote:
> > I've been bitten by the same issue recently, also cause by one of these
> > cocci devm patches. devm_clk_get is only available if the generic
> > clk_get/clk_put implementation is used. Not all architectures do this and
> > some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
> > is merely a wrapper around clk_get/clk_put there is no reason why it should
> > depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
> > available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
> > but it is on a different machine right now, will try to submit it later today.
> 
> Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
> all architectures, and I have no way of compiling code for these
> architectures...  But I wonder why it is not, since devm-ness doesn't seem
> to have anything to do with architecture-specific details?  It would be
> really nice to have it for all architectures, because the clock functions
> are just as (or at least almost as) common as kzalloc, ioremap, etc.

It looks like Lars is going to fix this.

I am personally fine if you send patches without build-testing them.
Your patches are generally of good quality and you send many of them, so
build-testing each would be too much for you. And at least for MTD, I
can build-test myself.


-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] drivers/spi/spi-sh-hspi.c: drop frees of devm_ alloc'd data
  2012-09-01 16:33 ` [PATCH 1/4] drivers/spi/spi-sh-hspi.c: " Julia Lawall
@ 2012-09-05 23:43   ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2012-09-05 23:43 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Grant Likely, kernel-janitors, spi-devel-general, linux-kernel

On Sat, Sep 01, 2012 at 06:33:08PM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> devm free functions should not have to be explicitly used.

Applied, thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-09-05 23:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-01 16:33 [PATCH 0/4] drop frees of devm_ alloc'd data Julia Lawall
2012-09-01 16:33 ` [PATCH 1/4] drivers/spi/spi-sh-hspi.c: " Julia Lawall
2012-09-05 23:43   ` Mark Brown
2012-09-01 16:33 ` [PATCH 2/4] drivers/tty/serial/sirfsoc_uart.c: " Julia Lawall
2012-09-01 16:33 ` [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups Julia Lawall
2012-09-04  8:42   ` Artem Bityutskiy
2012-09-04  8:53     ` Lars-Peter Clausen
2012-09-04  9:44       ` Julia Lawall
2012-09-04  9:54         ` Artem Bityutskiy
2012-09-01 16:33 ` [PATCH 3/4] drivers/mtd/maps/autcpu12-nvram.c: drop frees of devm_ alloc'd data Julia Lawall
2012-09-04  7:50   ` Artem Bityutskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).