RE: [PATCH v2 4/5] mtd: nand: omap2: Use devm_kzalloc
Hi, This simplifies the error path and makes the code less error-prone. Signed-off-by: Ezequiel Garcia ezequiel.gar...@free-electrons.com --- drivers/mtd/nand/omap2.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index e01a936..d3155b2 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1837,7 +1837,7 @@ static int omap_nand_probe(struct platform_device *pdev) return -ENODEV; } - info = kzalloc(sizeof(struct omap_nand_info), GFP_KERNEL); + info = devm_kzalloc(pdev-dev, sizeof(*info), GFP_KERNEL); if (!info) return -ENOMEM; @@ -2067,8 +2067,6 @@ out_release_mem_region: free_irq(info-gpmc_irq_fifo, info); release_mem_region(info-phys_base, info-mem_size); out_free_info: - kfree(info); - return err; } @@ -2091,7 +2089,6 @@ static int omap_nand_remove(struct platform_device *pdev) nand_release(info-mtd); iounmap(info-nand.IO_ADDR_R); release_mem_region(info-phys_base, info-mem_size); - kfree(info); return 0; } -- 1.8.1.5 I think these changes are already done as part of following patch.. http://lists.infradead.org/pipermail/linux-mtd/2013-October/049418.html Did your rebase on my patch-set ? with regards, pekon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] mtd: nand: omap2: Use devm_kzalloc
On Fri, Oct 25, 2013 at 10:25:02AM +, Gupta, Pekon wrote: Hi, This simplifies the error path and makes the code less error-prone. Signed-off-by: Ezequiel Garcia ezequiel.gar...@free-electrons.com --- drivers/mtd/nand/omap2.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index e01a936..d3155b2 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1837,7 +1837,7 @@ static int omap_nand_probe(struct platform_device *pdev) return -ENODEV; } - info = kzalloc(sizeof(struct omap_nand_info), GFP_KERNEL); + info = devm_kzalloc(pdev-dev, sizeof(*info), GFP_KERNEL); if (!info) return -ENOMEM; @@ -2067,8 +2067,6 @@ out_release_mem_region: free_irq(info-gpmc_irq_fifo, info); release_mem_region(info-phys_base, info-mem_size); out_free_info: - kfree(info); - return err; } @@ -2091,7 +2089,6 @@ static int omap_nand_remove(struct platform_device *pdev) nand_release(info-mtd); iounmap(info-nand.IO_ADDR_R); release_mem_region(info-phys_base, info-mem_size); - kfree(info); return 0; } -- 1.8.1.5 I think these changes are already done as part of following patch.. http://lists.infradead.org/pipermail/linux-mtd/2013-October/049418.html Did your rebase on my patch-set ? Hm.. well the problem with that patch is that it's in the middle of an unrelated series. As I already told you, I think you should have pushed that as a one-patch fix. Have you seen that suggestion? On the other side, you're fixing too many things in that single patch, for my taste. Maybe I'm not the smarter developer, but going through that patch is not easy to catch if there's no mistake done. Usually if it's possible to split a patch (maintaining consistency) it makes the reviewing process easier. If you'd rather send this devm_xxx change yourself that's fine by me, but *please* split the patch in two and write proper commit messages. Anyway: this is just a silly change, the important one is the other nand_scan_ident() fix. Could you help me review that? I'm interested in knowing how will that work with 8-bit and 16-bit devices. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 4/5] mtd: nand: omap2: Use devm_kzalloc
From: Ezequiel Garcia [mailto:ezequiel.gar...@free-electrons.com] Hm.. well the problem with that patch is that it's in the middle of an unrelated series. As I already told you, I think you should have pushed that as a one-patch fix. Have you seen that suggestion? Yes, I know.. actually the original patch series, when it started somewhere April (or before) is very different from the version v11 now :-). This devm_ update was added in middle of v6-v7 version change (Most of the changes since first version of this patchset is captured in Cover-letter). On the other side, you're fixing too many things in that single patch, for my taste. Maybe I'm not the smarter developer, but going through that patch is not easy to catch if there's no mistake done. Usually if it's possible to split a patch (maintaining consistency) it makes the reviewing process easier. If you'd rather send this devm_xxx change yourself that's fine by me, Ahh nothing like that.. Brian had already reviewed these couple of times And it was only [Patch 04/10] which was last one remaining.. I just said it because this might show up in merge conflict .. or rejects.. but *please* split the patch in two and write proper commit messages. Anyway: this is just a silly change, the important one is the other nand_scan_ident() fix. Could you help me review that? I'm interested in knowing how will that work with 8-bit and 16-bit devices. -- Yes, I'm just preparing the scenario where BUSWIDTH_AUTO would fail.. unless you do GPMC driver changes also.. same issue was found by Matthieu CASTET (matthieu.cas...@parrot.com) (please see my other mail) with regards, pekon
Re: [PATCH v2 4/5] mtd: nand: omap2: Use devm_kzalloc
On Fri, Oct 25, 2013 at 11:09:14AM +, Gupta, Pekon wrote: From: Ezequiel Garcia [mailto:ezequiel.gar...@free-electrons.com] Hm.. well the problem with that patch is that it's in the middle of an unrelated series. As I already told you, I think you should have pushed that as a one-patch fix. Have you seen that suggestion? Yes, I know.. actually the original patch series, when it started somewhere April (or before) is very different from the version v11 now :-). This devm_ update was added in middle of v6-v7 version change (Most of the changes since first version of this patchset is captured in Cover-letter). Well, in order to *avoid* having a patchset flowing for 5 months and 11 revisions you coudl try to keep series small. You could have that single fix merged if you send it alone. Not sure why you insist in *not* doing that. On the other side, you're fixing too many things in that single patch, for my taste. Maybe I'm not the smarter developer, but going through that patch is not easy to catch if there's no mistake done. Usually if it's possible to split a patch (maintaining consistency) it makes the reviewing process easier. If you'd rather send this devm_xxx change yourself that's fine by me, Ahh nothing like that.. Brian had already reviewed these couple of times Ah, good. In that case you should add Reviewed-by if Brian already reviewed it. IMHO, the patch could be cleaner and the commit message could be better. And it was only [Patch 04/10] which was last one remaining.. Yes, and because you added *another* patch to the series you keep spinning patchset versions. I just said it because this might show up in merge conflict .. or rejects.. but *please* split the patch in two and write proper commit messages. Anyway: this is just a silly change, the important one is the other nand_scan_ident() fix. Could you help me review that? I'm interested in knowing how will that work with 8-bit and 16-bit devices. -- Yes, I'm just preparing the scenario where BUSWIDTH_AUTO would fail.. unless you do GPMC driver changes also.. same issue was found by Matthieu CASTET (matthieu.cas...@parrot.com) (please see my other mail) OK, let's try to focus in that patch alone, I'd like to move forward. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
