All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Adam Ford <aford173@gmail.com>
Cc: Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	linux-mtd@lists.infradead.org, Julien Su <juliensu@mxic.com.tw>,
	ycllin@mxic.com.tw,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Linux-OMAP <linux-omap@vger.kernel.org>
Subject: Re: [PATCH 04/20] mtd: nand: ecc-bch: Stop exporting the private structure
Date: Mon, 11 Jan 2021 11:20:27 +0100	[thread overview]
Message-ID: <20210111112027.7cbda0ba@xps13> (raw)
In-Reply-To: <CAHCN7xLHAd8B_OgR2_vPFZZJ95mwNGR29dmqFeKhhSU9_y_imA@mail.gmail.com>

Hi Adam,

Adam Ford <aford173@gmail.com> wrote on Sat, 9 Jan 2021 08:46:44 -0600:

> On Tue, Sep 29, 2020 at 6:09 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > The NAND BCH control structure has nothing to do outside of this
> > driver, all users of the nand_bch_init/free() functions just save it
> > to chip->ecc.priv so do it in this driver directly and return a
> > regular error code instead.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---  
> 
> Starting with this commit:  3c0fe36abebe, the kernel either doesn't
> build or returns errors on some omap2plus devices with the following
> error:
> 
>     nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xbc
>     nand: Micron MT29F4G16ABBDA3W
>     nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>     nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
>     Invalid ECC layout
>     omap2-nand 30000000.nand: unable to use BCH library
>     omap2-nand: probe of 30000000.nand failed with error -22
>     8<--- cut here ---
> 
> There are few commits using git bisect that have build errors, so it
> wasn't possible for me to determine the exact commit that broke the
> ECC.  If the build failed, I marked it as 'bad' to git bisect.

I am sorry to hear that, I regularly rebase with a make run between each
pick and push my branches to a 0-day repository to have robots check
for such errors, but sometimes I fail.

> Newer commits have remedied the build issue, but the Invalid ECC
> layout error still exists as of 5.11-RC2.

Ok so let's focus on these.

> Do you have any suggestions on what I can do to remedy this?  I am
> willing to try and test.

Glad to hear that.

Can you share the NAND controller DT node you are using?

Also, can you please add a few printk's like below and give me the
output?

---8<---

diff --git a/drivers/mtd/nand/ecc-sw-bch.c b/drivers/mtd/nand/ecc-sw-bch.c
index 0a0ac11d5725..0d3e948d02e9 100644
--- a/drivers/mtd/nand/ecc-sw-bch.c
+++ b/drivers/mtd/nand/ecc-sw-bch.c
@@ -205,6 +205,7 @@ int nand_ecc_sw_bch_init_ctx(struct nand_device *nand)
        }
 
        nsteps = mtd->writesize / conf->step_size;
+       printk("writesize %d, step_size %d, nsteps %d\n", mtd->writesize, conf->step_size, nsteps);
 
        /* Maximize */
        if (nand->ecc.user_conf.flags & NAND_ECC_MAXIMIZE_STRENGTH) {
@@ -213,11 +214,14 @@ int nand_ecc_sw_bch_init_ctx(struct nand_device *nand)
                /* Reserve 2 bytes for the BBM */
                code_size = (mtd->oobsize - 2) / nsteps;
                conf->strength = code_size * 8 / fls(8 * conf->step_size);
+               printk("Maximize => nsteps %d, code_size %d\n", nsteps, code_size);
        }
 
-       if (!code_size)
+       if (!code_size) {
                code_size = DIV_ROUND_UP(conf->strength *
                                         fls(8 * conf->step_size), 8);
+               printk("strength %d, step size %d, code_size %d\n", conf->strength, conf->step_size, code_size);
+       }
 
        if (!conf->strength)
                conf->strength = (code_size * 8) / fls(8 * conf->step_size);
@@ -252,6 +256,7 @@ int nand_ecc_sw_bch_init_ctx(struct nand_device *nand)
                goto free_bufs;
 
        /* Verify the layout validity */
+       printk("count eccbytes %d\n", mtd_ooblayout_count_eccbytes(mtd));
        if (mtd_ooblayout_count_eccbytes(mtd) !=
            engine_conf->nsteps * engine_conf->code_size) {
                pr_err("Invalid ECC layout\n");

--->8---

Thanks,
Miquèl

WARNING: multiple messages have this Message-ID
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Adam Ford <aford173@gmail.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	Julien Su <juliensu@mxic.com.tw>,
	Richard Weinberger <richard@nod.at>,
	ycllin@mxic.com.tw, linux-mtd@lists.infradead.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Linux-OMAP <linux-omap@vger.kernel.org>
Subject: Re: [PATCH 04/20] mtd: nand: ecc-bch: Stop exporting the private structure
Date: Mon, 11 Jan 2021 11:20:27 +0100	[thread overview]
Message-ID: <20210111112027.7cbda0ba@xps13> (raw)
In-Reply-To: <CAHCN7xLHAd8B_OgR2_vPFZZJ95mwNGR29dmqFeKhhSU9_y_imA@mail.gmail.com>

Hi Adam,

Adam Ford <aford173@gmail.com> wrote on Sat, 9 Jan 2021 08:46:44 -0600:

> On Tue, Sep 29, 2020 at 6:09 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > The NAND BCH control structure has nothing to do outside of this
> > driver, all users of the nand_bch_init/free() functions just save it
> > to chip->ecc.priv so do it in this driver directly and return a
> > regular error code instead.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---  
> 
> Starting with this commit:  3c0fe36abebe, the kernel either doesn't
> build or returns errors on some omap2plus devices with the following
> error:
> 
>     nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xbc
>     nand: Micron MT29F4G16ABBDA3W
>     nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>     nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
>     Invalid ECC layout
>     omap2-nand 30000000.nand: unable to use BCH library
>     omap2-nand: probe of 30000000.nand failed with error -22
>     8<--- cut here ---
> 
> There are few commits using git bisect that have build errors, so it
> wasn't possible for me to determine the exact commit that broke the
> ECC.  If the build failed, I marked it as 'bad' to git bisect.

I am sorry to hear that, I regularly rebase with a make run between each
pick and push my branches to a 0-day repository to have robots check
for such errors, but sometimes I fail.

> Newer commits have remedied the build issue, but the Invalid ECC
> layout error still exists as of 5.11-RC2.

Ok so let's focus on these.

> Do you have any suggestions on what I can do to remedy this?  I am
> willing to try and test.

Glad to hear that.

Can you share the NAND controller DT node you are using?

Also, can you please add a few printk's like below and give me the
output?

---8<---

diff --git a/drivers/mtd/nand/ecc-sw-bch.c b/drivers/mtd/nand/ecc-sw-bch.c
index 0a0ac11d5725..0d3e948d02e9 100644
--- a/drivers/mtd/nand/ecc-sw-bch.c
+++ b/drivers/mtd/nand/ecc-sw-bch.c
@@ -205,6 +205,7 @@ int nand_ecc_sw_bch_init_ctx(struct nand_device *nand)
        }
 
        nsteps = mtd->writesize / conf->step_size;
+       printk("writesize %d, step_size %d, nsteps %d\n", mtd->writesize, conf->step_size, nsteps);
 
        /* Maximize */
        if (nand->ecc.user_conf.flags & NAND_ECC_MAXIMIZE_STRENGTH) {
@@ -213,11 +214,14 @@ int nand_ecc_sw_bch_init_ctx(struct nand_device *nand)
                /* Reserve 2 bytes for the BBM */
                code_size = (mtd->oobsize - 2) / nsteps;
                conf->strength = code_size * 8 / fls(8 * conf->step_size);
+               printk("Maximize => nsteps %d, code_size %d\n", nsteps, code_size);
        }
 
-       if (!code_size)
+       if (!code_size) {
                code_size = DIV_ROUND_UP(conf->strength *
                                         fls(8 * conf->step_size), 8);
+               printk("strength %d, step size %d, code_size %d\n", conf->strength, conf->step_size, code_size);
+       }
 
        if (!conf->strength)
                conf->strength = (code_size * 8) / fls(8 * conf->step_size);
@@ -252,6 +256,7 @@ int nand_ecc_sw_bch_init_ctx(struct nand_device *nand)
                goto free_bufs;
 
        /* Verify the layout validity */
+       printk("count eccbytes %d\n", mtd_ooblayout_count_eccbytes(mtd));
        if (mtd_ooblayout_count_eccbytes(mtd) !=
            engine_conf->nsteps * engine_conf->code_size) {
                pr_err("Invalid ECC layout\n");

--->8---

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2021-01-11 10:48 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-29 23:01 [PATCH 00/20] Create generic software ECC engines Miquel Raynal
2020-09-29 23:01 ` [PATCH 01/20] mtd: nand: ecc: Add an I/O request tweaking mechanism Miquel Raynal
2020-09-30  7:53   ` Thomas Petazzoni
2020-09-30  8:16     ` Miquel Raynal
2020-10-30 17:30   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 02/20] mtd: nand: ecc-bch: Move BCH code to the generic NAND layer Miquel Raynal
2020-10-30 17:30   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 03/20] mtd: nand: ecc-bch: Cleanup and style fixes Miquel Raynal
2020-10-30 17:30   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 04/20] mtd: nand: ecc-bch: Stop exporting the private structure Miquel Raynal
2021-01-09 14:46   ` Adam Ford
2021-01-09 14:46     ` Adam Ford
2021-01-11 10:20     ` Miquel Raynal [this message]
2021-01-11 10:20       ` Miquel Raynal
2021-01-12 14:35       ` Miquel Raynal
2021-01-12 14:35         ` Miquel Raynal
2021-01-12 16:01         ` Adam Ford
2021-01-12 16:01           ` Adam Ford
2021-01-12 17:20           ` Adam Ford
2021-01-12 17:20             ` Adam Ford
2021-01-14 15:42             ` Miquel Raynal
2021-01-14 15:42               ` Miquel Raynal
2021-01-15 12:23               ` Adam Ford
2021-01-15 12:23                 ` Adam Ford
2021-01-15 16:06                 ` Adam Ford
2021-01-15 16:06                   ` Adam Ford
2021-01-15 16:17                   ` Miquel Raynal
2021-01-15 16:17                     ` Miquel Raynal
2021-01-15 16:28                     ` Adam Ford
2021-01-15 16:28                       ` Adam Ford
2021-01-19 11:56                       ` Miquel Raynal
2021-01-19 11:56                         ` Miquel Raynal
2021-01-19 14:21                         ` Adam Ford
2021-01-19 14:21                           ` Adam Ford
2021-01-19 14:36                           ` Miquel Raynal
2021-01-19 14:36                             ` Miquel Raynal
2021-01-19 15:49                             ` Adam Ford
2021-01-19 15:49                               ` Adam Ford
2021-01-19 15:53                               ` Miquel Raynal
2021-01-19 15:53                                 ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 05/20] mtd: nand: ecc-bch: Return only valid error codes Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 06/20] mtd: nand: ecc-bch: Drop mtd_nand_has_bch() Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 07/20] mtd: nand: ecc-bch: Update the prototypes to be more generic Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 08/20] mtd: nand: ecc-bch: Stop using raw NAND structures Miquel Raynal
2020-09-29 23:01 ` [PATCH 09/20] mtd: nand: ecc-bch: Create the software BCH engine Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 10/20] mtd: rawnand: Get rid of chip->ecc.priv Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 11/20] mtd: nand: ecc-hamming: Move Hamming code to the generic NAND layer Miquel Raynal
2020-09-29 23:01 ` [PATCH 12/20] mtd: nand: ecc-hamming: Clarify the driver descriptions Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 13/20] mtd: nand: ecc-hamming: Drop/fix the kernel doc Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 14/20] mtd: nand: ecc-hamming: Cleanup and style fixes Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 15/20] mtd: nand: ecc-hamming: Rename the exported functions Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 16/20] mtd: nand: ecc-hamming: Stop using raw NAND structures Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 17/20] mtd: nand: ecc-hamming: Remove useless includes Miquel Raynal
2020-09-29 23:01 ` [PATCH 18/20] mtd: nand: ecc-hamming: Let the software Hamming ECC engine be unselected Miquel Raynal
2020-10-30 17:29   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 19/20] mtd: nand: ecc-hamming: Create the software Hamming engine Miquel Raynal
2020-10-30 17:28   ` Miquel Raynal
2020-09-29 23:01 ` [PATCH 20/20] mtd: nand: Let software ECC engines be retrieved from the NAND core Miquel Raynal
2020-10-30 17:28   ` Miquel Raynal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210111112027.7cbda0ba@xps13 \
    --to=miquel.raynal@bootlin.com \
    --cc=Tudor.Ambarus@microchip.com \
    --cc=aford173@gmail.com \
    --cc=juliensu@mxic.com.tw \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=richard@nod.at \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vigneshr@ti.com \
    --cc=ycllin@mxic.com.tw \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.