Re: [PATCH 01/14] mtd: nand: brcm: switch to mtd_ooblayout_ops

[William Zhang]

On 01/26/2023 12:43 AM, Linus Walleij wrote:

On Thu, Jan 26, 2023 at 2:02 AM William Zhang
<william.zh...@broadcom.com> wrote:

Unfortunately the u-boot nand base code still uses nand_ecclayout
structure because it was based on old kernel nand driver.  Your change
cause bugcheck in the nand_scan_tail at line 4978 when mtd->oobsize is
not one of the default size (i.e. some large nand with BCH-8 ecc
requirement and has 224 bytes oobsize per 4K page) because ecc->layout
is never set. Also certainly any data built based on nand_cclayout like
mtd->oobavail will not be correct.

I actually converted back to nand_ecclayout structure from mtd_ooblayout
with this fix to solve the above issues.
Fixes: e365de90517b ("drivers: nand: brcmnand: fix nand_chip ecc layout
structure")

Argh yeah I see. Let's hold this series off then.
It was worth a try!

I can see your point to get the latest from the brcmnand linux kernel
driver but this requires updating the u-boot nand base driver to use
mtd_ooblayout as well and all others nand controller drivers too. I am
not sure if this is something you want to tackle right now.

No I can't do that I do not have a big enough experience with NAND
flash and no testbed for it either.

As far as I can see, all other oob/ecc layout setting patches you back
ported in this series are not in the original brcmstb_choose_ecc_layout
code you replaced. So I am not worried about that we must switch to
mtd_ooblayout_ops at this point.  If indeed there is bug in
brcmstb_choose_ecc_layout, we can port and convert the fix to
nand_ecclayout structure from kernel code.

OK no they seemed to be mostly improvements of the algorithm so we
can certainly live without.

Was the bug you were hunting down in the code related to this patch?

Actually not, I think, it's one of the other patches I sent, the one enabling
BCH-1 by reading the proper ECC properties from the device tree.
That made it finally work.

>

The iproc NAND driver I sent should also work pretty much as-is, nothing
depends on these backports.

Yes the patches that are not relate to the ooblayout should still be good to have.

Yours,
Linus Walleij

smime.p7s
Description: S/MIME Cryptographic Signature