Re: [PATCH v2 4/5] mtd: nand: omap2: Use devm_kzalloc

[Ezequiel Garcia <ezequiel.garcia@free-electrons.com>]

On Fri, Oct 25, 2013 at 10:25:02AM +0000, Gupta, Pekon wrote:
> Hi,
> 
> > 
> > This simplifies the error path and makes the code less error-prone.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html