[PATCH 1/2] mtd: nand: denali: Avoid using ecc->code_buf as a temporary buffer

[PATCH 1/2] mtd: nand: denali: Avoid using ecc->code_buf as a temporary buffer

[Boris Brezillon]

ECC bytes are contiguous in the ->oob_poi buffer, which means we don't
have to copy them into the ->code_buf (here used as a temporary buffer)
before passing them to the nand_check_erased_ecc_chunk() function.

This change will allow us to allocate ecc->{code,calc}_buf only when
ecc->calculate() or ecc->correct() is specified.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/denali.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 00698b33cb22..313c7f50621b 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -330,16 +330,12 @@ static int denali_check_erased_page(struct mtd_info *mtd,
 				    unsigned long uncor_ecc_flags,
 				    unsigned int max_bitflips)
 {
-	uint8_t *ecc_code = chip->ecc.code_buf;
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	uint8_t *ecc_code = chip->oob_poi + denali->oob_skip_bytes;
 	int ecc_steps = chip->ecc.steps;
 	int ecc_size = chip->ecc.size;
 	int ecc_bytes = chip->ecc.bytes;
-	int i, ret, stat;
-
-	ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
-					 chip->ecc.total);
-	if (ret)
-		return ret;
+	int i, stat;
 
 	for (i = 0; i < ecc_steps; i++) {
 		if (!(uncor_ecc_flags & BIT(i)))
-- 
2.11.0

[PATCH 2/2] mtd: nand: Only allocate ecc->{calc, code}_buf when actually needed

[Boris Brezillon]

The only users of the ecc->{calc,code}_buf buffers are NAND controller
drivers implementing ecc->calculate() and/or ecc->correct(). Since the
->oobsize can be non-negligle, especially on modern NAND devices, we'd
better allocate it only when it is actually required.

Make ecc->{calc,code}_buf allocation dependent on the presence of
ecc->calculate() or ecc->correct().

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 71e70c4964f3..e6873d10c574 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -5318,21 +5318,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 		return -EINVAL;
 	}
 
-	ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
-	if (!ecc->calc_buf)
-		return -ENOMEM;
-
-	ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
-	if (!ecc->code_buf) {
-		ret = -ENOMEM;
-		goto err_free_buf;
-	}
-
 	chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
-	if (!chip->data_buf) {
-		ret = -ENOMEM;
-		goto err_free_buf;
-	}
+	if (!chip->data_buf)
+		return -ENOMEM;
 
 	/*
 	 * FIXME: some NAND manufacturer drivers expect the first die to be
@@ -5495,6 +5483,15 @@ int nand_scan_tail(struct mtd_info *mtd)
 		goto err_nand_manuf_cleanup;
 	}
 
+	if (ecc->correct || ecc->calculate) {
+		ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
+		ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
+		if (!ecc->calc_buf || !ecc->code_buf) {
+			ret = -ENOMEM;
+			goto err_nand_manuf_cleanup;
+		}
+	}
+
 	/* For many systems, the standard OOB write also works for raw */
 	if (!ecc->read_oob_raw)
 		ecc->read_oob_raw = ecc->read_oob;
-- 
2.11.0

Re: [PATCH 1/2] mtd: nand: denali: Avoid using ecc->code_buf as a temporary buffer

[Masahiro Yamada]

2017-12-05 20:09 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> ECC bytes are contiguous in the ->oob_poi buffer, which means we don't
> have to copy them into the ->code_buf (here used as a temporary buffer)
> before passing them to the nand_check_erased_ecc_chunk() function.
>
> This change will allow us to allocate ecc->{code,calc}_buf only when
> ecc->calculate() or ecc->correct() is specified.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Thanks!




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] mtd: nand: Only allocate ecc->{calc, code}_buf when actually needed

[Masahiro Yamada]

2017-12-05 20:09 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> The only users of the ecc->{calc,code}_buf buffers are NAND controller
> drivers implementing ecc->calculate() and/or ecc->correct(). Since the
> ->oobsize can be non-negligle, especially on modern NAND devices, we'd
> better allocate it only when it is actually required.
>
> Make ecc->{calc,code}_buf allocation dependent on the presence of
> ecc->calculate() or ecc->correct().
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/nand/nand_base.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 71e70c4964f3..e6873d10c574 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -5318,21 +5318,9 @@ int nand_scan_tail(struct mtd_info *mtd)
>                 return -EINVAL;
>         }
>
> -       ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> -       if (!ecc->calc_buf)
> -               return -ENOMEM;
> -
> -       ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> -       if (!ecc->code_buf) {
> -               ret = -ENOMEM;
> -               goto err_free_buf;
> -       }
> -
>         chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
> -       if (!chip->data_buf) {
> -               ret = -ENOMEM;
> -               goto err_free_buf;
> -       }
> +       if (!chip->data_buf)
> +               return -ENOMEM;
>
>         /*
>          * FIXME: some NAND manufacturer drivers expect the first die to be
> @@ -5495,6 +5483,15 @@ int nand_scan_tail(struct mtd_info *mtd)
>                 goto err_nand_manuf_cleanup;
>         }
>
> +       if (ecc->correct || ecc->calculate) {
> +               ecc->calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> +               ecc->code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> +               if (!ecc->calc_buf || !ecc->code_buf) {
> +                       ret = -ENOMEM;
> +                       goto err_nand_manuf_cleanup;
> +               }
> +       }
> +
>         /* For many systems, the standard OOB write also works for raw */
>         if (!ecc->read_oob_raw)
>                 ecc->read_oob_raw = ecc->read_oob;
> --
> 2.11.0


I wondered if chip->data_buf allocation could be moved as well,
but otherwise, looks good to me.


Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/2] mtd: nand: denali: Avoid using ecc->code_buf as a temporary buffer

[Boris Brezillon]

On Tue, 5 Dec 2017 21:37:45 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> 2017-12-05 20:09 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> > ECC bytes are contiguous in the ->oob_poi buffer, which means we don't
> > have to copy them into the ->code_buf (here used as a temporary buffer)
> > before passing them to the nand_check_erased_ecc_chunk() function.
> >
> > This change will allow us to allocate ecc->{code,calc}_buf only when
> > ecc->calculate() or ecc->correct() is specified.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
> 
> Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Applied those 2 patches.

Boris

> 
> Thanks!
> 
> 
> 
>