Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
From: Julia Lawall
Using devm_kzalloc simplifies the code and ensures that the use of
devm_request_irq is safe. When kzalloc and kfree were used, the interrupt
could be triggered after the handler's data argument had been freed.
This also introduces some missing initializations of the return variable
ret, and uses devm_request_and_ioremap instead of the combination of
devm_request_mem_region and devm_ioremap.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
//
@r exists@
expression e1,e2,x,a,b,c,d;
identifier free;
position p1,p2;
@@
devm_request_irq@p1(e1,e2,...,x)
... when any
when != e2 = a
when != x = b
if (...) {
... when != e2 = c
when != x = d
free@p2(...,x,...);
...
return ...;
}
//
Signed-off-by: Julia Lawall
---
This is the same patch I sent before. I had no trouble applying it after
cloning the staging/for_v3.7 branch.
drivers/media/video/mx2_emmaprp.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 2810015..dab380a 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -896,7 +896,7 @@ static int emmaprp_probe(struct platform_device *pdev)
int irq_emma;
int ret;
- pcdev = kzalloc(sizeof *pcdev, GFP_KERNEL);
+ pcdev = devm_kzalloc(>dev, sizeof(*pcdev), GFP_KERNEL);
if (!pcdev)
return -ENOMEM;
@@ -904,27 +904,24 @@ static int emmaprp_probe(struct platform_device *pdev)
pcdev->clk_emma_ipg = devm_clk_get(>dev, "ipg");
if (IS_ERR(pcdev->clk_emma_ipg)) {
- ret = PTR_ERR(pcdev->clk_emma_ipg);
- goto free_dev;
+ return PTR_ERR(pcdev->clk_emma_ipg);
}
pcdev->clk_emma_ahb = devm_clk_get(>dev, "ahb");
if (IS_ERR(pcdev->clk_emma_ipg)) {
- ret = PTR_ERR(pcdev->clk_emma_ahb);
- goto free_dev;
+ return PTR_ERR(pcdev->clk_emma_ahb);
}
irq_emma = platform_get_irq(pdev, 0);
res_emma = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (irq_emma < 0 || res_emma == NULL) {
dev_err(>dev, "Missing platform resources data\n");
- ret = -ENODEV;
- goto free_dev;
+ return -ENODEV;
}
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto free_dev;
+ return ret;
mutex_init(>dev_mutex);
@@ -946,21 +943,20 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
- if (devm_request_mem_region(>dev, res_emma->start,
- resource_size(res_emma), MEM2MEM_NAME) == NULL)
- goto rel_vdev;
-
- pcdev->base_emma = devm_ioremap(>dev, res_emma->start,
- resource_size(res_emma));
- if (!pcdev->base_emma)
+ pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma);
+ if (!pcdev->base_emma) {
+ ret = -ENXIO;
goto rel_vdev;
+ }
pcdev->irq_emma = irq_emma;
pcdev->res_emma = res_emma;
if (devm_request_irq(>dev, pcdev->irq_emma, emmaprp_irq,
-0, MEM2MEM_NAME, pcdev) < 0)
+0, MEM2MEM_NAME, pcdev) < 0) {
+ ret = -ENODEV;
goto rel_vdev;
+ }
pcdev->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
if (IS_ERR(pcdev->alloc_ctx)) {
@@ -993,8 +989,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(>v4l2_dev);
-free_dev:
- kfree(pcdev);
return ret;
}
@@ -1009,7 +1003,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev->m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
v4l2_device_unregister(>v4l2_dev);
- kfree(pcdev);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Em 14-08-2012 03:30, Julia Lawall escreveu: >> Well, I've massively applied hundreds of patches today, but not much >> on this driver. Maybe it is better for you to wait for a couple of >> days for these to be at -next, or use, instead, our tree as the basis for >> it: >> git://linuxtv.org/media_tree.git staging/for_v3.7 > > I cloned this, but it doesn't seem to contain the file: > >> :~: cd staging/for_v3.7/ staging/for_v3.7 is the name of the branch ;) So, you need to use "--branch staging/for_v3.7" on your git clone command. >> :for_v3.7: ls drivers/media/video/mx2_emmaprp.c > ls: cannot access drivers/media/video/mx2_emmaprp.c: No such file or directory PS.: I started yesterday to apply a major reorganization of the drivers along drivers/media/. This driver is still there at the same path, but it will be moving soon to another place. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Well, I've massively applied hundreds of patches today, but not much on this driver. Maybe it is better for you to wait for a couple of days for these to be at -next, or use, instead, our tree as the basis for it: git://linuxtv.org/media_tree.git staging/for_v3.7 I cloned this, but it doesn't seem to contain the file: :~: cd staging/for_v3.7/ :for_v3.7: ls drivers/media/video/mx2_emmaprp.c ls: cannot access drivers/media/video/mx2_emmaprp.c: No such file or directory julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Well, I've massively applied hundreds of patches today, but not much on this driver. Maybe it is better for you to wait for a couple of days for these to be at -next, or use, instead, our tree as the basis for it: git://linuxtv.org/media_tree.git staging/for_v3.7 I cloned this, but it doesn't seem to contain the file: :~: cd staging/for_v3.7/ :for_v3.7: ls drivers/media/video/mx2_emmaprp.c ls: cannot access drivers/media/video/mx2_emmaprp.c: No such file or directory julia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Em 14-08-2012 03:30, Julia Lawall escreveu: Well, I've massively applied hundreds of patches today, but not much on this driver. Maybe it is better for you to wait for a couple of days for these to be at -next, or use, instead, our tree as the basis for it: git://linuxtv.org/media_tree.git staging/for_v3.7 I cloned this, but it doesn't seem to contain the file: :~: cd staging/for_v3.7/ staging/for_v3.7 is the name of the branch ;) So, you need to use --branch staging/for_v3.7 on your git clone command. :for_v3.7: ls drivers/media/video/mx2_emmaprp.c ls: cannot access drivers/media/video/mx2_emmaprp.c: No such file or directory PS.: I started yesterday to apply a major reorganization of the drivers along drivers/media/. This driver is still there at the same path, but it will be moving soon to another place. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
From: Julia Lawall julia.law...@lip6.fr
Using devm_kzalloc simplifies the code and ensures that the use of
devm_request_irq is safe. When kzalloc and kfree were used, the interrupt
could be triggered after the handler's data argument had been freed.
This also introduces some missing initializations of the return variable
ret, and uses devm_request_and_ioremap instead of the combination of
devm_request_mem_region and devm_ioremap.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
// smpl
@r exists@
expression e1,e2,x,a,b,c,d;
identifier free;
position p1,p2;
@@
devm_request_irq@p1(e1,e2,...,x)
... when any
when != e2 = a
when != x = b
if (...) {
... when != e2 = c
when != x = d
free@p2(...,x,...);
...
return ...;
}
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
This is the same patch I sent before. I had no trouble applying it after
cloning the staging/for_v3.7 branch.
drivers/media/video/mx2_emmaprp.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 2810015..dab380a 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -896,7 +896,7 @@ static int emmaprp_probe(struct platform_device *pdev)
int irq_emma;
int ret;
- pcdev = kzalloc(sizeof *pcdev, GFP_KERNEL);
+ pcdev = devm_kzalloc(pdev-dev, sizeof(*pcdev), GFP_KERNEL);
if (!pcdev)
return -ENOMEM;
@@ -904,27 +904,24 @@ static int emmaprp_probe(struct platform_device *pdev)
pcdev-clk_emma_ipg = devm_clk_get(pdev-dev, ipg);
if (IS_ERR(pcdev-clk_emma_ipg)) {
- ret = PTR_ERR(pcdev-clk_emma_ipg);
- goto free_dev;
+ return PTR_ERR(pcdev-clk_emma_ipg);
}
pcdev-clk_emma_ahb = devm_clk_get(pdev-dev, ahb);
if (IS_ERR(pcdev-clk_emma_ipg)) {
- ret = PTR_ERR(pcdev-clk_emma_ahb);
- goto free_dev;
+ return PTR_ERR(pcdev-clk_emma_ahb);
}
irq_emma = platform_get_irq(pdev, 0);
res_emma = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (irq_emma 0 || res_emma == NULL) {
dev_err(pdev-dev, Missing platform resources data\n);
- ret = -ENODEV;
- goto free_dev;
+ return -ENODEV;
}
ret = v4l2_device_register(pdev-dev, pcdev-v4l2_dev);
if (ret)
- goto free_dev;
+ return ret;
mutex_init(pcdev-dev_mutex);
@@ -946,21 +943,20 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
- if (devm_request_mem_region(pdev-dev, res_emma-start,
- resource_size(res_emma), MEM2MEM_NAME) == NULL)
- goto rel_vdev;
-
- pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start,
- resource_size(res_emma));
- if (!pcdev-base_emma)
+ pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma);
+ if (!pcdev-base_emma) {
+ ret = -ENXIO;
goto rel_vdev;
+ }
pcdev-irq_emma = irq_emma;
pcdev-res_emma = res_emma;
if (devm_request_irq(pdev-dev, pcdev-irq_emma, emmaprp_irq,
-0, MEM2MEM_NAME, pcdev) 0)
+0, MEM2MEM_NAME, pcdev) 0) {
+ ret = -ENODEV;
goto rel_vdev;
+ }
pcdev-alloc_ctx = vb2_dma_contig_init_ctx(pdev-dev);
if (IS_ERR(pcdev-alloc_ctx)) {
@@ -993,8 +989,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(pcdev-v4l2_dev);
-free_dev:
- kfree(pcdev);
return ret;
}
@@ -1009,7 +1003,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev-m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev-alloc_ctx);
v4l2_device_unregister(pcdev-v4l2_dev);
- kfree(pcdev);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Em 13-08-2012 17:20, Julia Lawall escreveu:
> From: Julia Lawall
>
> Using devm_kzalloc simplifies the code and ensures that the use of
> devm_request_irq is safe. When kzalloc and kfree were used, the interrupt
> could be triggered after the handler's data argument had been freed.
>
> This also introduces some missing initializations of the return variable
> ret, and uses devm_request_and_ioremap instead of the combination of
> devm_request_mem_region and devm_ioremap.
>
> The problem of a free after a devm_request_irq was found using the
> following semantic match (http://coccinelle.lip6.fr/)
>
> //
> @r exists@
> expression e1,e2,x,a,b,c,d;
> identifier free;
> position p1,p2;
> @@
>
> devm_request_irq@p1(e1,e2,...,x)
> ... when any
> when != e2 = a
> when != x = b
> if (...) {
> ... when != e2 = c
> when != x = d
> free@p2(...,x,...);
> ...
> return ...;
> }
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> v3: due to a conflict with another patch
Not sure what tree you used for it, but the result was
worse ;)
patching file drivers/media/video/mx2_emmaprp.c
Hunk #1 FAILED at 896.
Hunk #2 FAILED at 904.
Hunk #3 FAILED at 946.
Hunk #4 FAILED at 993.
Hunk #5 FAILED at 1009.
5 out of 5 hunks FAILED -- rejects in file drivers/media/video/mx2_emmaprp.c
Well, I've massively applied hundreds of patches today, but not much
on this driver. Maybe it is better for you to wait for a couple of
days for these to be at -next, or use, instead, our tree as the basis for
it:
git://linuxtv.org/media_tree.git staging/for_v3.7
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
From: Julia Lawall
Using devm_kzalloc simplifies the code and ensures that the use of
devm_request_irq is safe. When kzalloc and kfree were used, the interrupt
could be triggered after the handler's data argument had been freed.
This also introduces some missing initializations of the return variable
ret, and uses devm_request_and_ioremap instead of the combination of
devm_request_mem_region and devm_ioremap.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
//
@r exists@
expression e1,e2,x,a,b,c,d;
identifier free;
position p1,p2;
@@
devm_request_irq@p1(e1,e2,...,x)
... when any
when != e2 = a
when != x = b
if (...) {
... when != e2 = c
when != x = d
free@p2(...,x,...);
...
return ...;
}
//
Signed-off-by: Julia Lawall
---
v3: due to a conflict with another patch
drivers/media/video/mx2_emmaprp.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 2810015..dab380a 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -896,7 +896,7 @@ static int emmaprp_probe(struct platform_device *pdev)
int irq_emma;
int ret;
- pcdev = kzalloc(sizeof *pcdev, GFP_KERNEL);
+ pcdev = devm_kzalloc(>dev, sizeof(*pcdev), GFP_KERNEL);
if (!pcdev)
return -ENOMEM;
@@ -904,27 +904,24 @@ static int emmaprp_probe(struct platform_device *pdev)
pcdev->clk_emma_ipg = devm_clk_get(>dev, "ipg");
if (IS_ERR(pcdev->clk_emma_ipg)) {
- ret = PTR_ERR(pcdev->clk_emma_ipg);
- goto free_dev;
+ return PTR_ERR(pcdev->clk_emma_ipg);
}
pcdev->clk_emma_ahb = devm_clk_get(>dev, "ahb");
if (IS_ERR(pcdev->clk_emma_ipg)) {
- ret = PTR_ERR(pcdev->clk_emma_ahb);
- goto free_dev;
+ return PTR_ERR(pcdev->clk_emma_ahb);
}
irq_emma = platform_get_irq(pdev, 0);
res_emma = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (irq_emma < 0 || res_emma == NULL) {
dev_err(>dev, "Missing platform resources data\n");
- ret = -ENODEV;
- goto free_dev;
+ return -ENODEV;
}
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto free_dev;
+ return ret;
mutex_init(>dev_mutex);
@@ -946,21 +943,20 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
- if (devm_request_mem_region(>dev, res_emma->start,
- resource_size(res_emma), MEM2MEM_NAME) == NULL)
- goto rel_vdev;
-
- pcdev->base_emma = devm_ioremap(>dev, res_emma->start,
- resource_size(res_emma));
- if (!pcdev->base_emma)
+ pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma);
+ if (!pcdev->base_emma) {
+ ret = -ENXIO;
goto rel_vdev;
+ }
pcdev->irq_emma = irq_emma;
pcdev->res_emma = res_emma;
if (devm_request_irq(>dev, pcdev->irq_emma, emmaprp_irq,
-0, MEM2MEM_NAME, pcdev) < 0)
+0, MEM2MEM_NAME, pcdev) < 0) {
+ ret = -ENODEV;
goto rel_vdev;
+ }
pcdev->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
if (IS_ERR(pcdev->alloc_ctx)) {
@@ -993,8 +989,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(>v4l2_dev);
-free_dev:
- kfree(pcdev);
return ret;
}
@@ -1009,7 +1003,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev->m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
v4l2_device_unregister(>v4l2_dev);
- kfree(pcdev);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On Mon, 13 Aug 2012, Mauro Carvalho Chehab wrote:
Em 10-08-2012 10:59, Julia Lawall escreveu:
From: Julia Lawall
Using devm_kzalloc and devm_clk_get simplifies the code and ensures that
the use of devm_request_irq is safe. When kzalloc and kfree were used, the
interrupt could be triggered after the handler's data argument had been
freed.
Add missing return code initializations in the error handling code for
devm_request_and_ioremap and devm_request_irq.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
Hi Julia,
This patch doesn't apply anymore, likely due to this changeset:
OK, I'll fix it. Indeed, devm_clk_get is already introduced by this
patch.
julia
commit 33eb46a7c2bdd10f9a761390ce1bf51169ff537a
Author: Javier Martin
Date: Mon Jul 30 04:37:30 2012 -0300
[media] media: i.MX27: Fix mx2_emmaprp mem2mem driver clocks
This driver wasn't converted to the new clock framework
(e038ed50a4a767add205094c035b6943e7b30140).
Signed-off-by: Javier Martin
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 5f8a6f5..728cac3 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -209,7 +209,7 @@ struct emmaprp_dev {
int irq_emma;
void __iomem*base_emma;
- struct clk *clk_emma;
+ struct clk *clk_emma_ahb, *clk_emma_ipg;
struct resource *res_emma;
struct v4l2_m2m_dev *m2m_dev;
@@ -804,7 +804,8 @@ static int emmaprp_open(struct file *file)
return ret;
}
- clk_enable(pcdev->clk_emma);
+ clk_prepare_enable(pcdev->clk_emma_ipg);
+ clk_prepare_enable(pcdev->clk_emma_ahb);
ctx->q_data[V4L2_M2M_SRC].fmt = [1];
ctx->q_data[V4L2_M2M_DST].fmt = [0];
@@ -820,7 +821,8 @@ static int emmaprp_release(struct file *file)
dprintk(pcdev, "Releasing instance %p\n", ctx);
- clk_disable(pcdev->clk_emma);
+ clk_disable_unprepare(pcdev->clk_emma_ahb);
+ clk_disable_unprepare(pcdev->clk_emma_ipg);
v4l2_m2m_ctx_release(ctx->m2m_ctx);
kfree(ctx);
@@ -880,9 +882,15 @@ static int emmaprp_probe(struct platform_device *pdev)
spin_lock_init(>irqlock);
- pcdev->clk_emma = clk_get(>dev, NULL);
- if (IS_ERR(pcdev->clk_emma)) {
- ret = PTR_ERR(pcdev->clk_emma);
+ pcdev->clk_emma_ipg = devm_clk_get(>dev, "ipg");
+ if (IS_ERR(pcdev->clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev->clk_emma_ipg);
+ goto free_dev;
+ }
+
+ pcdev->clk_emma_ahb = devm_clk_get(>dev, "ahb");
+ if (IS_ERR(pcdev->clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev->clk_emma_ahb);
goto free_dev;
}
@@ -891,12 +899,12 @@ static int emmaprp_probe(struct platform_device *pdev)
if (irq_emma < 0 || res_emma == NULL) {
dev_err(>dev, "Missing platform resources data\n");
ret = -ENODEV;
- goto free_clk;
+ goto free_dev;
}
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto free_clk;
+ goto free_dev;
mutex_init(>dev_mutex);
@@ -969,8 +977,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(>v4l2_dev);
-free_clk:
- clk_put(pcdev->clk_emma);
free_dev:
kfree(pcdev);
@@ -987,7 +993,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev->m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
v4l2_device_unregister(>v4l2_dev);
- clk_put(pcdev->clk_emma);
kfree(pcdev);
return 0;
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Em 10-08-2012 10:59, Julia Lawall escreveu:
> From: Julia Lawall
>
> Using devm_kzalloc and devm_clk_get simplifies the code and ensures that
> the use of devm_request_irq is safe. When kzalloc and kfree were used, the
> interrupt could be triggered after the handler's data argument had been
> freed.
>
> Add missing return code initializations in the error handling code for
> devm_request_and_ioremap and devm_request_irq.
>
> The problem of a free after a devm_request_irq was found using the
> following semantic match (http://coccinelle.lip6.fr/)
Hi Julia,
This patch doesn't apply anymore, likely due to this changeset:
commit 33eb46a7c2bdd10f9a761390ce1bf51169ff537a
Author: Javier Martin
Date: Mon Jul 30 04:37:30 2012 -0300
[media] media: i.MX27: Fix mx2_emmaprp mem2mem driver clocks
This driver wasn't converted to the new clock framework
(e038ed50a4a767add205094c035b6943e7b30140).
Signed-off-by: Javier Martin
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 5f8a6f5..728cac3 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -209,7 +209,7 @@ struct emmaprp_dev {
int irq_emma;
void __iomem*base_emma;
- struct clk *clk_emma;
+ struct clk *clk_emma_ahb, *clk_emma_ipg;
struct resource *res_emma;
struct v4l2_m2m_dev *m2m_dev;
@@ -804,7 +804,8 @@ static int emmaprp_open(struct file *file)
return ret;
}
- clk_enable(pcdev->clk_emma);
+ clk_prepare_enable(pcdev->clk_emma_ipg);
+ clk_prepare_enable(pcdev->clk_emma_ahb);
ctx->q_data[V4L2_M2M_SRC].fmt = [1];
ctx->q_data[V4L2_M2M_DST].fmt = [0];
@@ -820,7 +821,8 @@ static int emmaprp_release(struct file *file)
dprintk(pcdev, "Releasing instance %p\n", ctx);
- clk_disable(pcdev->clk_emma);
+ clk_disable_unprepare(pcdev->clk_emma_ahb);
+ clk_disable_unprepare(pcdev->clk_emma_ipg);
v4l2_m2m_ctx_release(ctx->m2m_ctx);
kfree(ctx);
@@ -880,9 +882,15 @@ static int emmaprp_probe(struct platform_device *pdev)
spin_lock_init(>irqlock);
- pcdev->clk_emma = clk_get(>dev, NULL);
- if (IS_ERR(pcdev->clk_emma)) {
- ret = PTR_ERR(pcdev->clk_emma);
+ pcdev->clk_emma_ipg = devm_clk_get(>dev, "ipg");
+ if (IS_ERR(pcdev->clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev->clk_emma_ipg);
+ goto free_dev;
+ }
+
+ pcdev->clk_emma_ahb = devm_clk_get(>dev, "ahb");
+ if (IS_ERR(pcdev->clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev->clk_emma_ahb);
goto free_dev;
}
@@ -891,12 +899,12 @@ static int emmaprp_probe(struct platform_device *pdev)
if (irq_emma < 0 || res_emma == NULL) {
dev_err(>dev, "Missing platform resources data\n");
ret = -ENODEV;
- goto free_clk;
+ goto free_dev;
}
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto free_clk;
+ goto free_dev;
mutex_init(>dev_mutex);
@@ -969,8 +977,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(>v4l2_dev);
-free_clk:
- clk_put(pcdev->clk_emma);
free_dev:
kfree(pcdev);
@@ -987,7 +993,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev->m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
v4l2_device_unregister(>v4l2_dev);
- clk_put(pcdev->clk_emma);
kfree(pcdev);
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Em 10-08-2012 10:59, Julia Lawall escreveu:
From: Julia Lawall julia.law...@lip6.fr
Using devm_kzalloc and devm_clk_get simplifies the code and ensures that
the use of devm_request_irq is safe. When kzalloc and kfree were used, the
interrupt could be triggered after the handler's data argument had been
freed.
Add missing return code initializations in the error handling code for
devm_request_and_ioremap and devm_request_irq.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
Hi Julia,
This patch doesn't apply anymore, likely due to this changeset:
commit 33eb46a7c2bdd10f9a761390ce1bf51169ff537a
Author: Javier Martin javier.mar...@vista-silicon.com
Date: Mon Jul 30 04:37:30 2012 -0300
[media] media: i.MX27: Fix mx2_emmaprp mem2mem driver clocks
This driver wasn't converted to the new clock framework
(e038ed50a4a767add205094c035b6943e7b30140).
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 5f8a6f5..728cac3 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -209,7 +209,7 @@ struct emmaprp_dev {
int irq_emma;
void __iomem*base_emma;
- struct clk *clk_emma;
+ struct clk *clk_emma_ahb, *clk_emma_ipg;
struct resource *res_emma;
struct v4l2_m2m_dev *m2m_dev;
@@ -804,7 +804,8 @@ static int emmaprp_open(struct file *file)
return ret;
}
- clk_enable(pcdev-clk_emma);
+ clk_prepare_enable(pcdev-clk_emma_ipg);
+ clk_prepare_enable(pcdev-clk_emma_ahb);
ctx-q_data[V4L2_M2M_SRC].fmt = formats[1];
ctx-q_data[V4L2_M2M_DST].fmt = formats[0];
@@ -820,7 +821,8 @@ static int emmaprp_release(struct file *file)
dprintk(pcdev, Releasing instance %p\n, ctx);
- clk_disable(pcdev-clk_emma);
+ clk_disable_unprepare(pcdev-clk_emma_ahb);
+ clk_disable_unprepare(pcdev-clk_emma_ipg);
v4l2_m2m_ctx_release(ctx-m2m_ctx);
kfree(ctx);
@@ -880,9 +882,15 @@ static int emmaprp_probe(struct platform_device *pdev)
spin_lock_init(pcdev-irqlock);
- pcdev-clk_emma = clk_get(pdev-dev, NULL);
- if (IS_ERR(pcdev-clk_emma)) {
- ret = PTR_ERR(pcdev-clk_emma);
+ pcdev-clk_emma_ipg = devm_clk_get(pdev-dev, ipg);
+ if (IS_ERR(pcdev-clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev-clk_emma_ipg);
+ goto free_dev;
+ }
+
+ pcdev-clk_emma_ahb = devm_clk_get(pdev-dev, ahb);
+ if (IS_ERR(pcdev-clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev-clk_emma_ahb);
goto free_dev;
}
@@ -891,12 +899,12 @@ static int emmaprp_probe(struct platform_device *pdev)
if (irq_emma 0 || res_emma == NULL) {
dev_err(pdev-dev, Missing platform resources data\n);
ret = -ENODEV;
- goto free_clk;
+ goto free_dev;
}
ret = v4l2_device_register(pdev-dev, pcdev-v4l2_dev);
if (ret)
- goto free_clk;
+ goto free_dev;
mutex_init(pcdev-dev_mutex);
@@ -969,8 +977,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(pcdev-v4l2_dev);
-free_clk:
- clk_put(pcdev-clk_emma);
free_dev:
kfree(pcdev);
@@ -987,7 +993,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev-m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev-alloc_ctx);
v4l2_device_unregister(pcdev-v4l2_dev);
- clk_put(pcdev-clk_emma);
kfree(pcdev);
return 0;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On Mon, 13 Aug 2012, Mauro Carvalho Chehab wrote:
Em 10-08-2012 10:59, Julia Lawall escreveu:
From: Julia Lawall julia.law...@lip6.fr
Using devm_kzalloc and devm_clk_get simplifies the code and ensures that
the use of devm_request_irq is safe. When kzalloc and kfree were used, the
interrupt could be triggered after the handler's data argument had been
freed.
Add missing return code initializations in the error handling code for
devm_request_and_ioremap and devm_request_irq.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
Hi Julia,
This patch doesn't apply anymore, likely due to this changeset:
OK, I'll fix it. Indeed, devm_clk_get is already introduced by this
patch.
julia
commit 33eb46a7c2bdd10f9a761390ce1bf51169ff537a
Author: Javier Martin javier.mar...@vista-silicon.com
Date: Mon Jul 30 04:37:30 2012 -0300
[media] media: i.MX27: Fix mx2_emmaprp mem2mem driver clocks
This driver wasn't converted to the new clock framework
(e038ed50a4a767add205094c035b6943e7b30140).
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 5f8a6f5..728cac3 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -209,7 +209,7 @@ struct emmaprp_dev {
int irq_emma;
void __iomem*base_emma;
- struct clk *clk_emma;
+ struct clk *clk_emma_ahb, *clk_emma_ipg;
struct resource *res_emma;
struct v4l2_m2m_dev *m2m_dev;
@@ -804,7 +804,8 @@ static int emmaprp_open(struct file *file)
return ret;
}
- clk_enable(pcdev-clk_emma);
+ clk_prepare_enable(pcdev-clk_emma_ipg);
+ clk_prepare_enable(pcdev-clk_emma_ahb);
ctx-q_data[V4L2_M2M_SRC].fmt = formats[1];
ctx-q_data[V4L2_M2M_DST].fmt = formats[0];
@@ -820,7 +821,8 @@ static int emmaprp_release(struct file *file)
dprintk(pcdev, Releasing instance %p\n, ctx);
- clk_disable(pcdev-clk_emma);
+ clk_disable_unprepare(pcdev-clk_emma_ahb);
+ clk_disable_unprepare(pcdev-clk_emma_ipg);
v4l2_m2m_ctx_release(ctx-m2m_ctx);
kfree(ctx);
@@ -880,9 +882,15 @@ static int emmaprp_probe(struct platform_device *pdev)
spin_lock_init(pcdev-irqlock);
- pcdev-clk_emma = clk_get(pdev-dev, NULL);
- if (IS_ERR(pcdev-clk_emma)) {
- ret = PTR_ERR(pcdev-clk_emma);
+ pcdev-clk_emma_ipg = devm_clk_get(pdev-dev, ipg);
+ if (IS_ERR(pcdev-clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev-clk_emma_ipg);
+ goto free_dev;
+ }
+
+ pcdev-clk_emma_ahb = devm_clk_get(pdev-dev, ahb);
+ if (IS_ERR(pcdev-clk_emma_ipg)) {
+ ret = PTR_ERR(pcdev-clk_emma_ahb);
goto free_dev;
}
@@ -891,12 +899,12 @@ static int emmaprp_probe(struct platform_device *pdev)
if (irq_emma 0 || res_emma == NULL) {
dev_err(pdev-dev, Missing platform resources data\n);
ret = -ENODEV;
- goto free_clk;
+ goto free_dev;
}
ret = v4l2_device_register(pdev-dev, pcdev-v4l2_dev);
if (ret)
- goto free_clk;
+ goto free_dev;
mutex_init(pcdev-dev_mutex);
@@ -969,8 +977,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(pcdev-v4l2_dev);
-free_clk:
- clk_put(pcdev-clk_emma);
free_dev:
kfree(pcdev);
@@ -987,7 +993,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev-m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev-alloc_ctx);
v4l2_device_unregister(pcdev-v4l2_dev);
- clk_put(pcdev-clk_emma);
kfree(pcdev);
return 0;
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
From: Julia Lawall julia.law...@lip6.fr
Using devm_kzalloc simplifies the code and ensures that the use of
devm_request_irq is safe. When kzalloc and kfree were used, the interrupt
could be triggered after the handler's data argument had been freed.
This also introduces some missing initializations of the return variable
ret, and uses devm_request_and_ioremap instead of the combination of
devm_request_mem_region and devm_ioremap.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
// smpl
@r exists@
expression e1,e2,x,a,b,c,d;
identifier free;
position p1,p2;
@@
devm_request_irq@p1(e1,e2,...,x)
... when any
when != e2 = a
when != x = b
if (...) {
... when != e2 = c
when != x = d
free@p2(...,x,...);
...
return ...;
}
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
v3: due to a conflict with another patch
drivers/media/video/mx2_emmaprp.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 2810015..dab380a 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -896,7 +896,7 @@ static int emmaprp_probe(struct platform_device *pdev)
int irq_emma;
int ret;
- pcdev = kzalloc(sizeof *pcdev, GFP_KERNEL);
+ pcdev = devm_kzalloc(pdev-dev, sizeof(*pcdev), GFP_KERNEL);
if (!pcdev)
return -ENOMEM;
@@ -904,27 +904,24 @@ static int emmaprp_probe(struct platform_device *pdev)
pcdev-clk_emma_ipg = devm_clk_get(pdev-dev, ipg);
if (IS_ERR(pcdev-clk_emma_ipg)) {
- ret = PTR_ERR(pcdev-clk_emma_ipg);
- goto free_dev;
+ return PTR_ERR(pcdev-clk_emma_ipg);
}
pcdev-clk_emma_ahb = devm_clk_get(pdev-dev, ahb);
if (IS_ERR(pcdev-clk_emma_ipg)) {
- ret = PTR_ERR(pcdev-clk_emma_ahb);
- goto free_dev;
+ return PTR_ERR(pcdev-clk_emma_ahb);
}
irq_emma = platform_get_irq(pdev, 0);
res_emma = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (irq_emma 0 || res_emma == NULL) {
dev_err(pdev-dev, Missing platform resources data\n);
- ret = -ENODEV;
- goto free_dev;
+ return -ENODEV;
}
ret = v4l2_device_register(pdev-dev, pcdev-v4l2_dev);
if (ret)
- goto free_dev;
+ return ret;
mutex_init(pcdev-dev_mutex);
@@ -946,21 +943,20 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
- if (devm_request_mem_region(pdev-dev, res_emma-start,
- resource_size(res_emma), MEM2MEM_NAME) == NULL)
- goto rel_vdev;
-
- pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start,
- resource_size(res_emma));
- if (!pcdev-base_emma)
+ pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma);
+ if (!pcdev-base_emma) {
+ ret = -ENXIO;
goto rel_vdev;
+ }
pcdev-irq_emma = irq_emma;
pcdev-res_emma = res_emma;
if (devm_request_irq(pdev-dev, pcdev-irq_emma, emmaprp_irq,
-0, MEM2MEM_NAME, pcdev) 0)
+0, MEM2MEM_NAME, pcdev) 0) {
+ ret = -ENODEV;
goto rel_vdev;
+ }
pcdev-alloc_ctx = vb2_dma_contig_init_ctx(pdev-dev);
if (IS_ERR(pcdev-alloc_ctx)) {
@@ -993,8 +989,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(pcdev-v4l2_dev);
-free_dev:
- kfree(pcdev);
return ret;
}
@@ -1009,7 +1003,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev-m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev-alloc_ctx);
v4l2_device_unregister(pcdev-v4l2_dev);
- kfree(pcdev);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Em 13-08-2012 17:20, Julia Lawall escreveu:
From: Julia Lawall julia.law...@lip6.fr
Using devm_kzalloc simplifies the code and ensures that the use of
devm_request_irq is safe. When kzalloc and kfree were used, the interrupt
could be triggered after the handler's data argument had been freed.
This also introduces some missing initializations of the return variable
ret, and uses devm_request_and_ioremap instead of the combination of
devm_request_mem_region and devm_ioremap.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
// smpl
@r exists@
expression e1,e2,x,a,b,c,d;
identifier free;
position p1,p2;
@@
devm_request_irq@p1(e1,e2,...,x)
... when any
when != e2 = a
when != x = b
if (...) {
... when != e2 = c
when != x = d
free@p2(...,x,...);
...
return ...;
}
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
v3: due to a conflict with another patch
Not sure what tree you used for it, but the result was
worse ;)
patching file drivers/media/video/mx2_emmaprp.c
Hunk #1 FAILED at 896.
Hunk #2 FAILED at 904.
Hunk #3 FAILED at 946.
Hunk #4 FAILED at 993.
Hunk #5 FAILED at 1009.
5 out of 5 hunks FAILED -- rejects in file drivers/media/video/mx2_emmaprp.c
Well, I've massively applied hundreds of patches today, but not much
on this driver. Maybe it is better for you to wait for a couple of
days for these to be at -next, or use, instead, our tree as the basis for
it:
git://linuxtv.org/media_tree.git staging/for_v3.7
Regards,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
From: Julia Lawall
Using devm_kzalloc and devm_clk_get simplifies the code and ensures that
the use of devm_request_irq is safe. When kzalloc and kfree were used, the
interrupt could be triggered after the handler's data argument had been
freed.
Add missing return code initializations in the error handling code for
devm_request_and_ioremap and devm_request_irq.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
//
@r exists@
expression e1,e2,x,a,b,c,d;
identifier free;
position p1,p2;
@@
devm_request_irq@p1(e1,e2,...,x)
... when any
when != e2 = a
when != x = b
if (...) {
... when != e2 = c
when != x = d
free@p2(...,x,...);
...
return ...;
}
//
Signed-off-by: Julia Lawall
---
v2: patch updated to add nearby ret initializations. Please let me know
if some other constants should be returned on the failure of
devm_request_and_ioremap and devm_request_irq.
drivers/media/video/mx2_emmaprp.c | 33 -
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 5f8a6f5..9fe9ec6 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -874,29 +874,27 @@ static int emmaprp_probe(struct platform_device *pdev)
int irq_emma;
int ret;
- pcdev = kzalloc(sizeof *pcdev, GFP_KERNEL);
+ pcdev = devm_kzalloc(>dev, sizeof(*pcdev), GFP_KERNEL);
if (!pcdev)
return -ENOMEM;
spin_lock_init(>irqlock);
- pcdev->clk_emma = clk_get(>dev, NULL);
+ pcdev->clk_emma = devm_clk_get(>dev, NULL);
if (IS_ERR(pcdev->clk_emma)) {
- ret = PTR_ERR(pcdev->clk_emma);
- goto free_dev;
+ return PTR_ERR(pcdev->clk_emma);
}
irq_emma = platform_get_irq(pdev, 0);
res_emma = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (irq_emma < 0 || res_emma == NULL) {
dev_err(>dev, "Missing platform resources data\n");
- ret = -ENODEV;
- goto free_clk;
+ return -ENODEV;
}
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto free_clk;
+ return ret;
mutex_init(>dev_mutex);
@@ -922,21 +920,20 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
- if (devm_request_mem_region(>dev, res_emma->start,
- resource_size(res_emma), MEM2MEM_NAME) == NULL)
- goto rel_vdev;
-
- pcdev->base_emma = devm_ioremap(>dev, res_emma->start,
- resource_size(res_emma));
- if (!pcdev->base_emma)
+ pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma);
+ if (!pcdev->base_emma) {
+ ret = -ENXIO;
goto rel_vdev;
+ }
pcdev->irq_emma = irq_emma;
pcdev->res_emma = res_emma;
if (devm_request_irq(>dev, pcdev->irq_emma, emmaprp_irq,
-0, MEM2MEM_NAME, pcdev) < 0)
+0, MEM2MEM_NAME, pcdev) < 0) {
+ ret = -ENODEV;
goto rel_vdev;
+ }
pcdev->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
if (IS_ERR(pcdev->alloc_ctx)) {
@@ -969,10 +966,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(>v4l2_dev);
-free_clk:
- clk_put(pcdev->clk_emma);
-free_dev:
- kfree(pcdev);
return ret;
}
@@ -987,8 +980,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev->m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
v4l2_device_unregister(>v4l2_dev);
- clk_put(pcdev->clk_emma);
- kfree(pcdev);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
From: Julia Lawall julia.law...@lip6.fr
Using devm_kzalloc and devm_clk_get simplifies the code and ensures that
the use of devm_request_irq is safe. When kzalloc and kfree were used, the
interrupt could be triggered after the handler's data argument had been
freed.
Add missing return code initializations in the error handling code for
devm_request_and_ioremap and devm_request_irq.
The problem of a free after a devm_request_irq was found using the
following semantic match (http://coccinelle.lip6.fr/)
// smpl
@r exists@
expression e1,e2,x,a,b,c,d;
identifier free;
position p1,p2;
@@
devm_request_irq@p1(e1,e2,...,x)
... when any
when != e2 = a
when != x = b
if (...) {
... when != e2 = c
when != x = d
free@p2(...,x,...);
...
return ...;
}
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
v2: patch updated to add nearby ret initializations. Please let me know
if some other constants should be returned on the failure of
devm_request_and_ioremap and devm_request_irq.
drivers/media/video/mx2_emmaprp.c | 33 -
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
index 5f8a6f5..9fe9ec6 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -874,29 +874,27 @@ static int emmaprp_probe(struct platform_device *pdev)
int irq_emma;
int ret;
- pcdev = kzalloc(sizeof *pcdev, GFP_KERNEL);
+ pcdev = devm_kzalloc(pdev-dev, sizeof(*pcdev), GFP_KERNEL);
if (!pcdev)
return -ENOMEM;
spin_lock_init(pcdev-irqlock);
- pcdev-clk_emma = clk_get(pdev-dev, NULL);
+ pcdev-clk_emma = devm_clk_get(pdev-dev, NULL);
if (IS_ERR(pcdev-clk_emma)) {
- ret = PTR_ERR(pcdev-clk_emma);
- goto free_dev;
+ return PTR_ERR(pcdev-clk_emma);
}
irq_emma = platform_get_irq(pdev, 0);
res_emma = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (irq_emma 0 || res_emma == NULL) {
dev_err(pdev-dev, Missing platform resources data\n);
- ret = -ENODEV;
- goto free_clk;
+ return -ENODEV;
}
ret = v4l2_device_register(pdev-dev, pcdev-v4l2_dev);
if (ret)
- goto free_clk;
+ return ret;
mutex_init(pcdev-dev_mutex);
@@ -922,21 +920,20 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
- if (devm_request_mem_region(pdev-dev, res_emma-start,
- resource_size(res_emma), MEM2MEM_NAME) == NULL)
- goto rel_vdev;
-
- pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start,
- resource_size(res_emma));
- if (!pcdev-base_emma)
+ pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma);
+ if (!pcdev-base_emma) {
+ ret = -ENXIO;
goto rel_vdev;
+ }
pcdev-irq_emma = irq_emma;
pcdev-res_emma = res_emma;
if (devm_request_irq(pdev-dev, pcdev-irq_emma, emmaprp_irq,
-0, MEM2MEM_NAME, pcdev) 0)
+0, MEM2MEM_NAME, pcdev) 0) {
+ ret = -ENODEV;
goto rel_vdev;
+ }
pcdev-alloc_ctx = vb2_dma_contig_init_ctx(pdev-dev);
if (IS_ERR(pcdev-alloc_ctx)) {
@@ -969,10 +966,6 @@ rel_vdev:
video_device_release(vfd);
unreg_dev:
v4l2_device_unregister(pcdev-v4l2_dev);
-free_clk:
- clk_put(pcdev-clk_emma);
-free_dev:
- kfree(pcdev);
return ret;
}
@@ -987,8 +980,6 @@ static int emmaprp_remove(struct platform_device *pdev)
v4l2_m2m_release(pcdev-m2m_dev);
vb2_dma_contig_cleanup_ctx(pcdev-alloc_ctx);
v4l2_device_unregister(pcdev-v4l2_dev);
- clk_put(pcdev-clk_emma);
- kfree(pcdev);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Quoting Lars-Peter Clausen : On 08/06/2012 04:26 PM, Dan Carpenter wrote: On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pcdev); - if (devm_request_mem_region(>dev, res_emma->start, - resource_size(res_emma), MEM2MEM_NAME) == NULL) - goto rel_vdev; - - pcdev->base_emma = devm_ioremap(>dev, res_emma->start, - resource_size(res_emma)); + pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma); if (!pcdev->base_emma) goto rel_vdev; This was in the original code, but there is a "ret = -ENOMEM;" missing here, and again a couple lines down in the original code. Or should that be -EIO instead of -ENOMEM? I'm not sure. -ENXIO is usually used in such a case. Thanks for the feedback. I won't be able to access my work machine until the end of the week, so I will fix it then. julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Quoting Lars-Peter Clausen : On 08/06/2012 04:26 PM, Dan Carpenter wrote: On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pcdev); - if (devm_request_mem_region(>dev, res_emma->start, - resource_size(res_emma), MEM2MEM_NAME) == NULL) - goto rel_vdev; - - pcdev->base_emma = devm_ioremap(>dev, res_emma->start, - resource_size(res_emma)); + pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma); if (!pcdev->base_emma) goto rel_vdev; This was in the original code, but there is a "ret = -ENOMEM;" missing here, and again a couple lines down in the original code. Or should that be -EIO instead of -ENOMEM? I'm not sure. -ENXIO is usually used in such a case. Thanks for the feedback. I won't be able to access my work machine until the end of the week, so I will fix it then. julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On 08/06/2012 04:26 PM, Dan Carpenter wrote: > On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: >> On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: >>> @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) >>> >>> platform_set_drvdata(pdev, pcdev); >>> >>> - if (devm_request_mem_region(>dev, res_emma->start, >>> - resource_size(res_emma), MEM2MEM_NAME) == NULL) >>> - goto rel_vdev; >>> - >>> - pcdev->base_emma = devm_ioremap(>dev, res_emma->start, >>> - resource_size(res_emma)); >>> + pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma); >>> if (!pcdev->base_emma) >>> goto rel_vdev; >> >> This was in the original code, but there is a "ret = -ENOMEM;" >> missing here, and again a couple lines down in the original code. >> > > Or should that be -EIO instead of -ENOMEM? I'm not sure. -ENXIO is usually used in such a case. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: > On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: > > @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) > > > > platform_set_drvdata(pdev, pcdev); > > > > - if (devm_request_mem_region(>dev, res_emma->start, > > - resource_size(res_emma), MEM2MEM_NAME) == NULL) > > - goto rel_vdev; > > - > > - pcdev->base_emma = devm_ioremap(>dev, res_emma->start, > > - resource_size(res_emma)); > > + pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma); > > if (!pcdev->base_emma) > > goto rel_vdev; > > This was in the original code, but there is a "ret = -ENOMEM;" > missing here, and again a couple lines down in the original code. > Or should that be -EIO instead of -ENOMEM? I'm not sure. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: > @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, pcdev); > > - if (devm_request_mem_region(>dev, res_emma->start, > - resource_size(res_emma), MEM2MEM_NAME) == NULL) > - goto rel_vdev; > - > - pcdev->base_emma = devm_ioremap(>dev, res_emma->start, > - resource_size(res_emma)); > + pcdev->base_emma = devm_request_and_ioremap(>dev, res_emma); > if (!pcdev->base_emma) > goto rel_vdev; This was in the original code, but there is a "ret = -ENOMEM;" missing here, and again a couple lines down in the original code. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pcdev); - if (devm_request_mem_region(pdev-dev, res_emma-start, - resource_size(res_emma), MEM2MEM_NAME) == NULL) - goto rel_vdev; - - pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start, - resource_size(res_emma)); + pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma); if (!pcdev-base_emma) goto rel_vdev; This was in the original code, but there is a ret = -ENOMEM; missing here, and again a couple lines down in the original code. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pcdev); - if (devm_request_mem_region(pdev-dev, res_emma-start, - resource_size(res_emma), MEM2MEM_NAME) == NULL) - goto rel_vdev; - - pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start, - resource_size(res_emma)); + pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma); if (!pcdev-base_emma) goto rel_vdev; This was in the original code, but there is a ret = -ENOMEM; missing here, and again a couple lines down in the original code. Or should that be -EIO instead of -ENOMEM? I'm not sure. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
On 08/06/2012 04:26 PM, Dan Carpenter wrote: On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pcdev); - if (devm_request_mem_region(pdev-dev, res_emma-start, - resource_size(res_emma), MEM2MEM_NAME) == NULL) - goto rel_vdev; - - pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start, - resource_size(res_emma)); + pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma); if (!pcdev-base_emma) goto rel_vdev; This was in the original code, but there is a ret = -ENOMEM; missing here, and again a couple lines down in the original code. Or should that be -EIO instead of -ENOMEM? I'm not sure. -ENXIO is usually used in such a case. - Lars -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Quoting Lars-Peter Clausen l...@metafoo.de: On 08/06/2012 04:26 PM, Dan Carpenter wrote: On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pcdev); - if (devm_request_mem_region(pdev-dev, res_emma-start, - resource_size(res_emma), MEM2MEM_NAME) == NULL) - goto rel_vdev; - - pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start, - resource_size(res_emma)); + pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma); if (!pcdev-base_emma) goto rel_vdev; This was in the original code, but there is a ret = -ENOMEM; missing here, and again a couple lines down in the original code. Or should that be -EIO instead of -ENOMEM? I'm not sure. -ENXIO is usually used in such a case. Thanks for the feedback. I won't be able to access my work machine until the end of the week, so I will fix it then. julia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/media/video/mx2_emmaprp.c: use devm_kzalloc and devm_clk_get
Quoting Lars-Peter Clausen l...@metafoo.de: On 08/06/2012 04:26 PM, Dan Carpenter wrote: On Mon, Aug 06, 2012 at 05:23:23PM +0300, Dan Carpenter wrote: On Sat, Aug 04, 2012 at 08:23:27PM +0200, Julia Lawall wrote: @@ -922,12 +920,7 @@ static int emmaprp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pcdev); - if (devm_request_mem_region(pdev-dev, res_emma-start, - resource_size(res_emma), MEM2MEM_NAME) == NULL) - goto rel_vdev; - - pcdev-base_emma = devm_ioremap(pdev-dev, res_emma-start, - resource_size(res_emma)); + pcdev-base_emma = devm_request_and_ioremap(pdev-dev, res_emma); if (!pcdev-base_emma) goto rel_vdev; This was in the original code, but there is a ret = -ENOMEM; missing here, and again a couple lines down in the original code. Or should that be -EIO instead of -ENOMEM? I'm not sure. -ENXIO is usually used in such a case. Thanks for the feedback. I won't be able to access my work machine until the end of the week, so I will fix it then. julia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
