linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arch/arm/mach-omap1/dma.c: Invert calls to platform_device_put and platform_device_del
@ 2011-05-13 16:46 Julia Lawall
  2011-05-31  7:40 ` Tony Lindgren
  0 siblings, 1 reply; 2+ messages in thread
From: Julia Lawall @ 2011-05-13 16:46 UTC (permalink / raw)
  To: linux-arm-kernel

From: Julia Lawall <julia@diku.dk>

Platform_device_del should be called before platform_device_put, as
platform_device_put can delete the structure.

Additionally, improve the error handling code for the call to ioremap, so
that it calls platform_device_put.

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

// <smpl>
@@
expression e1,e2;
@@

*platform_device_put(e1);
... when != e1 = e2
*platform_device_del(e1);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
Is an ioremap needed also?  Not clear, because dma_base i a global variable.

 arch/arm/mach-omap1/dma.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap1/dma.c b/arch/arm/mach-omap1/dma.c
index d855934..f5a5220 100644
--- a/arch/arm/mach-omap1/dma.c
+++ b/arch/arm/mach-omap1/dma.c
@@ -284,14 +284,15 @@ static int __init omap1_system_dma_init(void)
 	dma_base = ioremap(res[0].start, resource_size(&res[0]));
 	if (!dma_base) {
 		pr_err("%s: Unable to ioremap\n", __func__);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto exit_device_put;
 	}
 
 	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
 	if (ret) {
 		dev_err(&pdev->dev, "%s: Unable to add resources for %s%d\n",
 			__func__, pdev->name, pdev->id);
-		goto exit_device_del;
+		goto exit_device_put;
 	}
 
 	p = kzalloc(sizeof(struct omap_system_dma_plat_info), GFP_KERNEL);
@@ -299,7 +300,7 @@ static int __init omap1_system_dma_init(void)
 		dev_err(&pdev->dev, "%s: Unable to allocate 'p' for %s\n",
 			__func__, pdev->name);
 		ret = -ENOMEM;
-		goto exit_device_put;
+		goto exit_device_del;
 	}
 
 	d = kzalloc(sizeof(struct omap_dma_dev_attr), GFP_KERNEL);
@@ -380,10 +381,10 @@ exit_release_d:
 	kfree(d);
 exit_release_p:
 	kfree(p);
-exit_device_put:
-	platform_device_put(pdev);
 exit_device_del:
 	platform_device_del(pdev);
+exit_device_put:
+	platform_device_put(pdev);
 
 	return ret;
 }

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

* [PATCH] arch/arm/mach-omap1/dma.c: Invert calls to platform_device_put and platform_device_del
  2011-05-13 16:46 [PATCH] arch/arm/mach-omap1/dma.c: Invert calls to platform_device_put and platform_device_del Julia Lawall
@ 2011-05-31  7:40 ` Tony Lindgren
  0 siblings, 0 replies; 2+ messages in thread
From: Tony Lindgren @ 2011-05-31  7:40 UTC (permalink / raw)
  To: linux-arm-kernel

* Julia Lawall <julia@diku.dk> [110513 09:42]:
> From: Julia Lawall <julia@diku.dk>
> 
> Platform_device_del should be called before platform_device_put, as
> platform_device_put can delete the structure.
> 
> Additionally, improve the error handling code for the call to ioremap, so
> that it calls platform_device_put.
> 
> The semantic match that finds this problem is:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e1,e2;
> @@
> 
> *platform_device_put(e1);
> ... when != e1 = e2
> *platform_device_del(e1);
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Thanks, applying into fixes.

> 
> ---
> Is an ioremap needed also?  Not clear, because dma_base i a global variable.

I guess you mean iounmap instead of ioremap? In this case we have static
mappings for ioremap, so iounmap is not doing anything. But should be done
for correct handling.

Regards,

Tony
 
>  arch/arm/mach-omap1/dma.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/dma.c b/arch/arm/mach-omap1/dma.c
> index d855934..f5a5220 100644
> --- a/arch/arm/mach-omap1/dma.c
> +++ b/arch/arm/mach-omap1/dma.c
> @@ -284,14 +284,15 @@ static int __init omap1_system_dma_init(void)
>  	dma_base = ioremap(res[0].start, resource_size(&res[0]));
>  	if (!dma_base) {
>  		pr_err("%s: Unable to ioremap\n", __func__);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto exit_device_put;
>  	}
>  
>  	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
>  	if (ret) {
>  		dev_err(&pdev->dev, "%s: Unable to add resources for %s%d\n",
>  			__func__, pdev->name, pdev->id);
> -		goto exit_device_del;
> +		goto exit_device_put;
>  	}
>  
>  	p = kzalloc(sizeof(struct omap_system_dma_plat_info), GFP_KERNEL);
> @@ -299,7 +300,7 @@ static int __init omap1_system_dma_init(void)
>  		dev_err(&pdev->dev, "%s: Unable to allocate 'p' for %s\n",
>  			__func__, pdev->name);
>  		ret = -ENOMEM;
> -		goto exit_device_put;
> +		goto exit_device_del;
>  	}
>  
>  	d = kzalloc(sizeof(struct omap_dma_dev_attr), GFP_KERNEL);
> @@ -380,10 +381,10 @@ exit_release_d:
>  	kfree(d);
>  exit_release_p:
>  	kfree(p);
> -exit_device_put:
> -	platform_device_put(pdev);
>  exit_device_del:
>  	platform_device_del(pdev);
> +exit_device_put:
> +	platform_device_put(pdev);
>  
>  	return ret;
>  }
> 

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

end of thread, other threads:[~2011-05-31  7:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-13 16:46 [PATCH] arch/arm/mach-omap1/dma.c: Invert calls to platform_device_put and platform_device_del Julia Lawall
2011-05-31  7:40 ` Tony Lindgren

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