Hi Miquel, I also had a hard time understanding your email. It was quite misleading. > El 12 may 2020, a las 9:08, Miquel Raynal <miquel.raynal@xxxxxxxxxxx> escribió: > > Hi Álvaro, > > Álvaro Fernández Rojas <noltari@xxxxxxxxx> wrote on Tue, 12 May 2020 > 08:00:23 +0200: > >> The current code generates 8 oob sections: >> S1 1-5 >> ECC 6-8 >> S2 9-15 >> S3 16-21 >> ECC 22-24 >> S4 25-31 >> S5 32-37 >> ECC 38-40 >> S6 41-47 >> S7 48-53 >> ECC 54-56 >> S8 57-63 >> >> Change it by merging continuous sections: >> S1 1-5 >> ECC 6-8 >> S2 9-21 >> ECC 22-24 >> S3 25-37 >> ECC 38-40 >> S4 41-53 >> ECC 54-56 >> S5 57-63 >> >> Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops") > > Sorry for leading you the wrong way, actually this patch does not > deserve a Fixes tag. Do I need to resend this again? Looks like no matter what I do it’s always wrong... > >> Signed-off-by: Álvaro Fernández Rojas <noltari@xxxxxxxxx> >> --- >> v3: invert patch order >> v2: keep original comment and fix correctly skip byte 6 for small-page nand >> >> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 37 ++++++++++++------------ >> 1 file changed, 18 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c >> index 1c1070111ebc..0a1d76fde37b 100644 >> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c >> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c >> @@ -1100,33 +1100,32 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section, >> struct brcmnand_cfg *cfg = &host->hwcfg; >> int sas = cfg->spare_area_size << cfg->sector_size_1k; >> int sectors = cfg->page_size / (512 << cfg->sector_size_1k); >> + u32 next; >> >> - if (section >= sectors * 2) >> + if (section > sectors) >> return -ERANGE; >> >> - oobregion->offset = (section / 2) * sas; >> + next = (section * sas); >> + if (section < sectors) >> + next += 6; >> >> - if (section & 1) { >> - oobregion->offset += 9; >> - oobregion->length = 7; >> + if (section) { >> + oobregion->offset = ((section - 1) * sas) + 9; >> } else { >> - oobregion->length = 6; >> - >> - /* First sector of each page may have BBI */ >> - if (!section) { >> - /* >> - * Small-page NAND use byte 6 for BBI while large-page >> - * NAND use bytes 0 and 1. >> - */ >> - if (cfg->page_size > 512) { >> - oobregion->offset += 2; >> - oobregion->length -= 2; >> - } else { >> - oobregion->length--; >> - } >> + /* >> + * Small-page NAND use byte 6 for BBI while large-page >> + * NAND use bytes 0 and 1. >> + */ >> + if (cfg->page_size > 512) { >> + oobregion->offset = 2; >> + } else { >> + oobregion->offset = 0; >> + next--; > > This next-- seems very strange, can you explain? In this case next will be 6 (which is the first ECC byte). However, for small page NANDs byte 5 is reserved for BBT, so we want next to be 5 only in this case. > >> } >> } >> >> + oobregion->length = next - oobregion->offset; >> + >> return 0; >> } >> > > > Thanks, > Miquèl Regards, Álvaro. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel