* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc [not found] <19F8576C6E063C45BE387C64729E739403CD517C16@dbde02.ent.ti.com> @ 2008-08-21 10:51 ` Singh, Vimal 2008-08-21 10:55 ` Artem Bityutskiy 2008-08-21 18:39 ` Frans Meulenbroeks 0 siblings, 2 replies; 12+ messages in thread From: Singh, Vimal @ 2008-08-21 10:51 UTC (permalink / raw) To: frans; +Cc: Woodhouse, linux-mtd@lists.infradead.org, David Hi Frans, Frans Meulenbroeks wrote: > This patch for nand_ecc.c fixes three issues > > - fix code so it also works on big endian architectures > - added a printk in case of an uncorrectable ecc error > - strenghten the test for correctable errors (decreasing the chance > that multiple bit faults by accident will be seen as correctable) > > Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com> > > diff --git a/drivers/mtd/nand/nand_ecc.c b/drivers/mtd/nand/nand_ecc.c > index a8e8413..d99e569 100644 We can make this to support for 512 byte ecc calculation and correction also. It can be helpful, particularly in cases where hardware ecc option is present and is using 512 byte ecc, to make software ecc and hardware ecc compatible. So that switching could be possible at run time i.e. both can be used by switching at load time. I have prepared a patch on top of your latest patch. Please review it and provide your comments on this. ---- Thanks and Regards, vimal singh Signed-off-by: Vimal Singh <vimalsingh@ti.com> Index: linux-omap-2.6/drivers/mtd/nand/nand_ecc.c =================================================================== --- linux-omap-2.6.orig/drivers/mtd/nand/nand_ecc.c 2008-08-21 10:33:30.000000000 +0530 +++ linux-omap-2.6/drivers/mtd/nand/nand_ecc.c 2008-08-21 10:50:39.000000000 +0530 @@ -42,6 +42,8 @@ #include <linux/types.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> #include <linux/mtd/nand_ecc.h> #include <asm/byteorder.h> #else @@ -158,10 +160,11 @@ { int i; const uint32_t *bp = (uint32_t *)buf; + /* 256 or 512 bytes/ecc */ + uint32_t j =(((struct nand_chip *)mtd->priv)->ecc.size)/256; uint32_t cur; /* current value in buffer */ - /* rp0..rp15 are the various accumulated parities (per byte) */ + /* rp0..rp15..rp17 are the various accumulated parities (per byte) */ uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7; - uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15; + uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15, rp16, rp17; uint32_t par; /* the cumulative parity for all data */ uint32_t tmppar; /* the cumulative parity for this iteration; for rp12 and rp14 at the end of the loop */ @@ -173,6 +176,8 @@ rp10 = 0; rp12 = 0; rp14 = 0; + if (j == 2) + rp16 = 0; /* * The loop is unrolled a number of times; @@ -184,7 +189,7 @@ * needed for calculating rp12, rp14 and par * also used as a performance improvement for rp6, rp8 and rp10 */ - for (i = 0; i < 4; i++) { + for (i = 0; i < 4*j; i++) { cur = *bp++; tmppar = cur; rp4 ^= cur; @@ -247,6 +252,8 @@ rp12 ^= tmppar; if ((i & 0x2) == 0) rp14 ^= tmppar; + if (j == 2 && (i & 0x4) == 0) + rp16 ^= tmppar; } /* @@ -273,6 +280,11 @@ rp14 ^= (rp14 >> 16); rp14 ^= (rp14 >> 8); rp14 &= 0xff; + if (j == 2) { + rp16 ^= (rp16 >> 16); + rp16 ^= (rp16 >> 8); + rp16 &= 0xff; + } /* * we also need to calculate the row parity for rp0..rp3 @@ -329,6 +341,8 @@ rp11 = (par ^ rp10) & 0xff; rp13 = (par ^ rp12) & 0xff; rp15 = (par ^ rp14) & 0xff; + if (j == 2) + rp17 = (par ^ rp16) & 0xff; /* * Finally calculate the ecc bits. @@ -375,14 +389,25 @@ (invparity[rp9] << 1) | (invparity[rp8]); #endif - code[2] = - (invparity[par & 0xf0] << 7) | - (invparity[par & 0x0f] << 6) | - (invparity[par & 0xcc] << 5) | - (invparity[par & 0x33] << 4) | - (invparity[par & 0xaa] << 3) | - (invparity[par & 0x55] << 2) | - 3; + if (j == 1) + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + 3; + else if (j == 2) + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + (invparity[rp17] << 1) | + (invparity[rp16] << 0); return 0; } EXPORT_SYMBOL(nand_calculate_ecc); @@ -401,6 +426,7 @@ { unsigned char b0, b1, b2; unsigned char byte_addr, bit_addr; + /* 256 or 512 bytes/ecc */ + uint32_t j = (((struct nand_chip *)mtd->priv)->ecc.size)/256; /* * b0 to b2 indicate which bit is faulty (if any) @@ -426,7 +452,8 @@ if ((((b0 ^ (b0 >> 1)) & 0x55) == 0x55) && (((b1 ^ (b1 >> 1)) & 0x55) == 0x55) && - (((b2 ^ (b2 >> 1)) & 0x54) == 0x54)) { /* single bit error */ + ((j == 1 && ((b2 ^ (b2 >> 1)) & 0x54) == 0x54) || + (j == 2 && ((b2 ^ (b2 >> 1)) & 0x55) == 0x55))) { + /* single bit error */ /* * rp15/13/11/9/7/5/3/1 indicate which byte is the faulty byte * cp 5/3/1 indicate the faulty bit. @@ -443,7 +470,10 @@ * We could also do addressbits[b2] >> 1 but for the * performace it does not make any difference */ - byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + if (j == 1) + byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + else if (j == 2) + byte_addr = (addressbits[b2 & 0x3] << 8) + + (addressbits[b1] << 4) + addressbits[b0]; bit_addr = addressbits[b2 >> 2]; /* flip the bit */ buf[byte_addr] ^= (1 << bit_addr); ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-21 10:51 ` [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc Singh, Vimal @ 2008-08-21 10:55 ` Artem Bityutskiy 2008-08-21 18:39 ` Frans Meulenbroeks 1 sibling, 0 replies; 12+ messages in thread From: Artem Bityutskiy @ 2008-08-21 10:55 UTC (permalink / raw) To: Singh, Vimal; +Cc: linux-mtd@lists.infradead.org, frans, Woodhouse, David Hi, > + /* 256 or 512 bytes/ecc */ > + uint32_t j =(((struct nand_chip *)mtd->priv)->ecc.size)/256; It's certainly faster to use >> 8 instead. At least on x86 GCC would not optimize this for you if the kernel is compiles with -Os, it would use idiv instead. -- Best regards, Artem Bityutskiy (Битюцкий Артём) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-21 10:51 ` [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc Singh, Vimal 2008-08-21 10:55 ` Artem Bityutskiy @ 2008-08-21 18:39 ` Frans Meulenbroeks 2008-08-22 5:39 ` Singh, Vimal 1 sibling, 1 reply; 12+ messages in thread From: Frans Meulenbroeks @ 2008-08-21 18:39 UTC (permalink / raw) To: Singh, Vimal; +Cc: David Woodhouse, linux-mtd@lists.infradead.org Hi Vimal, Thanks for your patch. I tried to apply your patch, but only hunk 1 succeeded with fuzz 2. Guess this is because shortly after submitting my patch another patch has been applied to it. Could you verify against the latest version in git and resubmit the patch. Also there are a few small optimisation possibilities. E.g. the code: + if (j == 2) + rp16 = 0; Here the if statement does not really add. for 256 byte ecc initialising rp16 does not harm so the if statement creates additional overhead. There are a fw more of these (like the /256 reported by Artem. I'll happily fix these once I have a patch that I can apply. Best regards, Frans ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-21 18:39 ` Frans Meulenbroeks @ 2008-08-22 5:39 ` Singh, Vimal 2008-08-22 5:53 ` Artem Bityutskiy 2008-08-22 6:07 ` Artem Bityutskiy 0 siblings, 2 replies; 12+ messages in thread From: Singh, Vimal @ 2008-08-22 5:39 UTC (permalink / raw) To: Frans Meulenbroeks; +Cc: Woodhouse, linux-mtd@lists.infradead.org, David Hi Frans, >I tried to apply your patch, but only hunk 1 succeeded with fuzz 2. >Guess this is because shortly after submitting my patch another patch >has been applied to it. >Could you verify against the latest version in git and resubmit the patch. I have prepared the patch once again on top of latest version in git. Hopefully it should apply properly. Regards, vimal Signed-off-by: Vimal Singh <vimalsingh@ti.com> ----- nand_ecc.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 49 insertions(+), 13 deletions(-) --- a/drivers/mtd/nand/nand_ecc.c 2008-08-22 10:41:53.000000000 +0530 +++ b/drivers/mtd/nand/nand_ecc.c 2008-08-22 10:47:40.000000000 +0530 @@ -42,6 +42,8 @@ #include <linux/types.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> #include <linux/mtd/nand_ecc.h> #else #include <stdint.h> @@ -154,10 +156,12 @@ int nand_calculate_ecc(struct mtd_info * { int i; const uint32_t *bp = (uint32_t *)buf; + /* 256 or 512 bytes/ecc */ + uint32_t j =(((struct nand_chip *)mtd->priv)->ecc.size) >> 8; uint32_t cur; /* current value in buffer */ - /* rp0..rp15 are the various accumulated parities (per byte) */ + /* rp0..rp15..rp17 are the various accumulated parities (per byte) */ uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7; - uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15; + uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15, rp16, rp17; uint32_t par; /* the cumulative parity for all data */ uint32_t tmppar; /* the cumulative parity for this iteration; for rp12 and rp14 at the end of the loop */ @@ -169,6 +173,8 @@ int nand_calculate_ecc(struct mtd_info * rp10 = 0; rp12 = 0; rp14 = 0; + rp16 = 0; + rp17 = 0; /* to make cmpiler happy */ /* * The loop is unrolled a number of times; @@ -180,7 +186,7 @@ int nand_calculate_ecc(struct mtd_info * * needed for calculating rp12, rp14 and par * also used as a performance improvement for rp6, rp8 and rp10 */ - for (i = 0; i < 4; i++) { + for (i = 0; i < 4*j; i++) { cur = *bp++; tmppar = cur; rp4 ^= cur; @@ -243,6 +249,8 @@ int nand_calculate_ecc(struct mtd_info * rp12 ^= tmppar; if ((i & 0x2) == 0) rp14 ^= tmppar; + if (j == 2 && (i & 0x4) == 0) + rp16 ^= tmppar; } /* @@ -269,6 +277,11 @@ int nand_calculate_ecc(struct mtd_info * rp14 ^= (rp14 >> 16); rp14 ^= (rp14 >> 8); rp14 &= 0xff; + if (j == 2) { + rp16 ^= (rp16 >> 16); + rp16 ^= (rp16 >> 8); + rp16 &= 0xff; + } /* * we also need to calculate the row parity for rp0..rp3 @@ -311,6 +324,8 @@ int nand_calculate_ecc(struct mtd_info * rp11 = (par ^ rp10) & 0xff; rp13 = (par ^ rp12) & 0xff; rp15 = (par ^ rp14) & 0xff; + if (j == 2) + rp17 = (par ^ rp16) & 0xff; /* * Finally calculate the ecc bits. @@ -357,14 +372,25 @@ int nand_calculate_ecc(struct mtd_info * (invparity[rp9] << 1) | (invparity[rp8]); #endif - code[2] = - (invparity[par & 0xf0] << 7) | - (invparity[par & 0x0f] << 6) | - (invparity[par & 0xcc] << 5) | - (invparity[par & 0x33] << 4) | - (invparity[par & 0xaa] << 3) | - (invparity[par & 0x55] << 2) | - 3; + if (j == 1) + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + 3; + else if (j == 2) + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + (invparity[rp17] << 1) | + (invparity[rp16] << 0); return 0; } EXPORT_SYMBOL(nand_calculate_ecc); @@ -384,6 +410,8 @@ int nand_correct_data(struct mtd_info *m int nr_bits; unsigned char b0, b1, b2; unsigned char byte_addr, bit_addr; + /* 256 or 512 bytes/ecc */ + uint32_t j = (((struct nand_chip *)mtd->priv)->ecc.size) >> 8; /* * b0 to b2 indicate which bit is faulty (if any) @@ -408,7 +436,11 @@ int nand_correct_data(struct mtd_info *m /* ordered in order of likelihood */ if (nr_bits == 0) return 0; /* no error */ - if (nr_bits == 11) { /* correctable error */ + if ((((b0 ^ (b0 >> 1)) & 0x55) == 0x55) && + (((b1 ^ (b1 >> 1)) & 0x55) == 0x55) && + ((j == 1 && ((b2 ^ (b2 >> 1)) & 0x54) == 0x54) || + (j == 2 && ((b2 ^ (b2 >> 1)) & 0x55) == 0x55))) { + /* single bit error */ /* * rp15/13/11/9/7/5/3/1 indicate which byte is the faulty byte * cp 5/3/1 indicate the faulty bit. @@ -425,7 +457,11 @@ int nand_correct_data(struct mtd_info *m * We could also do addressbits[b2] >> 1 but for the * performace it does not make any difference */ - byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + if (j == 1) + byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + else if (j == 2) + byte_addr = (addressbits[b2 & 0x3] << 8) + + (addressbits[b1] << 4) + addressbits[b0]; bit_addr = addressbits[b2 >> 2]; /* flip the bit */ buf[byte_addr] ^= (1 << bit_addr); ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 5:39 ` Singh, Vimal @ 2008-08-22 5:53 ` Artem Bityutskiy 2008-08-22 6:07 ` Artem Bityutskiy 1 sibling, 0 replies; 12+ messages in thread From: Artem Bityutskiy @ 2008-08-22 5:53 UTC (permalink / raw) To: Singh, Vimal Cc: linux-mtd@lists.infradead.org, Frans Meulenbroeks, Woodhouse, David On Fri, 2008-08-22 at 11:09 +0530, Singh, Vimal wrote: > @@ -154,10 +156,12 @@ int nand_calculate_ecc(struct mtd_info * > { > int i; > const uint32_t *bp = (uint32_t *)buf; > + /* 256 or 512 bytes/ecc */ > + uint32_t j =(((struct nand_chip *)mtd->priv)->ecc.size) >> 8; "j" is not descriptive. And I guess "const" modifier may be used for it. Not sure it'll give any performance benefit, but it may affect gcc optimizations. I would instead introduce: const int eccsize_mult = ((struct nand_chip *)mtd->priv)->ecc.size >> 8; > uint32_t cur; /* current value in buffer */ > - /* rp0..rp15 are the various accumulated parities (per byte) */ > + /* rp0..rp15..rp17 are the various accumulated parities (per byte) */ > uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7; > - uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15; > + uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15, rp16, rp17; > uint32_t par; /* the cumulative parity for all data */ > uint32_t tmppar; /* the cumulative parity for this iteration; > for rp12 and rp14 at the end of the loop */ > @@ -169,6 +173,8 @@ int nand_calculate_ecc(struct mtd_info * > rp10 = 0; > rp12 = 0; > rp14 = 0; > + rp16 = 0; > + rp17 = 0; /* to make cmpiler happy */ > > /* > * The loop is unrolled a number of times; > @@ -180,7 +186,7 @@ int nand_calculate_ecc(struct mtd_info * > * needed for calculating rp12, rp14 and par > * also used as a performance improvement for rp6, rp8 and rp10 > */ > - for (i = 0; i < 4; i++) { > + for (i = 0; i < 4*j; i++) { Here I would rather use: for (i = 0; i < eccsize_mult << 2; i++) { to avoid multiplication. > + code[2] = > + (invparity[par & 0xf0] << 7) | > + (invparity[par & 0x0f] << 6) | > + (invparity[par & 0xcc] << 5) | > + (invparity[par & 0x33] << 4) | > + (invparity[par & 0xaa] << 3) | > + (invparity[par & 0x55] << 2) | > + 3; > + else if (j == 2) this should be just else, not else if. > + code[2] = > + (invparity[par & 0xf0] << 7) | > + (invparity[par & 0x0f] << 6) | > + (invparity[par & 0xcc] << 5) | > + (invparity[par & 0x33] << 4) | > + (invparity[par & 0xaa] << 3) | > + (invparity[par & 0x55] << 2) | > + (invparity[rp17] << 1) | > + (invparity[rp16] << 0); > - byte_addr = (addressbits[b1] << 4) + addressbits[b0]; > + if (j == 1) > + byte_addr = (addressbits[b1] << 4) + addressbits[b0]; > + else if (j == 2) > + byte_addr = (addressbits[b2 & 0x3] << 8) + > + (addressbits[b1] << 4) + addressbits[b0]; Similar. -- Best regards, Artem Bityutskiy (Битюцкий Артём) ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 5:39 ` Singh, Vimal 2008-08-22 5:53 ` Artem Bityutskiy @ 2008-08-22 6:07 ` Artem Bityutskiy 2008-08-22 8:43 ` Frans Meulenbroeks 2008-08-22 8:45 ` Singh, Vimal 1 sibling, 2 replies; 12+ messages in thread From: Artem Bityutskiy @ 2008-08-22 6:07 UTC (permalink / raw) To: Singh, Vimal Cc: linux-mtd@lists.infradead.org, Frans Meulenbroeks, Woodhouse, David On Fri, 2008-08-22 at 11:09 +0530, Singh, Vimal wrote: > + rp16 = 0; > + rp17 = 0; /* to make cmpiler happy */ Please, use uint32_t uninitialized_var(rp17); to avoid unnecessary initialization to 0 and avoid gcc warning as well. -- Best regards, Artem Bityutskiy (Битюцкий Артём) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 6:07 ` Artem Bityutskiy @ 2008-08-22 8:43 ` Frans Meulenbroeks 2008-08-22 9:06 ` David Woodhouse 2008-08-22 8:45 ` Singh, Vimal 1 sibling, 1 reply; 12+ messages in thread From: Frans Meulenbroeks @ 2008-08-22 8:43 UTC (permalink / raw) To: dedekind; +Cc: linux-mtd@lists.infradead.org, Singh, Vimal, Woodhouse, David @Vimal: I'll try to look at your patch this weekend (no time this evening) Let me know if you want to handle the remarks from Artem (and come with a new patch) or if you want me to handle them (there are maybe one or two other small changes I want to make. @David (dwmw2) Please let me know what the preferred way to integrate this. I can either provide a patch on top of Vimal' patch if needed, or I can add my changes to it and submit a patch containing the work of both of us, or I can send my changes as review comment to Vimal and let him make the patch. What is the preferred way? (and I highly prefer a solution where Vimal gets credit for his changes) Frans. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 8:43 ` Frans Meulenbroeks @ 2008-08-22 9:06 ` David Woodhouse 0 siblings, 0 replies; 12+ messages in thread From: David Woodhouse @ 2008-08-22 9:06 UTC (permalink / raw) To: Frans Meulenbroeks; +Cc: Singh, Vimal, linux-mtd@lists.infradead.org, David On Fri, 2008-08-22 at 10:43 +0200, Frans Meulenbroeks wrote: > @David (dwmw2) > Please let me know what the preferred way to integrate this. I can > either provide a patch on top of Vimal' patch if needed, or I can add > my changes to it and submit a patch containing the work of both of us, > or I can send my changes as review comment to Vimal and let him make > the patch. What is the preferred way? (and I highly prefer a solution > where Vimal gets credit for his changes) When you forward the patch on to me, put 'From: Vimal Singh <...>' as the first line of the _body_ of the email so that it is still attributed to him. It's reasonable to preserve his signed-off-by, if you're only making relatively minor changes, and document what _you_ changed in square brackets. For an example, see http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg00461.html Alternatively, if his patch isn't actually introducing a regression, it doesn't hurt to do it as two separate patches -- whichever you prefer. When resending an updated patch, make sure the proper commit comment remains intact -- if you want to add some commentary like 'fixed up according to review and made some other fixes', which doesn't live in the commit comment but follows on from the email thread, then add it after a line with three dashes -- again, you can see that in the example above. -- David Woodhouse Open Source Technology Centre David.Woodhouse@intel.com Intel Corporation ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 6:07 ` Artem Bityutskiy 2008-08-22 8:43 ` Frans Meulenbroeks @ 2008-08-22 8:45 ` Singh, Vimal 2008-08-22 8:58 ` Frans Meulenbroeks 1 sibling, 1 reply; 12+ messages in thread From: Singh, Vimal @ 2008-08-22 8:45 UTC (permalink / raw) To: dedekind@infradead.org Cc: linux-mtd@lists.infradead.org, Frans Meulenbroeks, Woodhouse, David@mgw-mx06.nokia.com >On Fri, 2008-08-22 at 11:09 +0530, Singh, Vimal wrote: >> + rp16 = 0; > >+ rp17 = 0; /* to make cmpiler happy */ >Please, use > >uint32_t uninitialized_var(rp17); > >to avoid unnecessary initialization to 0 and avoid gcc warning as well. Here is the patch after taking your inputs with some additional changes in comments. ---- Thanks and Regards, vimal singh Signed-off-by: Vimal Singh <vimalsingh@ti.com> --- drivers/mtd/nand/nand_ecc.c | 84 ++++++++++++++++++++++++++++++++------------ 1 files changed, 62 insertions(+), 22 deletions(-) Index: linux-omap-2.6/drivers/mtd/nand/nand_ecc.c =================================================================== --- linux-omap-2.6.orig/drivers/mtd/nand/nand_ecc.c 2008-08-22 12:58:33.000000000 +0530 +++ linux-omap-2.6/drivers/mtd/nand/nand_ecc.c 2008-08-22 13:10:10.000000000 +0530 @@ -42,6 +42,8 @@ #include <linux/types.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> #include <linux/mtd/nand_ecc.h> #else #include <stdint.h> @@ -144,7 +146,8 @@ }; /** - * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256-byte block + * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte + * block * @mtd: MTD block structure (unused) * @dat: raw data * @ecc_code: buffer for ECC @@ -154,13 +157,18 @@ { int i; const uint32_t *bp = (uint32_t *)buf; + /* 256 or 512 bytes/ecc */ + const uint32_t eccsize_mult = + (((struct nand_chip *)mtd->priv)->ecc.size) >> 8; uint32_t cur; /* current value in buffer */ - /* rp0..rp15 are the various accumulated parities (per byte) */ + /* rp0..rp15..rp17 are the various accumulated parities (per byte) */ uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7; - uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15; + uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15, rp16; + uint32_t uninitialized_var(rp17); /* to keep compiler happy */ uint32_t par; /* the cumulative parity for all data */ uint32_t tmppar; /* the cumulative parity for this iteration; - for rp12 and rp14 at the end of the loop */ + for rp12, rp14 and rp16 at the end of the + loop */ par = 0; rp4 = 0; @@ -169,6 +177,7 @@ rp10 = 0; rp12 = 0; rp14 = 0; + rp16 = 0; /* * The loop is unrolled a number of times; @@ -177,10 +186,10 @@ * Note: passing unaligned data might give a performance penalty. * It is assumed that the buffers are aligned. * tmppar is the cumulative sum of this iteration. - * needed for calculating rp12, rp14 and par + * needed for calculating rp12, rp14, rp16 and par * also used as a performance improvement for rp6, rp8 and rp10 */ - for (i = 0; i < 4; i++) { + for (i = 0; i < eccsize_mult << 2; i++) { cur = *bp++; tmppar = cur; rp4 ^= cur; @@ -243,12 +252,14 @@ rp12 ^= tmppar; if ((i & 0x2) == 0) rp14 ^= tmppar; + if (eccsize_mult == 2 && (i & 0x4) == 0) + rp16 ^= tmppar; } /* * handle the fact that we use longword operations - * we'll bring rp4..rp14 back to single byte entities by shifting and - * xoring first fold the upper and lower 16 bits, + * we'll bring rp4..rp14..rp16 back to single byte entities by + * shifting and xoring first fold the upper and lower 16 bits, * then the upper and lower 8 bits. */ rp4 ^= (rp4 >> 16); @@ -269,6 +280,11 @@ rp14 ^= (rp14 >> 16); rp14 ^= (rp14 >> 8); rp14 &= 0xff; + if (eccsize_mult == 2) { + rp16 ^= (rp16 >> 16); + rp16 ^= (rp16 >> 8); + rp16 &= 0xff; + } /* * we also need to calculate the row parity for rp0..rp3 @@ -297,7 +313,7 @@ par &= 0xff; /* - * and calculate rp5..rp15 + * and calculate rp5..rp15..rp17 * note that par = rp4 ^ rp5 and due to the commutative property * of the ^ operator we can say: * rp5 = (par ^ rp4); @@ -311,6 +327,8 @@ rp11 = (par ^ rp10) & 0xff; rp13 = (par ^ rp12) & 0xff; rp15 = (par ^ rp14) & 0xff; + if (eccsize_mult == 2) + rp17 = (par ^ rp16) & 0xff; /* * Finally calculate the ecc bits. @@ -357,14 +375,25 @@ (invparity[rp9] << 1) | (invparity[rp8]); #endif - code[2] = - (invparity[par & 0xf0] << 7) | - (invparity[par & 0x0f] << 6) | - (invparity[par & 0xcc] << 5) | - (invparity[par & 0x33] << 4) | - (invparity[par & 0xaa] << 3) | - (invparity[par & 0x55] << 2) | - 3; + if (eccsize_mult == 1) + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + 3; + else + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + (invparity[rp17] << 1) | + (invparity[rp16] << 0); return 0; } EXPORT_SYMBOL(nand_calculate_ecc); @@ -376,7 +405,7 @@ * @read_ecc: ECC from the chip * @calc_ecc: the ECC calculated from raw data * - * Detect and correct a 1 bit error for 256 byte block + * Detect and correct a 1 bit error for 256/512 byte block */ int nand_correct_data(struct mtd_info *mtd, unsigned char *buf, unsigned char *read_ecc, unsigned char *calc_ecc) @@ -384,6 +413,9 @@ int nr_bits; unsigned char b0, b1, b2; unsigned char byte_addr, bit_addr; + /* 256 or 512 bytes/ecc */ + const uint32_t eccsize_mult = + (((struct nand_chip *)mtd->priv)->ecc.size) >> 8; /* * b0 to b2 indicate which bit is faulty (if any) @@ -408,10 +440,14 @@ /* ordered in order of likelihood */ if (nr_bits == 0) return 0; /* no error */ - if (nr_bits == 11) { /* correctable error */ + if ((((b0 ^ (b0 >> 1)) & 0x55) == 0x55) && + (((b1 ^ (b1 >> 1)) & 0x55) == 0x55) && + ((eccsize_mult == 1 && ((b2 ^ (b2 >> 1)) & 0x54) == 0x54) || + (eccsize_mult == 2 && ((b2 ^ (b2 >> 1)) & 0x55) == 0x55))) { + /* single bit error */ /* - * rp15/13/11/9/7/5/3/1 indicate which byte is the faulty byte - * cp 5/3/1 indicate the faulty bit. + * rp17/rp15/13/11/9/7/5/3/1 indicate which byte is the faulty + * byte, cp 5/3/1 indicate the faulty bit. * A lookup table (called addressbits) is used to filter * the bits from the byte they are in. * A marginal optimisation is possible by having three @@ -425,7 +461,11 @@ * We could also do addressbits[b2] >> 1 but for the * performace it does not make any difference */ - byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + if (eccsize_mult == 1) + byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + else + byte_addr = (addressbits[b2 & 0x3] << 8) + + (addressbits[b1] << 4) + addressbits[b0]; bit_addr = addressbits[b2 >> 2]; /* flip the bit */ buf[byte_addr] ^= (1 << bit_addr); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 8:45 ` Singh, Vimal @ 2008-08-22 8:58 ` Frans Meulenbroeks 2008-08-22 9:27 ` Singh, Vimal 0 siblings, 1 reply; 12+ messages in thread From: Frans Meulenbroeks @ 2008-08-22 8:58 UTC (permalink / raw) To: Singh, Vimal Cc: linux-mtd@lists.infradead.org, Woodhouse, David@mgw-mx06.nokia.com Vimal, This is not a diff against the latest git version. E.g. this line > - if (nr_bits == 11) { /* correctable error */ was already replaced by the 0x55 tests in this commit: http://git.infradead.org/mtd-2.6.git?a=commitdiff;h=1077be58ad7baadd86e47e8b4f6209fa5b6364a5 Also in your diff I see that this patch: http://git.infradead.org/mtd-2.6.git?a=commitdiff;h=17c1d2be28e485c0c8b09661db39d5bf2605069d is not in the version you compare against. I suggest to merge your changes with the mtd git version of nand_ecc.c The current HEAD for this file can be found here: http://git.infradead.org/mtd-2.6.git?a=blob_plain;f=drivers/mtd/nand/nand_ecc.c;hb=17c1d2be28e485c0c8b09661db39d5bf2605069d Best regards, Frans. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 8:58 ` Frans Meulenbroeks @ 2008-08-22 9:27 ` Singh, Vimal 2008-08-23 16:18 ` frans 0 siblings, 1 reply; 12+ messages in thread From: Singh, Vimal @ 2008-08-22 9:27 UTC (permalink / raw) To: Frans Meulenbroeks Cc: linux-mtd@lists.infradead.org, Woodhouse, David@mgw-mx06.nokia.com Frans, Below is the patch prepared on top of the current git head. >I suggest to merge your changes with the mtd git version of nand_ecc.c >The current HEAD for this file can be found here: >http://git.infradead.org/mtd-2.6.git?a=blob_plain;f=drivers/mtd/nand/nand_ecc.c;hb=17c1d2be28e485c0c8b09661db39d5bf2605069d Sorry for sending patches on wrong versions... --- drivers/mtd/nand/nand_ecc.c | 82 ++++++++++++++++++++++++++++++++------------ 1 files changed, 60 insertions(+), 22 deletions(-) Index: linux-omap-2.6/drivers/mtd/nand/nand_ecc.c =================================================================== --- linux-omap-2.6.orig/drivers/mtd/nand/nand_ecc.c 2008-08-22 14:35:48.000000000 +0530 +++ linux-omap-2.6/drivers/mtd/nand/nand_ecc.c 2008-08-22 14:46:41.000000000 +0530 @@ -42,6 +42,8 @@ #include <linux/types.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> #include <linux/mtd/nand_ecc.h> #include <asm/byteorder.h> #else @@ -148,7 +150,8 @@ }; /** - * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256-byte block + * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte + * block * @mtd: MTD block structure (unused) * @buf: input buffer with raw data * @code: output buffer with ECC @@ -158,13 +161,18 @@ { int i; const uint32_t *bp = (uint32_t *)buf; + /* 256 or 512 bytes/ecc */ + const uint32_t eccsize_mult = + (((struct nand_chip *)mtd->priv)->ecc.size) >> 8; uint32_t cur; /* current value in buffer */ - /* rp0..rp15 are the various accumulated parities (per byte) */ + /* rp0..rp15..rp17 are the various accumulated parities (per byte) */ uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7; - uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15; + uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15, rp16; + uint32_t uninitialized_var(rp17); /* to make compiler happy */ uint32_t par; /* the cumulative parity for all data */ uint32_t tmppar; /* the cumulative parity for this iteration; - for rp12 and rp14 at the end of the loop */ + for rp12, rp14 and rp16 at the end of the + loop */ par = 0; rp4 = 0; @@ -173,6 +181,7 @@ rp10 = 0; rp12 = 0; rp14 = 0; + rp16 = 0; /* * The loop is unrolled a number of times; @@ -181,10 +190,10 @@ * Note: passing unaligned data might give a performance penalty. * It is assumed that the buffers are aligned. * tmppar is the cumulative sum of this iteration. - * needed for calculating rp12, rp14 and par + * needed for calculating rp12, rp14, rp16 and par * also used as a performance improvement for rp6, rp8 and rp10 */ - for (i = 0; i < 4; i++) { + for (i = 0; i < eccsize_mult << 2; i++) { cur = *bp++; tmppar = cur; rp4 ^= cur; @@ -247,12 +256,14 @@ rp12 ^= tmppar; if ((i & 0x2) == 0) rp14 ^= tmppar; + if (eccsize_mult == 2 && (i & 0x4) == 0) + rp16 ^= tmppar; } /* * handle the fact that we use longword operations - * we'll bring rp4..rp14 back to single byte entities by shifting and - * xoring first fold the upper and lower 16 bits, + * we'll bring rp4..rp14..rp16 back to single byte entities by + * shifting and xoring first fold the upper and lower 16 bits, * then the upper and lower 8 bits. */ rp4 ^= (rp4 >> 16); @@ -273,6 +284,11 @@ rp14 ^= (rp14 >> 16); rp14 ^= (rp14 >> 8); rp14 &= 0xff; + if (eccsize_mult == 2) { + rp16 ^= (rp16 >> 16); + rp16 ^= (rp16 >> 8); + rp16 &= 0xff; + } /* * we also need to calculate the row parity for rp0..rp3 @@ -315,7 +331,7 @@ par &= 0xff; /* - * and calculate rp5..rp15 + * and calculate rp5..rp15..rp17 * note that par = rp4 ^ rp5 and due to the commutative property * of the ^ operator we can say: * rp5 = (par ^ rp4); @@ -329,6 +345,8 @@ rp11 = (par ^ rp10) & 0xff; rp13 = (par ^ rp12) & 0xff; rp15 = (par ^ rp14) & 0xff; + if (eccsize_mult == 2) + rp17 = (par ^ rp16) & 0xff; /* * Finally calculate the ecc bits. @@ -375,14 +393,25 @@ (invparity[rp9] << 1) | (invparity[rp8]); #endif - code[2] = - (invparity[par & 0xf0] << 7) | - (invparity[par & 0x0f] << 6) | - (invparity[par & 0xcc] << 5) | - (invparity[par & 0x33] << 4) | - (invparity[par & 0xaa] << 3) | - (invparity[par & 0x55] << 2) | - 3; + if (eccsize_mult == 1) + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + 3; + else + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + (invparity[rp17] << 1) | + (invparity[rp16] << 0); return 0; } EXPORT_SYMBOL(nand_calculate_ecc); @@ -394,13 +423,16 @@ * @read_ecc: ECC from the chip * @calc_ecc: the ECC calculated from raw data * - * Detect and correct a 1 bit error for 256 byte block + * Detect and correct a 1 bit error for 256/512 byte block */ int nand_correct_data(struct mtd_info *mtd, unsigned char *buf, unsigned char *read_ecc, unsigned char *calc_ecc) { unsigned char b0, b1, b2; unsigned char byte_addr, bit_addr; + /* 256 or 512 bytes/ecc */ + const uint32_t eccsize_mult = + (((struct nand_chip *)mtd->priv)->ecc.size) >> 8; /* * b0 to b2 indicate which bit is faulty (if any) @@ -426,10 +458,12 @@ if ((((b0 ^ (b0 >> 1)) & 0x55) == 0x55) && (((b1 ^ (b1 >> 1)) & 0x55) == 0x55) && - (((b2 ^ (b2 >> 1)) & 0x54) == 0x54)) { /* single bit error */ + ((eccsize_mult == 1 && ((b2 ^ (b2 >> 1)) & 0x54) == 0x54) || + (eccsize_mult == 2 && ((b2 ^ (b2 >> 1)) & 0x55) == 0x55))) { + /* single bit error */ /* - * rp15/13/11/9/7/5/3/1 indicate which byte is the faulty byte - * cp 5/3/1 indicate the faulty bit. + * rp17/rp15/13/11/9/7/5/3/1 indicate which byte is the faulty + * byte, cp 5/3/1 indicate the faulty bit. * A lookup table (called addressbits) is used to filter * the bits from the byte they are in. * A marginal optimisation is possible by having three @@ -443,7 +477,11 @@ * We could also do addressbits[b2] >> 1 but for the * performace it does not make any difference */ - byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + if (eccsize_mult == 1) + byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + else + byte_addr = (addressbits[b2 & 0x3] << 8) + + (addressbits[b1] << 4) + addressbits[b0]; bit_addr = addressbits[b2 >> 2]; /* flip the bit */ buf[byte_addr] ^= (1 << bit_addr); ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc 2008-08-22 9:27 ` Singh, Vimal @ 2008-08-23 16:18 ` frans 0 siblings, 0 replies; 12+ messages in thread From: frans @ 2008-08-23 16:18 UTC (permalink / raw) To: Singh, Vimal; +Cc: linux-mtd@lists.infradead.org, Woodhouse From: "Singh, Vimal" <vimalsingh@ti.com> Support 512 byte ECC calculation [FM: updated two comments] Signed-off-by: Vimal Singh <vimalsingh@ti.com> Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com> --- recreated the patch after making the changes there are a few other minor optimisations possible, but I am planning to rework the code and rebench for ARM (there are a few statements like rp2 = (par ^ rp3) & 0xff; which were marginally faster on pentium but might be less efficient on ARM and/or MIPS. As this is typically used for embedded processors it makes sense to optimise for those targets (especially if the effect for x86 is hardly or not noticable) Frans. diff --git a/drivers/mtd/nand/nand_ecc.c b/drivers/mtd/nand/nand_ecc.c index fd19787..868147a 100644 --- a/drivers/mtd/nand/nand_ecc.c +++ b/drivers/mtd/nand/nand_ecc.c @@ -42,6 +42,8 @@ #include <linux/types.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> #include <linux/mtd/nand_ecc.h> #include <asm/byteorder.h> #else @@ -148,8 +150,9 @@ static const char addressbits[256] = { }; /** - * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256-byte block - * @mtd: MTD block structure (unused) + * nand_calculate_ecc - [NAND Interface] Calculate 3-byte ECC for 256/512-byte + * block + * @mtd: MTD block structure * @buf: input buffer with raw data * @code: output buffer with ECC */ @@ -158,13 +161,18 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, { int i; const uint32_t *bp = (uint32_t *)buf; + /* 256 or 512 bytes/ecc */ + const uint32_t eccsize_mult = + (((struct nand_chip *)mtd->priv)->ecc.size) >> 8; uint32_t cur; /* current value in buffer */ - /* rp0..rp15 are the various accumulated parities (per byte) */ + /* rp0..rp15..rp17 are the various accumulated parities (per byte) */ uint32_t rp0, rp1, rp2, rp3, rp4, rp5, rp6, rp7; - uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15; + uint32_t rp8, rp9, rp10, rp11, rp12, rp13, rp14, rp15, rp16; + uint32_t uninitialized_var(rp17); /* to make compiler happy */ uint32_t par; /* the cumulative parity for all data */ uint32_t tmppar; /* the cumulative parity for this iteration; - for rp12 and rp14 at the end of the loop */ + for rp12, rp14 and rp16 at the end of the + loop */ par = 0; rp4 = 0; @@ -173,6 +181,7 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, rp10 = 0; rp12 = 0; rp14 = 0; + rp16 = 0; /* * The loop is unrolled a number of times; @@ -181,10 +190,10 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, * Note: passing unaligned data might give a performance penalty. * It is assumed that the buffers are aligned. * tmppar is the cumulative sum of this iteration. - * needed for calculating rp12, rp14 and par + * needed for calculating rp12, rp14, rp16 and par * also used as a performance improvement for rp6, rp8 and rp10 */ - for (i = 0; i < 4; i++) { + for (i = 0; i < eccsize_mult << 2; i++) { cur = *bp++; tmppar = cur; rp4 ^= cur; @@ -247,12 +256,14 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, rp12 ^= tmppar; if ((i & 0x2) == 0) rp14 ^= tmppar; + if (eccsize_mult == 2 && (i & 0x4) == 0) + rp16 ^= tmppar; } /* * handle the fact that we use longword operations - * we'll bring rp4..rp14 back to single byte entities by shifting and - * xoring first fold the upper and lower 16 bits, + * we'll bring rp4..rp14..rp16 back to single byte entities by + * shifting and xoring first fold the upper and lower 16 bits, * then the upper and lower 8 bits. */ rp4 ^= (rp4 >> 16); @@ -273,6 +284,11 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, rp14 ^= (rp14 >> 16); rp14 ^= (rp14 >> 8); rp14 &= 0xff; + if (eccsize_mult == 2) { + rp16 ^= (rp16 >> 16); + rp16 ^= (rp16 >> 8); + rp16 &= 0xff; + } /* * we also need to calculate the row parity for rp0..rp3 @@ -315,7 +331,7 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, par &= 0xff; /* - * and calculate rp5..rp15 + * and calculate rp5..rp15..rp17 * note that par = rp4 ^ rp5 and due to the commutative property * of the ^ operator we can say: * rp5 = (par ^ rp4); @@ -329,6 +345,8 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, rp11 = (par ^ rp10) & 0xff; rp13 = (par ^ rp12) & 0xff; rp15 = (par ^ rp14) & 0xff; + if (eccsize_mult == 2) + rp17 = (par ^ rp16) & 0xff; /* * Finally calculate the ecc bits. @@ -375,32 +393,46 @@ int nand_calculate_ecc(struct mtd_info *mtd, const unsigned char *buf, (invparity[rp9] << 1) | (invparity[rp8]); #endif - code[2] = - (invparity[par & 0xf0] << 7) | - (invparity[par & 0x0f] << 6) | - (invparity[par & 0xcc] << 5) | - (invparity[par & 0x33] << 4) | - (invparity[par & 0xaa] << 3) | - (invparity[par & 0x55] << 2) | - 3; + if (eccsize_mult == 1) + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + 3; + else + code[2] = + (invparity[par & 0xf0] << 7) | + (invparity[par & 0x0f] << 6) | + (invparity[par & 0xcc] << 5) | + (invparity[par & 0x33] << 4) | + (invparity[par & 0xaa] << 3) | + (invparity[par & 0x55] << 2) | + (invparity[rp17] << 1) | + (invparity[rp16] << 0); return 0; } EXPORT_SYMBOL(nand_calculate_ecc); /** * nand_correct_data - [NAND Interface] Detect and correct bit error(s) - * @mtd: MTD block structure (unused) + * @mtd: MTD block structure * @buf: raw data read from the chip * @read_ecc: ECC from the chip * @calc_ecc: the ECC calculated from raw data * - * Detect and correct a 1 bit error for 256 byte block + * Detect and correct a 1 bit error for 256/512 byte block */ int nand_correct_data(struct mtd_info *mtd, unsigned char *buf, unsigned char *read_ecc, unsigned char *calc_ecc) { unsigned char b0, b1, b2; unsigned char byte_addr, bit_addr; + /* 256 or 512 bytes/ecc */ + const uint32_t eccsize_mult = + (((struct nand_chip *)mtd->priv)->ecc.size) >> 8; /* * b0 to b2 indicate which bit is faulty (if any) @@ -426,10 +458,12 @@ int nand_correct_data(struct mtd_info *mtd, unsigned char *buf, if ((((b0 ^ (b0 >> 1)) & 0x55) == 0x55) && (((b1 ^ (b1 >> 1)) & 0x55) == 0x55) && - (((b2 ^ (b2 >> 1)) & 0x54) == 0x54)) { /* single bit error */ + ((eccsize_mult == 1 && ((b2 ^ (b2 >> 1)) & 0x54) == 0x54) || + (eccsize_mult == 2 && ((b2 ^ (b2 >> 1)) & 0x55) == 0x55))) { + /* single bit error */ /* - * rp15/13/11/9/7/5/3/1 indicate which byte is the faulty byte - * cp 5/3/1 indicate the faulty bit. + * rp17/rp15/13/11/9/7/5/3/1 indicate which byte is the faulty + * byte, cp 5/3/1 indicate the faulty bit. * A lookup table (called addressbits) is used to filter * the bits from the byte they are in. * A marginal optimisation is possible by having three @@ -443,7 +477,11 @@ int nand_correct_data(struct mtd_info *mtd, unsigned char *buf, * We could also do addressbits[b2] >> 1 but for the * performace it does not make any difference */ - byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + if (eccsize_mult == 1) + byte_addr = (addressbits[b1] << 4) + addressbits[b0]; + else + byte_addr = (addressbits[b2 & 0x3] << 8) + + (addressbits[b1] << 4) + addressbits[b0]; bit_addr = addressbits[b2 >> 2]; /* flip the bit */ buf[byte_addr] ^= (1 << bit_addr); ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-08-23 16:18 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <19F8576C6E063C45BE387C64729E739403CD517C16@dbde02.ent.ti.com>
2008-08-21 10:51 ` [PATCH] [MTD] [NAND] nand_ecc.c: adding support for 512 byte ecc Singh, Vimal
2008-08-21 10:55 ` Artem Bityutskiy
2008-08-21 18:39 ` Frans Meulenbroeks
2008-08-22 5:39 ` Singh, Vimal
2008-08-22 5:53 ` Artem Bityutskiy
2008-08-22 6:07 ` Artem Bityutskiy
2008-08-22 8:43 ` Frans Meulenbroeks
2008-08-22 9:06 ` David Woodhouse
2008-08-22 8:45 ` Singh, Vimal
2008-08-22 8:58 ` Frans Meulenbroeks
2008-08-22 9:27 ` Singh, Vimal
2008-08-23 16:18 ` frans
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox