lkml.org 
[lkml]   [2016]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/4] mtd: nand: implement two pairing scheme
I was just browsing LKML history and wanted to understand this
concept, 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 with
even pages i+3, and group = !(i & 1).

If there are N pages total (numbered 0..N-1), this leaves four exceptions
to deal with:

-3 would be apired with 0
-1 would be paired with 2
N-3 would be paired with N
N-1 would be paired with N+2

This 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 condition
detects it, but the change to "dist" doesn't matter since the first half
of 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 the
problems 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 should
get 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]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site