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

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(-)

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 ?


with regards, pekon

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.

> From: Ezequiel Garcia [mailto:ezequiel.garcia@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.castet@parrot.com)
(please see my other mail)


with regards, pekon

On Fri, Oct 25, 2013 at 11:09:14AM +0000, Gupta, Pekon wrote:
> > From: Ezequiel Garcia [mailto:ezequiel.garcia@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.castet@parrot.com)
> (please see my other mail)
> 

OK, let's try to focus in that patch alone, I'd like to move forward.