Re: [PATCH v6] mtd: sharpslpart: Add sharpslpart partition parser

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Fri, 25 Aug 2017 00:11:29 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:


> > +/*
> > + * The logical block number assigned to a physical block is stored in the OOB
> > + * of the first page, in 3 16-bit copies with the following layout:
> > + *
> > + * 01234567 89abcdef
> > + * -------- --------
> > + * ECC BB   xyxyxy
> > + *
> > + * When reading we check that the first two copies agree.
> > + * In case of error, matching is tried using the following pairs.
> > + * Reserved values 0xffff mean the block is kept for wear leveling.
> > + *
> > + * 01234567 89abcdef
> > + * -------- --------
> > + * ECC BB   xyxy    oob[8]==oob[10] && oob[9]==oob[11]   -> byte0=8   byte1=9
> > + * ECC BB     xyxy  oob[10]==oob[12] && oob[11]==oob[13] -> byte0=10  byte1=11
> > + * ECC BB   xy  xy  oob[12]==oob[8] && oob[13]==oob[9]   -> byte0=12  byte1=13  
> 
> I know there's a depends on "MTD_NAND_SHARPSL || MTD_NAND_TMIO" in the
> Kconfig entry, but one could enable those options just to use the sharpsl
> part parser even if the OOB layout is incompatible with the sharpsl FTL.
> 
> I'd recommend that you check that OOB bytes 8 to 15 are actually free
> to be used by the MTD user in sharpsl_parse_mtd_partitions().
> 
> Can be done with something like that:
> 
> static int sharpsl_nand_check_ooblayout(struct mtd_info *mtd)
> {
> 	u8 freebytes = 0;
> 	int section = 0;
> 
> 	while (true) {
> 		struct mtd_oob_region oobfree = { };
> 		int ret, i;
> 
> 		ret = mtd_ooblayout_free(mtd, section++, &oobfree);
> 		if (ret)
> 			break;
> 
> 		if (!oobfree.length || oobfree.offset > 15 ||
> 		    (oobfree.offset + oobfree.length) < 8)
> 			continue;
> 
> 		i = oobfree.offset >= 8 ? : 8;

As you reported on IRC there's an mistake here, it should be:

		i = oobfree.offset >= 8 ? oobfree.offset : 8

> 		for (; i < oobfree.offset + oobfree.length && i < 16; i++)
> 			freebytes |= BIT(i - 8);
> 
> 		if (freebytes == 0xff)
> 			return 0;
> 	}
> 
> 	return -ENOTSUPP;
> }