Date 11 Jun 2016 18:30:04 -0400 From "George Spelvin" <> Subject Re: [PATCH 2/4] mtd: nand: implement two pairing scheme
`I was just browsing LKML history and wanted to understand thisconcept, but while reading I think I spotted an error.+static void nand_pairing_dist3_get_info(struct mtd_info *mtd, int page,+					struct mtd_pairing_info *info)+{+	int lastpage = (mtd->erasesize / mtd->writesize) - 1;+	int dist = 3;++	if (page == lastpage)+		dist = 2;++	if (!page || (page & 1)) {+		info->group = 0;+		info->pair = (page + 1) / 2;+	} else {+		info->group = 1;+		info->pair = (page + 1 - dist) / 2;+	}+}As I understand this, the general rule is that odd pages i are paired witheven pages i+3, and group = !(i & 1).If there are N pages total (numbered 0..N-1), this leaves four exceptionsto deal with:-3 would be apired with 0-1 would be paired with 2N-3 would be paired with NN-1 would be paired with N+2This is solved by pairing 0 with 2, and N-1 with N-3.In terms of group addresses, there are only two exceptions:* 0 is pair 0, group 0 (not pair -1, group 1)* N-1 is pair N/2/-1, group 1 (not pair N/2, group 0)You have the first exception correct, but not the second; the conditiondetects it, but the change to "dist" doesn't matter since the first halfof the second if() will be taken.I think the easiest way to get this right is the following:	page = page + (page != 0) + (page == lastpage);	info->group = page & 1;	if (page & 1)		page -= dist;	info->pair = page >> 1;Rather than make up too many rules, this just maps 0 -> -1 and N-1 -> N,then applies the general rule.It also applies an offset of +1, to avoid negative numbers and theproblems of signed divides.Also, a very minor note: divides are expensive.A cheaper way to evaluate the "page == lastpage" condition is	if ((page + 1) * mtd->writesize == mtd->erasesize)(or you could add an mtd->write_per_erase field).Applying all of this, the corrected code looks like the following:(Note the addition of "const" and "__pure" annotations, which shouldget copied to the "struct mtd_pairing_scheme" declaration.)Signed-off-by: George Spelvin <linux@sciencehorizons.net>/* * In distance-3 pairing, odd pages i are group 0, and are paired * with even pages i+3.  The exceptions are the first page (page 0) * and last page (page N-1) in an erase group.  These pair as if * they were pages -1 and N, respectively. */static void nand_pairing_dist3_get_info(const struct mtd_info *mtd, int page,					struct mtd_pairing_info *info){	page += (page != 0) + ((page + 1) * mtd->writesize == mtd->erasesize);	info->group = page & 1;	if (page & 1)		page -= 3;	info->pair = page >> 1;}static int __pure nand_pairing_dist3_get_wunit(const struct mtd_info *mtd,					const struct mtd_pairing_info *info){	int page = 2 * info->pair + 3 * info->group;	page -= (page != 0) + (page * mtd->writesize > mtd->erasesize);	return page;}const struct mtd_pairing_scheme dist3_pairing_scheme = {	.ngroups = 2,	.get_info = nand_pairing_dist3_get_info,	.get_wunit = nand_pairing_dist3_get_wunit,};EXPORT_SYMBOL_GPL(dist3_pairing_scheme);/* * Distance-6 pairing works like distance-3 pairing, except that pages * are taken two at a time.  The lsbit of the page number is chopped off * and later re-added as the lsbit of the pair number. */static void nand_pairing_dist6_get_info(const struct mtd_info *mtd, int page,					struct mtd_pairing_info *info){	bool lsbit = page & 1;	page >>= 1;	page += (page != 0) + ((page+1) * 2*mtd->writesize == mtd->erasesize);	info->group = page & 1;	if (page & 1)		page -= 3;	info->pair = page | lsbit;}static int __pure nand_pairing_dist6_get_wunit(const struct mtd_info *mtd,				       const struct mtd_pairing_info *info){	int page = (info->pair & ~1) + 3 * info->group;	page -= (page != 0) + (page * 2 * mtd->writesize > mtd->erasesize);	return 2 * page + (info->pair & 1);}const struct mtd_pairing_scheme dist6_pairing_scheme = {	.ngroups = 2,	.get_info = nand_pairing_dist6_get_info,	.get_wunit = nand_pairing_dist6_get_wunit,};EXPORT_SYMBOL_GPL(dist6_pairing_scheme);`

Last update: 2016-06-12 00:41    [W:0.054 / U:0.968 seconds]