Message ID | alpine.DEB.2.02.1208132219060.2355@localhost6.localdomain6 (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Em 13-08-2012 17:20, Julia Lawall escreveu: > From: Julia Lawall <Julia.Lawall@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.Lawall@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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }