[PATCH V1] mtd : nand : denali :- No need of devm functions

[PATCH V1] mtd : nand : denali :- No need of devm functions

[Arvind Yadav]

In function denali_init, the memory allocated for denali->buf.buf
is live within the function only. After the allocation it is
immediately freed with devm_kfree. There is no need to allocate
memory for denali->buf.buf with devm function so replace
devm_kzalloc with kzalloc and devm_kfree with kfree.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/mtd/nand/denali.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 0476ae8..ce3c31f 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1476,8 +1476,7 @@ int denali_init(struct denali_nand_info *denali)
 	}
 
 	/* allocate a temporary buffer for nand_scan_ident() */
-	denali->buf.buf = devm_kzalloc(denali->dev, PAGE_SIZE,
-					GFP_DMA | GFP_KERNEL);
+	denali->buf.buf = kzalloc(PAGE_SIZE, GFP_DMA | GFP_KERNEL);
 	if (!denali->buf.buf)
 		return -ENOMEM;
 
@@ -1492,6 +1491,7 @@ int denali_init(struct denali_nand_info *denali)
 	if (request_irq(denali->irq, denali_isr, IRQF_SHARED,
 			DENALI_NAND_NAME, denali)) {
 		pr_err("Spectra: Unable to allocate IRQ\n");
+		kfree(denali->buf.buf);
 		return -ENODEV;
 	}
 
@@ -1512,11 +1512,12 @@ int denali_init(struct denali_nand_info *denali)
 	 */
 	if (nand_scan_ident(mtd, denali->max_banks, NULL)) {
 		ret = -ENXIO;
+		kfree(denali->buf.buf);
 		goto failed_req_irq;
 	}
 
 	/* allocate the right size buffer now */
-	devm_kfree(denali->dev, denali->buf.buf);
+	kfree(denali->buf.buf);
 	denali->buf.buf = devm_kzalloc(denali->dev,
 			     mtd->writesize + mtd->oobsize,
 			     GFP_KERNEL);
-- 
2.7.4

Re: [PATCH V1] mtd : nand : denali :- No need of devm functions

[Boris Brezillon]

On Wed,  7 Dec 2016 20:45:11 +0530
Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:

> In function denali_init, the memory allocated for denali->buf.buf
> is live within the function only. After the allocation it is
> immediately freed with devm_kfree. There is no need to allocate
> memory for denali->buf.buf with devm function so replace
> devm_kzalloc with kzalloc and devm_kfree with kfree.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
>  drivers/mtd/nand/denali.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Not sure it's any better than the previous logic (see the diffstat). The
devm_kzalloc() approach has the benefit of simplifying the different
error paths.

> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 0476ae8..ce3c31f 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1476,8 +1476,7 @@ int denali_init(struct denali_nand_info *denali)
>  	}
>  
>  	/* allocate a temporary buffer for nand_scan_ident() */
> -	denali->buf.buf = devm_kzalloc(denali->dev, PAGE_SIZE,
> -					GFP_DMA | GFP_KERNEL);
> +	denali->buf.buf = kzalloc(PAGE_SIZE, GFP_DMA | GFP_KERNEL);
>  	if (!denali->buf.buf)
>  		return -ENOMEM;
>  
> @@ -1492,6 +1491,7 @@ int denali_init(struct denali_nand_info *denali)
>  	if (request_irq(denali->irq, denali_isr, IRQF_SHARED,
>  			DENALI_NAND_NAME, denali)) {
>  		pr_err("Spectra: Unable to allocate IRQ\n");
> +		kfree(denali->buf.buf);
>  		return -ENODEV;
>  	}
>  
> @@ -1512,11 +1512,12 @@ int denali_init(struct denali_nand_info *denali)
>  	 */
>  	if (nand_scan_ident(mtd, denali->max_banks, NULL)) {
>  		ret = -ENXIO;
> +		kfree(denali->buf.buf);
>  		goto failed_req_irq;
>  	}
>  
>  	/* allocate the right size buffer now */
> -	devm_kfree(denali->dev, denali->buf.buf);
> +	kfree(denali->buf.buf);
>  	denali->buf.buf = devm_kzalloc(denali->dev,
>  			     mtd->writesize + mtd->oobsize,
>  			     GFP_KERNEL);

Re: [PATCH V1] mtd : nand : denali :- No need of devm functions

[Arvind Yadav]

yes, if Memory is live out side function. Then devm_kzalloc()
approach has the benefit of simplifying the different error paths.

Here, Memory is alive with in function. we are going to free allocate memory
then why we need devm api. In this case Devm will first add this entry to
list and immediately it will remove from list. In this case, It's just a 
overhead
for devm api.

-Arvind


On Thursday 29 December 2016 10:54 PM, Boris Brezillon wrote:
> The
> devm_kzalloc() approach has the benefit of simplifying the different
> error paths.

Re: [PATCH V1] mtd : nand : denali :- No need of devm functions

[Boris Brezillon]

On Mon, 2 Jan 2017 13:26:01 +0530
Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:

> yes, if Memory is live out side function. Then devm_kzalloc()
> approach has the benefit of simplifying the different error paths.
> 
> Here, Memory is alive with in function. we are going to free allocate memory
> then why we need devm api. In this case Devm will first add this entry to
> list and immediately it will remove from list. In this case, It's just a 
> overhead
> for devm api.

Yes, it adds a small overhead, but ITOH, it simplifies the code (see
the kfree() calls you added in different error paths with your
approach). Sometime a small runtime overhead (especially when the code
is executed once at probe time) is acceptable if it improves
readability.