diff mbox series

[5/7] mtd: nand: Fix ecc in mxs_nand_spl onfi mode

Message ID 20220727093748.1415135-6-michael@amarulasolutions.com
State New
Headers
Series NAND new improvements |

Commit Message

Michael Trimarchi July 27, 2022, 9:37 a.m. UTC 
We need to calculate the ecc parameters in a way that are the same
in uboot and spl. The parameters are connected to the onfi
computation. We need to assign all the value of chip in order
to have same ecc strength parameters before calling

mxs_nand_set_geometry that calculate the ecc layout

/* use the legacy bch setting by default */
if ((!nand_info->use_minimum_ecc && mtd->oobsize < 1024) ||
     !(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0)) {
	dev_dbg(mtd->dev, "use legacy bch geometry\n");
	err = mxs_nand_legacy_calc_ecc_layout(geo, mtd);
	if (!err)
		return 0;
}

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/mtd/nand/raw/mxs_nand_spl.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Dario Binacchi July 27, 2022, 12:37 p.m. UTC | #1 
Hi Michael,

On Wed, Jul 27, 2022 at 11:37 AM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> We need to calculate the ecc parameters in a way that are the same
> in uboot and spl. The parameters are connected to the onfi
> computation. We need to assign all the value of chip in order
> to have same ecc strength parameters before calling
>
> mxs_nand_set_geometry that calculate the ecc layout
>
> /* use the legacy bch setting by default */
> if ((!nand_info->use_minimum_ecc && mtd->oobsize < 1024) ||
>      !(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0)) {
>         dev_dbg(mtd->dev, "use legacy bch geometry\n");
>         err = mxs_nand_legacy_calc_ecc_layout(geo, mtd);
>         if (!err)
>                 return 0;
> }
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/mxs_nand_spl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> index 4dffa76eaf..6d8ec5b3cb 100644
> --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> @@ -139,6 +139,10 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
>         mtd->writesize = le32_to_cpu(p->byte_per_page);
>         mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
>         mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> +       if (p->ecc_bits != 0xff) {
> +               chip->ecc_strength_ds = p->ecc_bits;
> +               chip->ecc_step_ds = 512;
> +       }
>         chip->chipsize = le32_to_cpu(p->blocks_per_lun);
>         chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
>         /* Calculate the address shift from the page size */
> @@ -152,6 +156,8 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
>         debug("writesize=%d (>>%d)\n", mtd->writesize, chip->page_shift);
>         debug("oobsize=%d\n", mtd->oobsize);
>         debug("chipsize=%lld\n", chip->chipsize);
> +       debug("ecc_strength_ds=%d\n", chip->ecc_strength_ds);
> +       debug("ecc_step_ds = %d\n", chip->ecc_step_ds);
>
>         return 0;
>  }
> --
> 2.34.1
>

What about thinking of a patch removing the last_block variable and using the
size > 0 check also for the external loop? IMHO we could use only the size and
block variables, lastblock can be removed, or am I missing something?

Dario
Dario Binacchi July 27, 2022, 12:39 p.m. UTC | #2 
Hi Michael,

On Wed, Jul 27, 2022 at 2:37 PM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> Hi Michael,
>
> On Wed, Jul 27, 2022 at 11:37 AM Michael Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > We need to calculate the ecc parameters in a way that are the same
> > in uboot and spl. The parameters are connected to the onfi
> > computation. We need to assign all the value of chip in order
> > to have same ecc strength parameters before calling
> >
> > mxs_nand_set_geometry that calculate the ecc layout
> >
> > /* use the legacy bch setting by default */
> > if ((!nand_info->use_minimum_ecc && mtd->oobsize < 1024) ||
> >      !(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0)) {
> >         dev_dbg(mtd->dev, "use legacy bch geometry\n");
> >         err = mxs_nand_legacy_calc_ecc_layout(geo, mtd);
> >         if (!err)
> >                 return 0;
> > }
> >
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > ---
> >  drivers/mtd/nand/raw/mxs_nand_spl.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > index 4dffa76eaf..6d8ec5b3cb 100644
> > --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> > +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > @@ -139,6 +139,10 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
> >         mtd->writesize = le32_to_cpu(p->byte_per_page);
> >         mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
> >         mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> > +       if (p->ecc_bits != 0xff) {
> > +               chip->ecc_strength_ds = p->ecc_bits;
> > +               chip->ecc_step_ds = 512;
> > +       }
> >         chip->chipsize = le32_to_cpu(p->blocks_per_lun);
> >         chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
> >         /* Calculate the address shift from the page size */
> > @@ -152,6 +156,8 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
> >         debug("writesize=%d (>>%d)\n", mtd->writesize, chip->page_shift);
> >         debug("oobsize=%d\n", mtd->oobsize);
> >         debug("chipsize=%lld\n", chip->chipsize);
> > +       debug("ecc_strength_ds=%d\n", chip->ecc_strength_ds);
> > +       debug("ecc_step_ds = %d\n", chip->ecc_step_ds);
> >
> >         return 0;
> >  }
> > --
> > 2.34.1
> >
>
> What about thinking of a patch removing the last_block variable and using the
> size > 0 check also for the external loop? IMHO we could use only the size and
> block variables, lastblock can be removed, or am I missing something?
>

Sorry, this comment was about the next patch.
This patch looks good to me.

Dario

> Dario
>
> --
>
> Dario Binacchi
>
> Embedded Linux Developer
>
> dario.binacchi@amarulasolutions.com
>
> __________________________________
>
>
> Amarula Solutions SRL
>
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
>
> T. +39 042 243 5310
> info@amarulasolutions.com
>
> www.amarulasolutions.com
Tommaso Merciai July 28, 2022, 8:07 a.m. UTC | #3 
On Wed, Jul 27, 2022 at 11:37:46AM +0200, Michael Trimarchi wrote:
> We need to calculate the ecc parameters in a way that are the same
> in uboot and spl. The parameters are connected to the onfi
> computation. We need to assign all the value of chip in order
> to have same ecc strength parameters before calling
> 
> mxs_nand_set_geometry that calculate the ecc layout
> 
> /* use the legacy bch setting by default */
> if ((!nand_info->use_minimum_ecc && mtd->oobsize < 1024) ||
>      !(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0)) {
> 	dev_dbg(mtd->dev, "use legacy bch geometry\n");
> 	err = mxs_nand_legacy_calc_ecc_layout(geo, mtd);
> 	if (!err)
> 		return 0;
> }
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/mxs_nand_spl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> index 4dffa76eaf..6d8ec5b3cb 100644
> --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> @@ -139,6 +139,10 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
>  	mtd->writesize = le32_to_cpu(p->byte_per_page);
>  	mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
>  	mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> +	if (p->ecc_bits != 0xff) {
> +		chip->ecc_strength_ds = p->ecc_bits;
> +		chip->ecc_step_ds = 512;
> +	}
>  	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
>  	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
>  	/* Calculate the address shift from the page size */
> @@ -152,6 +156,8 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
>  	debug("writesize=%d (>>%d)\n", mtd->writesize, chip->page_shift);
>  	debug("oobsize=%d\n", mtd->oobsize);
>  	debug("chipsize=%lld\n", chip->chipsize);
> +	debug("ecc_strength_ds=%d\n", chip->ecc_strength_ds);
> +	debug("ecc_step_ds = %d\n", chip->ecc_step_ds);
>  
>  	return 0;
>  }
> -- 
> 2.34.1
> 

Look's good to me.

Tommaso
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
index 4dffa76eaf..6d8ec5b3cb 100644
--- a/drivers/mtd/nand/raw/mxs_nand_spl.c
+++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
@@ -139,6 +139,10 @@  static int mxs_flash_onfi_ident(struct mtd_info *mtd)
 	mtd->writesize = le32_to_cpu(p->byte_per_page);
 	mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
 	mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
+	if (p->ecc_bits != 0xff) {
+		chip->ecc_strength_ds = p->ecc_bits;
+		chip->ecc_step_ds = 512;
+	}
 	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
 	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
 	/* Calculate the address shift from the page size */
@@ -152,6 +156,8 @@  static int mxs_flash_onfi_ident(struct mtd_info *mtd)
 	debug("writesize=%d (>>%d)\n", mtd->writesize, chip->page_shift);
 	debug("oobsize=%d\n", mtd->oobsize);
 	debug("chipsize=%lld\n", chip->chipsize);
+	debug("ecc_strength_ds=%d\n", chip->ecc_strength_ds);
+	debug("ecc_step_ds = %d\n", chip->ecc_step_ds);
 
 	return 0;
 }