* [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-07-11 8:58 ` Julia Lawall 0 siblings, 0 replies; 24+ messages in thread From: Julia Lawall @ 2012-07-11 8:58 UTC (permalink / raw) To: David Woodhouse, Stefan Roese; +Cc: linux-mtd, kernel-janitors, linux-kernel From: Julia Lawall <Julia.Lawall@lip6.fr> dev_get_platdata returns a pointer, so the failure value would be NULL rather than a negative integer. The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) // <smpl> @@ expression x,e; statement S1,S2; @@ *x = dev_get_platdata(...) ... when != x = e *if (x < 0) S1 else S2 // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- drivers/mtd/devices/spear_smi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index b85f183..7c10466 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -935,7 +935,7 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } else { pdata = dev_get_platdata(&pdev->dev); - if (pdata < 0) { + if (!pdata) { ret = -ENODEV; dev_err(&pdev->dev, "no platform data\n"); goto err; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-07-11 8:58 ` Julia Lawall 0 siblings, 0 replies; 24+ messages in thread From: Julia Lawall @ 2012-07-11 8:58 UTC (permalink / raw) To: David Woodhouse, Stefan Roese; +Cc: kernel-janitors, linux-mtd, linux-kernel From: Julia Lawall <Julia.Lawall@lip6.fr> dev_get_platdata returns a pointer, so the failure value would be NULL rather than a negative integer. The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) // <smpl> @@ expression x,e; statement S1,S2; @@ *x = dev_get_platdata(...) ... when != x = e *if (x < 0) S1 else S2 // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- drivers/mtd/devices/spear_smi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index b85f183..7c10466 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -935,7 +935,7 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } else { pdata = dev_get_platdata(&pdev->dev); - if (pdata < 0) { + if (!pdata) { ret = -ENODEV; dev_err(&pdev->dev, "no platform data\n"); goto err; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-07-11 8:58 ` Julia Lawall 0 siblings, 0 replies; 24+ messages in thread From: Julia Lawall @ 2012-07-11 8:58 UTC (permalink / raw) To: David Woodhouse, Stefan Roese; +Cc: linux-mtd, kernel-janitors, linux-kernel From: Julia Lawall <Julia.Lawall@lip6.fr> dev_get_platdata returns a pointer, so the failure value would be NULL rather than a negative integer. The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) // <smpl> @@ expression x,e; statement S1,S2; @@ *x = dev_get_platdata(...) ... when != x = e *if (x < 0) S1 else S2 // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- drivers/mtd/devices/spear_smi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index b85f183..7c10466 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -935,7 +935,7 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } else { pdata = dev_get_platdata(&pdev->dev); - if (pdata < 0) { + if (!pdata) { ret = -ENODEV; dev_err(&pdev->dev, "no platform data\n"); goto err; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer 2012-07-11 8:58 ` Julia Lawall (?) @ 2012-07-11 9:01 ` viresh kumar -1 siblings, 0 replies; 24+ messages in thread From: viresh kumar @ 2012-07-11 9:01 UTC (permalink / raw) To: Julia Lawall, Shiraz Hashim Cc: spear-devel, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, Stefan Roese, David Woodhouse Adding spear-devel mailing list in cc. @Shiraz: Can you update ST's mailing list address in entire MAINTAINERS file, so that people can include ST's list easily? On 11/07/12 09:58, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression x,e; > statement S1,S2; > @@ > > *x = dev_get_platdata(...) > ... when != x = e > *if (x < 0) S1 else S2 > // </smpl> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > --- > drivers/mtd/devices/spear_smi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c > index b85f183..7c10466 100644 > --- a/drivers/mtd/devices/spear_smi.c > +++ b/drivers/mtd/devices/spear_smi.c > @@ -935,7 +935,7 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) > } > } else { > pdata = dev_get_platdata(&pdev->dev); > - if (pdata < 0) { > + if (!pdata) { > ret = -ENODEV; > dev_err(&pdev->dev, "no platform data\n"); > goto err; Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> -- Viresh -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-07-11 9:01 ` viresh kumar 0 siblings, 0 replies; 24+ messages in thread From: viresh kumar @ 2012-07-11 9:01 UTC (permalink / raw) To: Julia Lawall, Shiraz Hashim Cc: David Woodhouse, Stefan Roese, kernel-janitors@vger.kernel.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, spear-devel Adding spear-devel mailing list in cc. @Shiraz: Can you update ST's mailing list address in entire MAINTAINERS file, so that people can include ST's list easily? On 11/07/12 09:58, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression x,e; > statement S1,S2; > @@ > > *x = dev_get_platdata(...) > ... when != x = e > *if (x < 0) S1 else S2 > // </smpl> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > --- > drivers/mtd/devices/spear_smi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c > index b85f183..7c10466 100644 > --- a/drivers/mtd/devices/spear_smi.c > +++ b/drivers/mtd/devices/spear_smi.c > @@ -935,7 +935,7 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) > } > } else { > pdata = dev_get_platdata(&pdev->dev); > - if (pdata < 0) { > + if (!pdata) { > ret = -ENODEV; > dev_err(&pdev->dev, "no platform data\n"); > goto err; Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> -- Viresh -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-07-11 9:01 ` viresh kumar 0 siblings, 0 replies; 24+ messages in thread From: viresh kumar @ 2012-07-11 9:01 UTC (permalink / raw) To: Julia Lawall, Shiraz Hashim Cc: spear-devel, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, Stefan Roese, David Woodhouse Adding spear-devel mailing list in cc. @Shiraz: Can you update ST's mailing list address in entire MAINTAINERS file, so that people can include ST's list easily? On 11/07/12 09:58, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression x,e; > statement S1,S2; > @@ > > *x = dev_get_platdata(...) > ... when != x = e > *if (x < 0) S1 else S2 > // </smpl> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > --- > drivers/mtd/devices/spear_smi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c > index b85f183..7c10466 100644 > --- a/drivers/mtd/devices/spear_smi.c > +++ b/drivers/mtd/devices/spear_smi.c > @@ -935,7 +935,7 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) > } > } else { > pdata = dev_get_platdata(&pdev->dev); > - if (pdata < 0) { > + if (!pdata) { > ret = -ENODEV; > dev_err(&pdev->dev, "no platform data\n"); > goto err; Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> -- Viresh -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer 2012-07-11 8:58 ` Julia Lawall (?) @ 2012-07-16 9:38 ` Stefan Roese -1 siblings, 0 replies; 24+ messages in thread From: Stefan Roese @ 2012-07-16 9:38 UTC (permalink / raw) To: Julia Lawall; +Cc: kernel-janitors, David Woodhouse, linux-kernel, linux-mtd On Wednesday 11 July 2012 10:58:38 Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression x,e; > statement S1,S2; > @@ > > *x = dev_get_platdata(...) > ... when != x = e > *if (x < 0) S1 else S2 > // </smpl> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Acked-by: Stefan Roese <sr@denx.de> Thanks, Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-07-16 9:38 ` Stefan Roese 0 siblings, 0 replies; 24+ messages in thread From: Stefan Roese @ 2012-07-16 9:38 UTC (permalink / raw) To: Julia Lawall; +Cc: David Woodhouse, kernel-janitors, linux-mtd, linux-kernel On Wednesday 11 July 2012 10:58:38 Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression x,e; > statement S1,S2; > @@ > > *x = dev_get_platdata(...) > ... when != x = e > *if (x < 0) S1 else S2 > // </smpl> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Acked-by: Stefan Roese <sr@denx.de> Thanks, Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-07-16 9:38 ` Stefan Roese 0 siblings, 0 replies; 24+ messages in thread From: Stefan Roese @ 2012-07-16 9:38 UTC (permalink / raw) To: Julia Lawall; +Cc: kernel-janitors, David Woodhouse, linux-kernel, linux-mtd On Wednesday 11 July 2012 10:58:38 Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression x,e; > statement S1,S2; > @@ > > *x = dev_get_platdata(...) > ... when != x = e > *if (x < 0) S1 else S2 > // </smpl> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Acked-by: Stefan Roese <sr@denx.de> Thanks, Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-04 20:36 ` Julia Lawall 0 siblings, 0 replies; 24+ messages in thread From: Julia Lawall @ 2012-08-04 20:36 UTC (permalink / raw) To: David Woodhouse; +Cc: linux-mtd, kernel-janitors, linux-kernel From: Julia Lawall <Julia.Lawall@lip6.fr> Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- drivers/mtd/devices/spear_smi.c | 72 +++++++++------------------------------- 1 file changed, 17 insertions(+), 55 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index b85f183..f889fad 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-04 20:36 ` Julia Lawall 0 siblings, 0 replies; 24+ messages in thread From: Julia Lawall @ 2012-08-04 20:36 UTC (permalink / raw) To: David Woodhouse; +Cc: kernel-janitors, linux-mtd, linux-kernel From: Julia Lawall <Julia.Lawall@lip6.fr> Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- drivers/mtd/devices/spear_smi.c | 72 +++++++++------------------------------- 1 file changed, 17 insertions(+), 55 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index b85f183..f889fad 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-04 20:36 ` Julia Lawall 0 siblings, 0 replies; 24+ messages in thread From: Julia Lawall @ 2012-08-04 20:36 UTC (permalink / raw) To: David Woodhouse; +Cc: linux-mtd, kernel-janitors, linux-kernel From: Julia Lawall <Julia.Lawall@lip6.fr> Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- drivers/mtd/devices/spear_smi.c | 72 +++++++++------------------------------- 1 file changed, 17 insertions(+), 55 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index b85f183..f889fad 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently 2012-08-04 20:36 ` Julia Lawall (?) @ 2012-08-24 11:35 ` Artem Bityutskiy -1 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-24 11:35 UTC (permalink / raw) To: Julia Lawall, Stefan Roese, Shiraz Hashim Cc: kernel-janitors, David Woodhouse, linux-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 2556 bytes --] [See here for the original patch: http://lists.infradead.org/pipermail/linux-mtd/2012-August/043112.html] Julia, sorry for long delay, aiaiai gives says that this patch introduced 3 warnings: -------------------------------------------------------------------------------- Successfully built configuration "arm-spear6xx_defconfig,arm,arm-unknown-linux-gnueabi-", results: --- before_patching.log +++ after_patching.log @@ @@ -drivers/mtd/devices/spear_smi.c: In function 'spear_smi_remove': -drivers/mtd/devices/spear_smi.c:1022:30: warning: variable 'pdata' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c: In function 'spear_smi_remove': +drivers/mtd/devices/spear_smi.c:1026:9: warning: variable 'irq' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c:1022:30: warning: variable 'pdata' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c:1024:19: warning: variable 'smi_base' set but not used [-Wunused-but-set-variable] -------------------------------------------------------------------------------- On Sat, 2012-08-04 at 22:36 +0200, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm > functions for other allocations as well. > > Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to > where its result is passed to devm_request_and_ioremap to make the lack of > need for a NULL test more evident. > > The semantic match that finds the inconsistency is as follows: > (http://coccinelle.lip6.fr/) ... > @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) > ret = mtd_device_unregister(&flash->mtd); > if (ret) > dev_err(&pdev->dev, "error removing mtd\n"); > - > - iounmap(flash->base_addr); > - kfree(flash); > } > > irq = platform_get_irq(pdev, 0); > - free_irq(irq, dev); I guess 'platform_get_irq()' should be killed as well? Stefan, this is strange code - we get irq, without checking for error, and then free it? What is the rationale? > clk_disable_unprepare(dev->clk); > - clk_put(dev->clk); > - iounmap(dev->io_base); > - kfree(dev); > > smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - release_mem_region(smi_base->start, resource_size(smi_base)); > platform_set_drvdata(pdev, NULL); Why do we set platform data to NULL, is this needed? -- 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] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-24 11:35 ` Artem Bityutskiy 0 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-24 11:35 UTC (permalink / raw) To: Julia Lawall, Stefan Roese, Shiraz Hashim Cc: David Woodhouse, kernel-janitors, linux-mtd, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2556 bytes --] [See here for the original patch: http://lists.infradead.org/pipermail/linux-mtd/2012-August/043112.html] Julia, sorry for long delay, aiaiai gives says that this patch introduced 3 warnings: -------------------------------------------------------------------------------- Successfully built configuration "arm-spear6xx_defconfig,arm,arm-unknown-linux-gnueabi-", results: --- before_patching.log +++ after_patching.log @@ @@ -drivers/mtd/devices/spear_smi.c: In function 'spear_smi_remove': -drivers/mtd/devices/spear_smi.c:1022:30: warning: variable 'pdata' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c: In function 'spear_smi_remove': +drivers/mtd/devices/spear_smi.c:1026:9: warning: variable 'irq' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c:1022:30: warning: variable 'pdata' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c:1024:19: warning: variable 'smi_base' set but not used [-Wunused-but-set-variable] -------------------------------------------------------------------------------- On Sat, 2012-08-04 at 22:36 +0200, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm > functions for other allocations as well. > > Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to > where its result is passed to devm_request_and_ioremap to make the lack of > need for a NULL test more evident. > > The semantic match that finds the inconsistency is as follows: > (http://coccinelle.lip6.fr/) ... > @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) > ret = mtd_device_unregister(&flash->mtd); > if (ret) > dev_err(&pdev->dev, "error removing mtd\n"); > - > - iounmap(flash->base_addr); > - kfree(flash); > } > > irq = platform_get_irq(pdev, 0); > - free_irq(irq, dev); I guess 'platform_get_irq()' should be killed as well? Stefan, this is strange code - we get irq, without checking for error, and then free it? What is the rationale? > clk_disable_unprepare(dev->clk); > - clk_put(dev->clk); > - iounmap(dev->io_base); > - kfree(dev); > > smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - release_mem_region(smi_base->start, resource_size(smi_base)); > platform_set_drvdata(pdev, NULL); Why do we set platform data to NULL, is this needed? -- 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] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-24 11:35 ` Artem Bityutskiy 0 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-24 11:35 UTC (permalink / raw) To: Julia Lawall, Stefan Roese, Shiraz Hashim Cc: kernel-janitors, David Woodhouse, linux-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 2556 bytes --] [See here for the original patch: http://lists.infradead.org/pipermail/linux-mtd/2012-August/043112.html] Julia, sorry for long delay, aiaiai gives says that this patch introduced 3 warnings: -------------------------------------------------------------------------------- Successfully built configuration "arm-spear6xx_defconfig,arm,arm-unknown-linux-gnueabi-", results: --- before_patching.log +++ after_patching.log @@ @@ -drivers/mtd/devices/spear_smi.c: In function 'spear_smi_remove': -drivers/mtd/devices/spear_smi.c:1022:30: warning: variable 'pdata' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c: In function 'spear_smi_remove': +drivers/mtd/devices/spear_smi.c:1026:9: warning: variable 'irq' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c:1022:30: warning: variable 'pdata' set but not used [-Wunused-but-set-variable] +drivers/mtd/devices/spear_smi.c:1024:19: warning: variable 'smi_base' set but not used [-Wunused-but-set-variable] -------------------------------------------------------------------------------- On Sat, 2012-08-04 at 22:36 +0200, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm > functions for other allocations as well. > > Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to > where its result is passed to devm_request_and_ioremap to make the lack of > need for a NULL test more evident. > > The semantic match that finds the inconsistency is as follows: > (http://coccinelle.lip6.fr/) ... > @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) > ret = mtd_device_unregister(&flash->mtd); > if (ret) > dev_err(&pdev->dev, "error removing mtd\n"); > - > - iounmap(flash->base_addr); > - kfree(flash); > } > > irq = platform_get_irq(pdev, 0); > - free_irq(irq, dev); I guess 'platform_get_irq()' should be killed as well? Stefan, this is strange code - we get irq, without checking for error, and then free it? What is the rationale? > clk_disable_unprepare(dev->clk); > - clk_put(dev->clk); > - iounmap(dev->io_base); > - kfree(dev); > > smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - release_mem_region(smi_base->start, resource_size(smi_base)); > platform_set_drvdata(pdev, NULL); Why do we set platform data to NULL, is this needed? -- 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] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently 2012-08-24 11:35 ` Artem Bityutskiy (?) @ 2012-08-24 13:14 ` Stefan Roese -1 siblings, 0 replies; 24+ messages in thread From: Stefan Roese @ 2012-08-24 13:14 UTC (permalink / raw) To: dedekind1 Cc: Shiraz Hashim, kernel-janitors, linux-kernel, Julia Lawall, linux-mtd, David Woodhouse On 08/24/2012 01:35 PM, Artem Bityutskiy wrote: >> @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) >> ret = mtd_device_unregister(&flash->mtd); >> if (ret) >> dev_err(&pdev->dev, "error removing mtd\n"); >> - >> - iounmap(flash->base_addr); >> - kfree(flash); >> } >> >> irq = platform_get_irq(pdev, 0); >> - free_irq(irq, dev); > > I guess 'platform_get_irq()' should be killed as well? Stefan, this is > strange code - we get irq, without checking for error, and then free it? > What is the rationale? Yes, this seems bogus. platform_get_irq() definitely should be removed from spear_smi_remove(). >> clk_disable_unprepare(dev->clk); >> - clk_put(dev->clk); >> - iounmap(dev->io_base); >> - kfree(dev); >> >> smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> - release_mem_region(smi_base->start, resource_size(smi_base)); >> platform_set_drvdata(pdev, NULL); > > Why do we set platform data to NULL, is this needed? It seems to be common practice to use this call to clear the drvdata in the driver remove function. I have to admit, that I'm not sure if its really needed though. Thanks, Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-24 13:14 ` Stefan Roese 0 siblings, 0 replies; 24+ messages in thread From: Stefan Roese @ 2012-08-24 13:14 UTC (permalink / raw) To: dedekind1 Cc: Julia Lawall, Shiraz Hashim, David Woodhouse, kernel-janitors, linux-mtd, linux-kernel On 08/24/2012 01:35 PM, Artem Bityutskiy wrote: >> @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) >> ret = mtd_device_unregister(&flash->mtd); >> if (ret) >> dev_err(&pdev->dev, "error removing mtd\n"); >> - >> - iounmap(flash->base_addr); >> - kfree(flash); >> } >> >> irq = platform_get_irq(pdev, 0); >> - free_irq(irq, dev); > > I guess 'platform_get_irq()' should be killed as well? Stefan, this is > strange code - we get irq, without checking for error, and then free it? > What is the rationale? Yes, this seems bogus. platform_get_irq() definitely should be removed from spear_smi_remove(). >> clk_disable_unprepare(dev->clk); >> - clk_put(dev->clk); >> - iounmap(dev->io_base); >> - kfree(dev); >> >> smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> - release_mem_region(smi_base->start, resource_size(smi_base)); >> platform_set_drvdata(pdev, NULL); > > Why do we set platform data to NULL, is this needed? It seems to be common practice to use this call to clear the drvdata in the driver remove function. I have to admit, that I'm not sure if its really needed though. Thanks, Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-24 13:14 ` Stefan Roese 0 siblings, 0 replies; 24+ messages in thread From: Stefan Roese @ 2012-08-24 13:14 UTC (permalink / raw) To: dedekind1 Cc: Shiraz Hashim, kernel-janitors, linux-kernel, Julia Lawall, linux-mtd, David Woodhouse On 08/24/2012 01:35 PM, Artem Bityutskiy wrote: >> @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) >> ret = mtd_device_unregister(&flash->mtd); >> if (ret) >> dev_err(&pdev->dev, "error removing mtd\n"); >> - >> - iounmap(flash->base_addr); >> - kfree(flash); >> } >> >> irq = platform_get_irq(pdev, 0); >> - free_irq(irq, dev); > > I guess 'platform_get_irq()' should be killed as well? Stefan, this is > strange code - we get irq, without checking for error, and then free it? > What is the rationale? Yes, this seems bogus. platform_get_irq() definitely should be removed from spear_smi_remove(). >> clk_disable_unprepare(dev->clk); >> - clk_put(dev->clk); >> - iounmap(dev->io_base); >> - kfree(dev); >> >> smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> - release_mem_region(smi_base->start, resource_size(smi_base)); >> platform_set_drvdata(pdev, NULL); > > Why do we set platform data to NULL, is this needed? It seems to be common practice to use this call to clear the drvdata in the driver remove function. I have to admit, that I'm not sure if its really needed though. Thanks, Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently 2012-08-24 13:14 ` Stefan Roese (?) @ 2012-08-24 14:43 ` Artem Bityutskiy -1 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-24 14:43 UTC (permalink / raw) To: Stefan Roese Cc: Shiraz Hashim, kernel-janitors, linux-kernel, Julia Lawall, linux-mtd, David Woodhouse [-- Attachment #1.1: Type: text/plain, Size: 7045 bytes --] On Fri, 2012-08-24 at 15:14 +0200, Stefan Roese wrote: > On 08/24/2012 01:35 PM, Artem Bityutskiy wrote: > >> @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) > >> ret = mtd_device_unregister(&flash->mtd); > >> if (ret) > >> dev_err(&pdev->dev, "error removing mtd\n"); > >> - > >> - iounmap(flash->base_addr); > >> - kfree(flash); > >> } > >> > >> irq = platform_get_irq(pdev, 0); > >> - free_irq(irq, dev); > > > > I guess 'platform_get_irq()' should be killed as well? Stefan, this is > > strange code - we get irq, without checking for error, and then free it? > > What is the rationale? > > Yes, this seems bogus. platform_get_irq() definitely should be removed > from spear_smi_remove(). OK, thanks, I've massaged Julia's patch and pushed the following (also attached) to l2-mtd.git, compile-tested only. From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001 From: Julia Lawall <Julia.Lawall@lip6.fr> Date: Sat, 4 Aug 2012 22:36:38 +0200 Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> --- drivers/mtd/devices/spear_smi.c | 83 +++++++++------------------------------ 1 file changed, 18 insertions(+), 65 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index 421bc65..dcc3c95 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1049,11 +1019,8 @@ err: static int __devexit spear_smi_remove(struct platform_device *pdev) { struct spear_smi *dev; - struct spear_smi_plat_data *pdata; struct spear_snor_flash *flash; - struct resource *smi_base; - int ret; - int i, irq; + int ret, i; dev = platform_get_drvdata(pdev); if (!dev) { @@ -1061,8 +1028,6 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) return -ENODEV; } - pdata = dev_get_platdata(&pdev->dev); - /* clean up for all nor flash */ for (i = 0; i < dev->num_flashes; i++) { flash = dev->flash[i]; @@ -1073,21 +1038,9 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } - irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); - clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); - - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; -- 1.7.10.4 -- Best Regards, Artem Bityutskiy [-- Attachment #1.2: 0001-mtd-spear_smi-use-devm_-functions-consistently.patch --] [-- Type: text/x-patch, Size: 6107 bytes --] From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001 From: Julia Lawall <Julia.Lawall@lip6.fr> Date: Sat, 4 Aug 2012 22:36:38 +0200 Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> --- drivers/mtd/devices/spear_smi.c | 83 +++++++++------------------------------ 1 file changed, 18 insertions(+), 65 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index 421bc65..dcc3c95 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1049,11 +1019,8 @@ err: static int __devexit spear_smi_remove(struct platform_device *pdev) { struct spear_smi *dev; - struct spear_smi_plat_data *pdata; struct spear_snor_flash *flash; - struct resource *smi_base; - int ret; - int i, irq; + int ret, i; dev = platform_get_drvdata(pdev); if (!dev) { @@ -1061,8 +1028,6 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) return -ENODEV; } - pdata = dev_get_platdata(&pdev->dev); - /* clean up for all nor flash */ for (i = 0; i < dev->num_flashes; i++) { flash = dev->flash[i]; @@ -1073,21 +1038,9 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } - irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); - clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); - - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; -- 1.7.10.4 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-24 14:43 ` Artem Bityutskiy 0 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-24 14:43 UTC (permalink / raw) To: Stefan Roese Cc: Julia Lawall, Shiraz Hashim, David Woodhouse, kernel-janitors, linux-mtd, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 7045 bytes --] On Fri, 2012-08-24 at 15:14 +0200, Stefan Roese wrote: > On 08/24/2012 01:35 PM, Artem Bityutskiy wrote: > >> @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) > >> ret = mtd_device_unregister(&flash->mtd); > >> if (ret) > >> dev_err(&pdev->dev, "error removing mtd\n"); > >> - > >> - iounmap(flash->base_addr); > >> - kfree(flash); > >> } > >> > >> irq = platform_get_irq(pdev, 0); > >> - free_irq(irq, dev); > > > > I guess 'platform_get_irq()' should be killed as well? Stefan, this is > > strange code - we get irq, without checking for error, and then free it? > > What is the rationale? > > Yes, this seems bogus. platform_get_irq() definitely should be removed > from spear_smi_remove(). OK, thanks, I've massaged Julia's patch and pushed the following (also attached) to l2-mtd.git, compile-tested only. From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001 From: Julia Lawall <Julia.Lawall@lip6.fr> Date: Sat, 4 Aug 2012 22:36:38 +0200 Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> --- drivers/mtd/devices/spear_smi.c | 83 +++++++++------------------------------ 1 file changed, 18 insertions(+), 65 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index 421bc65..dcc3c95 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1049,11 +1019,8 @@ err: static int __devexit spear_smi_remove(struct platform_device *pdev) { struct spear_smi *dev; - struct spear_smi_plat_data *pdata; struct spear_snor_flash *flash; - struct resource *smi_base; - int ret; - int i, irq; + int ret, i; dev = platform_get_drvdata(pdev); if (!dev) { @@ -1061,8 +1028,6 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) return -ENODEV; } - pdata = dev_get_platdata(&pdev->dev); - /* clean up for all nor flash */ for (i = 0; i < dev->num_flashes; i++) { flash = dev->flash[i]; @@ -1073,21 +1038,9 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } - irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); - clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); - - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; -- 1.7.10.4 -- Best Regards, Artem Bityutskiy [-- Attachment #1.2: 0001-mtd-spear_smi-use-devm_-functions-consistently.patch --] [-- Type: text/x-patch, Size: 6107 bytes --] From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001 From: Julia Lawall <Julia.Lawall@lip6.fr> Date: Sat, 4 Aug 2012 22:36:38 +0200 Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> --- drivers/mtd/devices/spear_smi.c | 83 +++++++++------------------------------ 1 file changed, 18 insertions(+), 65 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index 421bc65..dcc3c95 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1049,11 +1019,8 @@ err: static int __devexit spear_smi_remove(struct platform_device *pdev) { struct spear_smi *dev; - struct spear_smi_plat_data *pdata; struct spear_snor_flash *flash; - struct resource *smi_base; - int ret; - int i, irq; + int ret, i; dev = platform_get_drvdata(pdev); if (!dev) { @@ -1061,8 +1028,6 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) return -ENODEV; } - pdata = dev_get_platdata(&pdev->dev); - /* clean up for all nor flash */ for (i = 0; i < dev->num_flashes; i++) { flash = dev->flash[i]; @@ -1073,21 +1038,9 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } - irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); - clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); - - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; -- 1.7.10.4 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently @ 2012-08-24 14:43 ` Artem Bityutskiy 0 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-24 14:43 UTC (permalink / raw) To: Stefan Roese Cc: Shiraz Hashim, kernel-janitors, linux-kernel, Julia Lawall, linux-mtd, David Woodhouse [-- Attachment #1.1: Type: text/plain, Size: 7045 bytes --] On Fri, 2012-08-24 at 15:14 +0200, Stefan Roese wrote: > On 08/24/2012 01:35 PM, Artem Bityutskiy wrote: > >> @@ -1073,21 +1043,13 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) > >> ret = mtd_device_unregister(&flash->mtd); > >> if (ret) > >> dev_err(&pdev->dev, "error removing mtd\n"); > >> - > >> - iounmap(flash->base_addr); > >> - kfree(flash); > >> } > >> > >> irq = platform_get_irq(pdev, 0); > >> - free_irq(irq, dev); > > > > I guess 'platform_get_irq()' should be killed as well? Stefan, this is > > strange code - we get irq, without checking for error, and then free it? > > What is the rationale? > > Yes, this seems bogus. platform_get_irq() definitely should be removed > from spear_smi_remove(). OK, thanks, I've massaged Julia's patch and pushed the following (also attached) to l2-mtd.git, compile-tested only. From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001 From: Julia Lawall <Julia.Lawall@lip6.fr> Date: Sat, 4 Aug 2012 22:36:38 +0200 Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> --- drivers/mtd/devices/spear_smi.c | 83 +++++++++------------------------------ 1 file changed, 18 insertions(+), 65 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index 421bc65..dcc3c95 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1049,11 +1019,8 @@ err: static int __devexit spear_smi_remove(struct platform_device *pdev) { struct spear_smi *dev; - struct spear_smi_plat_data *pdata; struct spear_snor_flash *flash; - struct resource *smi_base; - int ret; - int i, irq; + int ret, i; dev = platform_get_drvdata(pdev); if (!dev) { @@ -1061,8 +1028,6 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) return -ENODEV; } - pdata = dev_get_platdata(&pdev->dev); - /* clean up for all nor flash */ for (i = 0; i < dev->num_flashes; i++) { flash = dev->flash[i]; @@ -1073,21 +1038,9 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } - irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); - clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); - - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; -- 1.7.10.4 -- Best Regards, Artem Bityutskiy [-- Attachment #1.2: 0001-mtd-spear_smi-use-devm_-functions-consistently.patch --] [-- Type: text/x-patch, Size: 6107 bytes --] From d44ab8be1a01b83c744d8077a54064e179b0e2b8 Mon Sep 17 00:00:00 2001 From: Julia Lawall <Julia.Lawall@lip6.fr> Date: Sat, 4 Aug 2012 22:36:38 +0200 Subject: [PATCH] mtd: spear_smi: use devm_ functions consistently Use devm_kzalloc for all calls to kzalloc and not just the first. Use devm functions for other allocations as well. Move the call to platform_get_resource(pdev, IORESOURCE_MEM, 0) closer to where its result is passed to devm_request_and_ioremap to make the lack of need for a NULL test more evident. The semantic match that finds the inconsistency is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ @@ *devm_kzalloc(...) ... *kzalloc(...) // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> --- drivers/mtd/devices/spear_smi.c | 83 +++++++++------------------------------ 1 file changed, 18 insertions(+), 65 deletions(-) diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c index 421bc65..dcc3c95 100644 --- a/drivers/mtd/devices/spear_smi.c +++ b/drivers/mtd/devices/spear_smi.c @@ -827,7 +827,7 @@ static int spear_smi_setup_banks(struct platform_device *pdev, if (!flash_info) return -ENODEV; - flash = kzalloc(sizeof(*flash), GFP_ATOMIC); + flash = devm_kzalloc(&pdev->dev, sizeof(*flash), GFP_ATOMIC); if (!flash) return -ENOMEM; flash->bank = bank; @@ -838,15 +838,13 @@ static int spear_smi_setup_banks(struct platform_device *pdev, flash_index = spear_smi_probe_flash(dev, bank); if (flash_index < 0) { dev_info(&dev->pdev->dev, "smi-nor%d not found\n", bank); - ret = flash_index; - goto err_probe; + return flash_index; } /* map the memory for nor flash chip */ - flash->base_addr = ioremap(flash_info->mem_base, flash_info->size); - if (!flash->base_addr) { - ret = -EIO; - goto err_probe; - } + flash->base_addr = devm_ioremap(&pdev->dev, flash_info->mem_base, + flash_info->size); + if (!flash->base_addr) + return -EIO; dev->flash[bank] = flash; flash->mtd.priv = dev; @@ -888,17 +886,10 @@ static int spear_smi_setup_banks(struct platform_device *pdev, count); if (ret) { dev_err(&dev->pdev->dev, "Err MTD partition=%d\n", ret); - goto err_map; + return ret; } return 0; - -err_map: - iounmap(flash->base_addr); - -err_probe: - kfree(flash); - return ret; } /** @@ -942,13 +933,6 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) } } - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!smi_base) { - ret = -ENODEV; - dev_err(&pdev->dev, "invalid smi base address\n"); - goto err; - } - irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = -ENODEV; @@ -956,26 +940,20 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) goto err; } - dev = kzalloc(sizeof(*dev), GFP_ATOMIC); + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_ATOMIC); if (!dev) { ret = -ENOMEM; dev_err(&pdev->dev, "mem alloc fail\n"); goto err; } - smi_base = request_mem_region(smi_base->start, resource_size(smi_base), - pdev->name); - if (!smi_base) { - ret = -EBUSY; - dev_err(&pdev->dev, "request mem region fail\n"); - goto err_mem; - } + smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dev->io_base = ioremap(smi_base->start, resource_size(smi_base)); + dev->io_base = devm_request_and_ioremap(&pdev->dev, smi_base); if (!dev->io_base) { ret = -EIO; - dev_err(&pdev->dev, "ioremap fail\n"); - goto err_ioremap; + dev_err(&pdev->dev, "devm_request_and_ioremap fail\n"); + goto err; } dev->pdev = pdev; @@ -991,17 +969,18 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) dev->num_flashes = MAX_NUM_FLASH_CHIP; } - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) { ret = PTR_ERR(dev->clk); - goto err_clk; + goto err; } ret = clk_prepare_enable(dev->clk); if (ret) - goto err_clk_prepare_enable; + goto err; - ret = request_irq(irq, spear_smi_int_handler, 0, pdev->name, dev); + ret = devm_request_irq(&pdev->dev, irq, spear_smi_int_handler, 0, + pdev->name, dev); if (ret) { dev_err(&dev->pdev->dev, "SMI IRQ allocation failed\n"); goto err_irq; @@ -1024,18 +1003,9 @@ static int __devinit spear_smi_probe(struct platform_device *pdev) return 0; err_bank_setup: - free_irq(irq, dev); platform_set_drvdata(pdev, NULL); err_irq: clk_disable_unprepare(dev->clk); -err_clk_prepare_enable: - clk_put(dev->clk); -err_clk: - iounmap(dev->io_base); -err_ioremap: - release_mem_region(smi_base->start, resource_size(smi_base)); -err_mem: - kfree(dev); err: return ret; } @@ -1049,11 +1019,8 @@ err: static int __devexit spear_smi_remove(struct platform_device *pdev) { struct spear_smi *dev; - struct spear_smi_plat_data *pdata; struct spear_snor_flash *flash; - struct resource *smi_base; - int ret; - int i, irq; + int ret, i; dev = platform_get_drvdata(pdev); if (!dev) { @@ -1061,8 +1028,6 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) return -ENODEV; } - pdata = dev_get_platdata(&pdev->dev); - /* clean up for all nor flash */ for (i = 0; i < dev->num_flashes; i++) { flash = dev->flash[i]; @@ -1073,21 +1038,9 @@ static int __devexit spear_smi_remove(struct platform_device *pdev) ret = mtd_device_unregister(&flash->mtd); if (ret) dev_err(&pdev->dev, "error removing mtd\n"); - - iounmap(flash->base_addr); - kfree(flash); } - irq = platform_get_irq(pdev, 0); - free_irq(irq, dev); - clk_disable_unprepare(dev->clk); - clk_put(dev->clk); - iounmap(dev->io_base); - kfree(dev); - - smi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(smi_base->start, resource_size(smi_base)); platform_set_drvdata(pdev, NULL); return 0; -- 1.7.10.4 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer 2012-07-11 8:58 ` Julia Lawall (?) @ 2012-08-17 10:03 ` Artem Bityutskiy -1 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-17 9:59 UTC (permalink / raw) To: Julia Lawall Cc: kernel-janitors, Stefan Roese, David Woodhouse, linux-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 370 bytes --] On Wed, 2012-07-11 at 10:58 +0200, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (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] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-08-17 10:03 ` Artem Bityutskiy 0 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-17 10:03 UTC (permalink / raw) To: Julia Lawall Cc: David Woodhouse, Stefan Roese, kernel-janitors, linux-mtd, linux-kernel [-- Attachment #1: Type: text/plain, Size: 370 bytes --] On Wed, 2012-07-11 at 10:58 +0200, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (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] 24+ messages in thread
* Re: [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer @ 2012-08-17 10:03 ` Artem Bityutskiy 0 siblings, 0 replies; 24+ messages in thread From: Artem Bityutskiy @ 2012-08-17 10:03 UTC (permalink / raw) To: Julia Lawall Cc: kernel-janitors, Stefan Roese, David Woodhouse, linux-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 370 bytes --] On Wed, 2012-07-11 at 10:58 +0200, Julia Lawall wrote: > From: Julia Lawall <Julia.Lawall@lip6.fr> > > dev_get_platdata returns a pointer, so the failure value would be NULL > rather than a negative integer. > > The semantic match that finds this problem is: (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] 24+ messages in thread
end of thread, other threads:[~2012-08-24 14:43 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-11 8:58 [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer Julia Lawall 2012-07-11 8:58 ` Julia Lawall 2012-07-11 8:58 ` Julia Lawall 2012-07-11 9:01 ` viresh kumar 2012-07-11 9:01 ` viresh kumar 2012-07-11 9:01 ` viresh kumar 2012-07-16 9:38 ` Stefan Roese 2012-07-16 9:38 ` Stefan Roese 2012-07-16 9:38 ` Stefan Roese 2012-08-04 20:36 ` [PATCH] drivers/mtd/devices/spear_smi.c: use devm_ functions consistently Julia Lawall 2012-08-04 20:36 ` Julia Lawall 2012-08-04 20:36 ` Julia Lawall 2012-08-24 11:35 ` Artem Bityutskiy 2012-08-24 11:35 ` Artem Bityutskiy 2012-08-24 11:35 ` Artem Bityutskiy 2012-08-24 13:14 ` Stefan Roese 2012-08-24 13:14 ` Stefan Roese 2012-08-24 13:14 ` Stefan Roese 2012-08-24 14:43 ` Artem Bityutskiy 2012-08-24 14:43 ` Artem Bityutskiy 2012-08-24 14:43 ` Artem Bityutskiy 2012-08-17 9:59 ` [PATCH] drivers/mtd/devices/spear_smi.c: failure test for null rather than negative integer Artem Bityutskiy 2012-08-17 10:03 ` Artem Bityutskiy 2012-08-17 10:03 ` Artem Bityutskiy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.