On Wed, 5 Jul 2017 09:04:53 +0200 Boris Brezillon <[email protected]> wrote:
> Hi Miquel, > > On Wed, 5 Jul 2017 08:51:09 +0200 > Miquel Raynal <[email protected]> wrote: > > > When using soft ecc, if no ooblayout is given, the core automatically > > uses one of the nand_ooblayout_{sp,lp}*() functions to determine the > > layout inside the out of band data. > > > > Until kernel version 4.6, struct nand_ecclayout was used for that > > purpose. During the migration from 4.6 to 4.7, an error shown up in the > > small page layout, in the case oob section is only 8 bytes long. > > > > The layout was using three bytes (0, 1, 2) for ecc, two bytes (3, 4) > > as free bytes, one byte (5) for bad block marker and finally > > two bytes (6, 7) as free bytes, as shown there: > > > > [linux-4.6] drivers/mtd/nand/nand_base.c:52 > > static struct nand_ecclayout nand_oob_8 = { > > .eccbytes = 3, > > .eccpos = {0, 1, 2}, > > .oobfree = { > > {.offset = 3, > > .length = 2}, > > {.offset = 6, > > .length = 2} } > > }; > > > > This fixes the current implementation which is incoherent. It > > references bit 3 at the same time as an ecc byte and a free byte. > > > > Furthermore, it is clear with the previous implementation that there > > is only one ecc section with 8 bytes oob sections. We shall return > > -ERANGE in the nand_ooblayout_ecc_sp() function when asked for the > > second section. Applied to nand/fixes. Thanks, Boris > > > > Signed-off-by: Miquel Raynal <[email protected]> > > Looks good to me. BTW, it looks like a good candidate for stable. > No need to send a v2, I'll add the following when applying: > > Fixes: 41b207a70d3a ("mtd: nand: implement the default mtd_ooblayout_ops") > Cc: <[email protected]> > > Thanks, > > Boris > > > --- > > drivers/mtd/nand/nand_base.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > > index b1dd12729f19..c5221795a1e8 100644 > > --- a/drivers/mtd/nand/nand_base.c > > +++ b/drivers/mtd/nand/nand_base.c > > @@ -65,8 +65,14 @@ static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, > > int section, > > > > if (!section) { > > oobregion->offset = 0; > > - oobregion->length = 4; > > + if (mtd->oobsize == 16) > > + oobregion->length = 4; > > + else > > + oobregion->length = 3; > > } else { > > + if (mtd->oobsize == 8) > > + return -ERANGE; > > + > > oobregion->offset = 6; > > oobregion->length = ecc->total - 4; > > } >
