* [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths @ 2017-12-09 7:24 Christophe JAILLET 2017-12-09 7:24 ` [PATCH 1/2 v2] mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks Christophe JAILLET ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Christophe JAILLET @ 2017-12-09 7:24 UTC (permalink / raw) To: kyungmin.park, dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard, cyrille.pitchen Cc: linux-mtd, linux-kernel, kernel-janitors, Christophe JAILLET The first patch converts 's3c_onenand_probe()' to devm_ functions. This fixes a leak in one path (line 872). This also free_irq which was not handled at all. (I hope I'm correct :) ) The 2nd patch is about an un-handled error code which looks spurious. Not sure if I'm right. While compile-testing it, I had to tweak the code because I don't have any cross-compiler. I commented the line "#include <asm/mach/flash.h>" and the compilation succeeded. So maybe, this include is also useless. I've left it as-is, though. Theses patches have been compile-tested-only. Christophe JAILLET (2): mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks mtd: onenand: samsung: return an error if 'mtd_device_parse_register()' fails drivers/mtd/onenand/samsung.c | 169 +++++++++--------------------------------- 1 file changed, 34 insertions(+), 135 deletions(-) -- 2.14.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2 v2] mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks 2017-12-09 7:24 [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths Christophe JAILLET @ 2017-12-09 7:24 ` Christophe JAILLET 2017-12-09 8:33 ` Boris Brezillon 2017-12-09 7:24 ` [PATCH 2/2 v2] mtd: onenand: samsung: return an error if 'mtd_device_parse_register()' fails Christophe JAILLET 2017-12-09 8:27 ` [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths Boris Brezillon 2 siblings, 1 reply; 5+ messages in thread From: Christophe JAILLET @ 2017-12-09 7:24 UTC (permalink / raw) To: kyungmin.park, dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard, cyrille.pitchen Cc: linux-mtd, linux-kernel, kernel-janitors, Christophe JAILLET Convert all error handling code in 's3c_onenand_probe()' to resource-managed alternatives in order to simplify code. This fixes a resource leak if 'platform_get_resource()' fails at line 872. The 'request_irq()' at line 971 was also un-balanced. It is now resource-managed. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Compile tested-only v2: use devm_ioremap_resource() --- drivers/mtd/onenand/samsung.c | 164 ++++++++---------------------------------- 1 file changed, 29 insertions(+), 135 deletions(-) diff --git a/drivers/mtd/onenand/samsung.c b/drivers/mtd/onenand/samsung.c index af0ac1a7bf8f..2e562c224060 100644 --- a/drivers/mtd/onenand/samsung.c +++ b/drivers/mtd/onenand/samsung.c @@ -129,16 +129,13 @@ struct s3c_onenand { struct platform_device *pdev; enum soc_type type; void __iomem *base; - struct resource *base_res; void __iomem *ahb_addr; - struct resource *ahb_res; int bootram_command; void __iomem *page_buf; void __iomem *oob_buf; unsigned int (*mem_addr)(int fba, int fpa, int fsa); unsigned int (*cmd_map)(unsigned int type, unsigned int val); void __iomem *dma_addr; - struct resource *dma_res; unsigned long phys_base; struct completion complete; }; @@ -851,15 +848,13 @@ static int s3c_onenand_probe(struct platform_device *pdev) /* No need to check pdata. the platform data is optional */ size = sizeof(struct mtd_info) + sizeof(struct onenand_chip); - mtd = kzalloc(size, GFP_KERNEL); + mtd = devm_kzalloc(&pdev->dev, size, GFP_KERNEL); if (!mtd) return -ENOMEM; - onenand = kzalloc(sizeof(struct s3c_onenand), GFP_KERNEL); - if (!onenand) { - err = -ENOMEM; - goto onenand_fail; - } + onenand = devm_kzalloc(&pdev->dev, sizeof(struct s3c_onenand), GFP_KERNEL); + if (!onenand) + return -ENOMEM; this = (struct onenand_chip *) &mtd[1]; mtd->priv = this; @@ -870,26 +865,10 @@ static int s3c_onenand_probe(struct platform_device *pdev) s3c_onenand_setup(mtd); r = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!r) { - dev_err(&pdev->dev, "no memory resource defined\n"); - return -ENOENT; - goto ahb_resource_failed; - } + onenand->base = devm_ioremap_resource(&pdev->dev, r); + if (IS_ERR(onenand->base)) + return PTR_ERR(onenand->base); - onenand->base_res = request_mem_region(r->start, resource_size(r), - pdev->name); - if (!onenand->base_res) { - dev_err(&pdev->dev, "failed to request memory resource\n"); - err = -EBUSY; - goto resource_failed; - } - - onenand->base = ioremap(r->start, resource_size(r)); - if (!onenand->base) { - dev_err(&pdev->dev, "failed to map memory resource\n"); - err = -EFAULT; - goto ioremap_failed; - } /* Set onenand_chip also */ this->base = onenand->base; @@ -898,40 +877,20 @@ static int s3c_onenand_probe(struct platform_device *pdev) if (onenand->type != TYPE_S5PC110) { r = platform_get_resource(pdev, IORESOURCE_MEM, 1); - if (!r) { - dev_err(&pdev->dev, "no buffer memory resource defined\n"); - err = -ENOENT; - goto ahb_resource_failed; - } - - onenand->ahb_res = request_mem_region(r->start, resource_size(r), - pdev->name); - if (!onenand->ahb_res) { - dev_err(&pdev->dev, "failed to request buffer memory resource\n"); - err = -EBUSY; - goto ahb_resource_failed; - } - - onenand->ahb_addr = ioremap(r->start, resource_size(r)); - if (!onenand->ahb_addr) { - dev_err(&pdev->dev, "failed to map buffer memory resource\n"); - err = -EINVAL; - goto ahb_ioremap_failed; - } + onenand->ahb_addr = devm_ioremap_resource(&pdev->dev, r); + if (IS_ERR(onenand->ahb_addr)) + return PTR_ERR(onenand->ahb_addr); /* Allocate 4KiB BufferRAM */ - onenand->page_buf = kzalloc(SZ_4K, GFP_KERNEL); - if (!onenand->page_buf) { - err = -ENOMEM; - goto page_buf_fail; - } + onenand->page_buf = devm_kzalloc(&pdev->dev, SZ_4K, + GFP_KERNEL); + if (!onenand->page_buf) + return -ENOMEM; /* Allocate 128 SpareRAM */ - onenand->oob_buf = kzalloc(128, GFP_KERNEL); - if (!onenand->oob_buf) { - err = -ENOMEM; - goto oob_buf_fail; - } + onenand->oob_buf = devm_kzalloc(&pdev->dev, 128, GFP_KERNEL); + if (!onenand->oob_buf) + return -ENOMEM; /* S3C doesn't handle subpage write */ mtd->subpage_sft = 0; @@ -939,28 +898,11 @@ static int s3c_onenand_probe(struct platform_device *pdev) } else { /* S5PC110 */ r = platform_get_resource(pdev, IORESOURCE_MEM, 1); - if (!r) { - dev_err(&pdev->dev, "no dma memory resource defined\n"); - err = -ENOENT; - goto dma_resource_failed; - } - - onenand->dma_res = request_mem_region(r->start, resource_size(r), - pdev->name); - if (!onenand->dma_res) { - dev_err(&pdev->dev, "failed to request dma memory resource\n"); - err = -EBUSY; - goto dma_resource_failed; - } + onenand->dma_addr = devm_ioremap_resource(&pdev->dev, r); + if (IS_ERR(onenand->dma_addr)) + return PTR_ERR(onenand->dma_addr); - onenand->dma_addr = ioremap(r->start, resource_size(r)); - if (!onenand->dma_addr) { - dev_err(&pdev->dev, "failed to map dma memory resource\n"); - err = -EINVAL; - goto dma_ioremap_failed; - } - - onenand->phys_base = onenand->base_res->start; + onenand->phys_base = r->start; s5pc110_dma_ops = s5pc110_dma_poll; /* Interrupt support */ @@ -968,19 +910,19 @@ static int s3c_onenand_probe(struct platform_device *pdev) if (r) { init_completion(&onenand->complete); s5pc110_dma_ops = s5pc110_dma_irq; - err = request_irq(r->start, s5pc110_onenand_irq, - IRQF_SHARED, "onenand", &onenand); + err = devm_request_irq(&pdev->dev, r->start, + s5pc110_onenand_irq, + IRQF_SHARED, "onenand", + &onenand); if (err) { dev_err(&pdev->dev, "failed to get irq\n"); - goto scan_failed; + return err; } } } - if (onenand_scan(mtd, 1)) { - err = -EFAULT; - goto scan_failed; - } + if (onenand_scan(mtd, 1)) + return -EFAULT; if (onenand->type != TYPE_S5PC110) { /* S3C doesn't handle subpage write */ @@ -998,36 +940,6 @@ static int s3c_onenand_probe(struct platform_device *pdev) platform_set_drvdata(pdev, mtd); return 0; - -scan_failed: - if (onenand->dma_addr) - iounmap(onenand->dma_addr); -dma_ioremap_failed: - if (onenand->dma_res) - release_mem_region(onenand->dma_res->start, - resource_size(onenand->dma_res)); - kfree(onenand->oob_buf); -oob_buf_fail: - kfree(onenand->page_buf); -page_buf_fail: - if (onenand->ahb_addr) - iounmap(onenand->ahb_addr); -ahb_ioremap_failed: - if (onenand->ahb_res) - release_mem_region(onenand->ahb_res->start, - resource_size(onenand->ahb_res)); -dma_resource_failed: -ahb_resource_failed: - iounmap(onenand->base); -ioremap_failed: - if (onenand->base_res) - release_mem_region(onenand->base_res->start, - resource_size(onenand->base_res)); -resource_failed: - kfree(onenand); -onenand_fail: - kfree(mtd); - return err; } static int s3c_onenand_remove(struct platform_device *pdev) @@ -1035,25 +947,7 @@ static int s3c_onenand_remove(struct platform_device *pdev) struct mtd_info *mtd = platform_get_drvdata(pdev); onenand_release(mtd); - if (onenand->ahb_addr) - iounmap(onenand->ahb_addr); - if (onenand->ahb_res) - release_mem_region(onenand->ahb_res->start, - resource_size(onenand->ahb_res)); - if (onenand->dma_addr) - iounmap(onenand->dma_addr); - if (onenand->dma_res) - release_mem_region(onenand->dma_res->start, - resource_size(onenand->dma_res)); - - iounmap(onenand->base); - release_mem_region(onenand->base_res->start, - resource_size(onenand->base_res)); - - kfree(onenand->oob_buf); - kfree(onenand->page_buf); - kfree(onenand); - kfree(mtd); + return 0; } -- 2.14.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2 v2] mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks 2017-12-09 7:24 ` [PATCH 1/2 v2] mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks Christophe JAILLET @ 2017-12-09 8:33 ` Boris Brezillon 0 siblings, 0 replies; 5+ messages in thread From: Boris Brezillon @ 2017-12-09 8:33 UTC (permalink / raw) To: Christophe JAILLET Cc: kyungmin.park, dwmw2, computersforpeace, marek.vasut, richard, cyrille.pitchen, linux-mtd, linux-kernel, kernel-janitors On Sat, 9 Dec 2017 08:24:04 +0100 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > Convert all error handling code in 's3c_onenand_probe()' to > resource-managed alternatives in order to simplify code. > > This fixes a resource leak if 'platform_get_resource()' fails at line 872. > > The 'request_irq()' at line 971 was also un-balanced. It is now > resource-managed. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > Compile tested-only > > v2: use devm_ioremap_resource() > --- > drivers/mtd/onenand/samsung.c | 164 ++++++++---------------------------------- > 1 file changed, 29 insertions(+), 135 deletions(-) > > diff --git a/drivers/mtd/onenand/samsung.c b/drivers/mtd/onenand/samsung.c > index af0ac1a7bf8f..2e562c224060 100644 > --- a/drivers/mtd/onenand/samsung.c > +++ b/drivers/mtd/onenand/samsung.c > @@ -129,16 +129,13 @@ struct s3c_onenand { > struct platform_device *pdev; > enum soc_type type; > void __iomem *base; > - struct resource *base_res; > void __iomem *ahb_addr; > - struct resource *ahb_res; > int bootram_command; > void __iomem *page_buf; > void __iomem *oob_buf; > unsigned int (*mem_addr)(int fba, int fpa, int fsa); > unsigned int (*cmd_map)(unsigned int type, unsigned int val); > void __iomem *dma_addr; > - struct resource *dma_res; > unsigned long phys_base; > struct completion complete; > }; > @@ -851,15 +848,13 @@ static int s3c_onenand_probe(struct platform_device *pdev) > /* No need to check pdata. the platform data is optional */ > > size = sizeof(struct mtd_info) + sizeof(struct onenand_chip); > - mtd = kzalloc(size, GFP_KERNEL); > + mtd = devm_kzalloc(&pdev->dev, size, GFP_KERNEL); > if (!mtd) > return -ENOMEM; > > - onenand = kzalloc(sizeof(struct s3c_onenand), GFP_KERNEL); > - if (!onenand) { > - err = -ENOMEM; > - goto onenand_fail; > - } > + onenand = devm_kzalloc(&pdev->dev, sizeof(struct s3c_onenand), GFP_KERNEL); > + if (!onenand) > + return -ENOMEM; > > this = (struct onenand_chip *) &mtd[1]; > mtd->priv = this; > @@ -870,26 +865,10 @@ static int s3c_onenand_probe(struct platform_device *pdev) > s3c_onenand_setup(mtd); > > r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (!r) { > - dev_err(&pdev->dev, "no memory resource defined\n"); > - return -ENOENT; > - goto ahb_resource_failed; > - } > + onenand->base = devm_ioremap_resource(&pdev->dev, r); > + if (IS_ERR(onenand->base)) > + return PTR_ERR(onenand->base); > > - onenand->base_res = request_mem_region(r->start, resource_size(r), > - pdev->name); > - if (!onenand->base_res) { > - dev_err(&pdev->dev, "failed to request memory resource\n"); > - err = -EBUSY; > - goto resource_failed; > - } > - > - onenand->base = ioremap(r->start, resource_size(r)); > - if (!onenand->base) { > - dev_err(&pdev->dev, "failed to map memory resource\n"); > - err = -EFAULT; > - goto ioremap_failed; > - } > /* Set onenand_chip also */ > this->base = onenand->base; > > @@ -898,40 +877,20 @@ static int s3c_onenand_probe(struct platform_device *pdev) > > if (onenand->type != TYPE_S5PC110) { > r = platform_get_resource(pdev, IORESOURCE_MEM, 1); > - if (!r) { > - dev_err(&pdev->dev, "no buffer memory resource defined\n"); > - err = -ENOENT; > - goto ahb_resource_failed; > - } > - > - onenand->ahb_res = request_mem_region(r->start, resource_size(r), > - pdev->name); > - if (!onenand->ahb_res) { > - dev_err(&pdev->dev, "failed to request buffer memory resource\n"); > - err = -EBUSY; > - goto ahb_resource_failed; > - } > - > - onenand->ahb_addr = ioremap(r->start, resource_size(r)); > - if (!onenand->ahb_addr) { > - dev_err(&pdev->dev, "failed to map buffer memory resource\n"); > - err = -EINVAL; > - goto ahb_ioremap_failed; > - } > + onenand->ahb_addr = devm_ioremap_resource(&pdev->dev, r); > + if (IS_ERR(onenand->ahb_addr)) > + return PTR_ERR(onenand->ahb_addr); > > /* Allocate 4KiB BufferRAM */ > - onenand->page_buf = kzalloc(SZ_4K, GFP_KERNEL); > - if (!onenand->page_buf) { > - err = -ENOMEM; > - goto page_buf_fail; > - } > + onenand->page_buf = devm_kzalloc(&pdev->dev, SZ_4K, > + GFP_KERNEL); > + if (!onenand->page_buf) > + return -ENOMEM; > > /* Allocate 128 SpareRAM */ > - onenand->oob_buf = kzalloc(128, GFP_KERNEL); > - if (!onenand->oob_buf) { > - err = -ENOMEM; > - goto oob_buf_fail; > - } > + onenand->oob_buf = devm_kzalloc(&pdev->dev, 128, GFP_KERNEL); > + if (!onenand->oob_buf) > + return -ENOMEM; > > /* S3C doesn't handle subpage write */ > mtd->subpage_sft = 0; > @@ -939,28 +898,11 @@ static int s3c_onenand_probe(struct platform_device *pdev) > > } else { /* S5PC110 */ > r = platform_get_resource(pdev, IORESOURCE_MEM, 1); > - if (!r) { > - dev_err(&pdev->dev, "no dma memory resource defined\n"); > - err = -ENOENT; > - goto dma_resource_failed; > - } > - > - onenand->dma_res = request_mem_region(r->start, resource_size(r), > - pdev->name); > - if (!onenand->dma_res) { > - dev_err(&pdev->dev, "failed to request dma memory resource\n"); > - err = -EBUSY; > - goto dma_resource_failed; > - } > + onenand->dma_addr = devm_ioremap_resource(&pdev->dev, r); > + if (IS_ERR(onenand->dma_addr)) > + return PTR_ERR(onenand->dma_addr); > > - onenand->dma_addr = ioremap(r->start, resource_size(r)); > - if (!onenand->dma_addr) { > - dev_err(&pdev->dev, "failed to map dma memory resource\n"); > - err = -EINVAL; > - goto dma_ioremap_failed; > - } > - > - onenand->phys_base = onenand->base_res->start; > + onenand->phys_base = r->start; This is wrong, ->phys_base should be assigned to base_res->start not dma_res->start (I pointed it in my previous review). > > s5pc110_dma_ops = s5pc110_dma_poll; > /* Interrupt support */ > @@ -968,19 +910,19 @@ static int s3c_onenand_probe(struct platform_device *pdev) > if (r) { > init_completion(&onenand->complete); > s5pc110_dma_ops = s5pc110_dma_irq; > - err = request_irq(r->start, s5pc110_onenand_irq, > - IRQF_SHARED, "onenand", &onenand); > + err = devm_request_irq(&pdev->dev, r->start, > + s5pc110_onenand_irq, > + IRQF_SHARED, "onenand", > + &onenand); > if (err) { > dev_err(&pdev->dev, "failed to get irq\n"); > - goto scan_failed; > + return err; > } > } > } > > - if (onenand_scan(mtd, 1)) { > - err = -EFAULT; > - goto scan_failed; > - } > + if (onenand_scan(mtd, 1)) > + return -EFAULT; Not related to this patch, but we should return the error code returned by onenand_scan() here instead of always returning -EFAULT. > > if (onenand->type != TYPE_S5PC110) { > /* S3C doesn't handle subpage write */ > @@ -998,36 +940,6 @@ static int s3c_onenand_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, mtd); > > return 0; > - > -scan_failed: > - if (onenand->dma_addr) > - iounmap(onenand->dma_addr); > -dma_ioremap_failed: > - if (onenand->dma_res) > - release_mem_region(onenand->dma_res->start, > - resource_size(onenand->dma_res)); > - kfree(onenand->oob_buf); > -oob_buf_fail: > - kfree(onenand->page_buf); > -page_buf_fail: > - if (onenand->ahb_addr) > - iounmap(onenand->ahb_addr); > -ahb_ioremap_failed: > - if (onenand->ahb_res) > - release_mem_region(onenand->ahb_res->start, > - resource_size(onenand->ahb_res)); > -dma_resource_failed: > -ahb_resource_failed: > - iounmap(onenand->base); > -ioremap_failed: > - if (onenand->base_res) > - release_mem_region(onenand->base_res->start, > - resource_size(onenand->base_res)); > -resource_failed: > - kfree(onenand); > -onenand_fail: > - kfree(mtd); > - return err; > } > > static int s3c_onenand_remove(struct platform_device *pdev) > @@ -1035,25 +947,7 @@ static int s3c_onenand_remove(struct platform_device *pdev) > struct mtd_info *mtd = platform_get_drvdata(pdev); > > onenand_release(mtd); > - if (onenand->ahb_addr) > - iounmap(onenand->ahb_addr); > - if (onenand->ahb_res) > - release_mem_region(onenand->ahb_res->start, > - resource_size(onenand->ahb_res)); > - if (onenand->dma_addr) > - iounmap(onenand->dma_addr); > - if (onenand->dma_res) > - release_mem_region(onenand->dma_res->start, > - resource_size(onenand->dma_res)); > - > - iounmap(onenand->base); > - release_mem_region(onenand->base_res->start, > - resource_size(onenand->base_res)); > - > - kfree(onenand->oob_buf); > - kfree(onenand->page_buf); > - kfree(onenand); > - kfree(mtd); > + > return 0; > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2 v2] mtd: onenand: samsung: return an error if 'mtd_device_parse_register()' fails 2017-12-09 7:24 [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths Christophe JAILLET 2017-12-09 7:24 ` [PATCH 1/2 v2] mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks Christophe JAILLET @ 2017-12-09 7:24 ` Christophe JAILLET 2017-12-09 8:27 ` [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths Boris Brezillon 2 siblings, 0 replies; 5+ messages in thread From: Christophe JAILLET @ 2017-12-09 7:24 UTC (permalink / raw) To: kyungmin.park, dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard, cyrille.pitchen Cc: linux-mtd, linux-kernel, kernel-janitors, Christophe JAILLET If 'mtd_device_parse_register()' fails, we still return 0 which mean success. Return the error code instead, as done in all the other error handling paths. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Compile tested-only v2: call 'onenand_release()' to undo 'onenand_scan()' --- drivers/mtd/onenand/samsung.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/mtd/onenand/samsung.c b/drivers/mtd/onenand/samsung.c index 2e562c224060..4031f750f334 100644 --- a/drivers/mtd/onenand/samsung.c +++ b/drivers/mtd/onenand/samsung.c @@ -936,6 +936,11 @@ static int s3c_onenand_probe(struct platform_device *pdev) err = mtd_device_parse_register(mtd, NULL, NULL, pdata ? pdata->parts : NULL, pdata ? pdata->nr_parts : 0); + if (err) { + dev_err(&pdev->dev, "failed to parse partitions and register the MTD device\n"); + onenand_release(mtd); + return err; + } platform_set_drvdata(pdev, mtd); -- 2.14.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths 2017-12-09 7:24 [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths Christophe JAILLET 2017-12-09 7:24 ` [PATCH 1/2 v2] mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks Christophe JAILLET 2017-12-09 7:24 ` [PATCH 2/2 v2] mtd: onenand: samsung: return an error if 'mtd_device_parse_register()' fails Christophe JAILLET @ 2017-12-09 8:27 ` Boris Brezillon 2 siblings, 0 replies; 5+ messages in thread From: Boris Brezillon @ 2017-12-09 8:27 UTC (permalink / raw) To: Christophe JAILLET Cc: kyungmin.park, dwmw2, computersforpeace, marek.vasut, richard, cyrille.pitchen, linux-mtd, linux-kernel, kernel-janitors On Sat, 9 Dec 2017 08:24:03 +0100 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > The first patch converts 's3c_onenand_probe()' to devm_ functions. > This fixes a leak in one path (line 872). > This also free_irq which was not handled at all. (I hope I'm correct :) ) > > The 2nd patch is about an un-handled error code which looks spurious. > Not sure if I'm right. > > > While compile-testing it, I had to tweak the code because I don't have any > cross-compiler. Oh come on! It's really not that complicated to install an arm toolchain and cross compile the kernel. > I commented the line "#include <asm/mach/flash.h>" and the compilation > succeeded. So maybe, this include is also useless. Yep, it's not surprising, the driver is really in a poor state. I also noticed that buffers allocated with kmalloc are flagged as __iomem regions, which is obviously wrong. And I fear we'll find plenty of other issues if we dig a bit more. > I've left it as-is, though. Can be fixed in another patch. > > > Theses patches have been compile-tested-only. > > > Christophe JAILLET (2): > mtd: onenand: samsung: use devm_ function to simplify code and fix > some leaks > mtd: onenand: samsung: return an error if > 'mtd_device_parse_register()' fails > > drivers/mtd/onenand/samsung.c | 169 +++++++++--------------------------------- > 1 file changed, 34 insertions(+), 135 deletions(-) > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-09 8:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-09 7:24 [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths Christophe JAILLET 2017-12-09 7:24 ` [PATCH 1/2 v2] mtd: onenand: samsung: use devm_ function to simplify code and fix some leaks Christophe JAILLET 2017-12-09 8:33 ` Boris Brezillon 2017-12-09 7:24 ` [PATCH 2/2 v2] mtd: onenand: samsung: return an error if 'mtd_device_parse_register()' fails Christophe JAILLET 2017-12-09 8:27 ` [PATCH 0/2 v2] mtd: onenand: samsung: Simplify code and fix leaks in error handling paths Boris Brezillon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).