Hi William
On Thu, Jan 26, 2023 at 6:39 PM William Zhang william.zhang@broadcom.com wrote:
On 01/26/2023 12:43 AM, Linus Walleij wrote:
On Thu, Jan 26, 2023 at 2:02 AM William Zhang william.zhang@broadcom.com wrote:
Can you add your review-by?
Michael
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