| Message ID | 4BFDCEC3.7070601@elphinstone.net |
|---|---|
| State | Accepted |
| Commit | c9fb67735b307a3cdf57e568b6c50c860248d1d3 |
| Headers | show |
No comments? I know DOC2000 is pretty old now, but somebody must remember them! Regards, Mark Mark Ware wrote: > The variable 'syn' was being used uninitialized. Also > fixed incorrect use of syn[] vs s[]. > > Tested on powerpc board with 64MB DOC2000. > --- > > I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header > mismatches causing a failure to detect the DOC device partitions. Tracing > through, I saw this variable being used uninitialized and I suspect > incorrectly also. > > I do not really understand how the ecc/syndrome code works, so I do not > know if this patch is the correct solution, but it did make my problem > go away... > > CC: Thomas Gleixner as I believe he may have written this function initially. > > drivers/mtd/nand/diskonchip.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c > index a5bf9ff..7da2321 100644 > --- a/drivers/mtd/nand/diskonchip.c > +++ b/drivers/mtd/nand/diskonchip.c > @@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) > uint8_t parity; > uint16_t ds[4], s[5], tmp, errval[8], syn[4]; > > + memset(syn, 0, sizeof(syn)); > /* Convert the ecc bytes into words */ > ds[0] = ((ecc[4] & 0xff) >> 0) | ((ecc[5] & 0x03) << 8); > ds[1] = ((ecc[5] & 0xfc) >> 2) | ((ecc[2] & 0x0f) << 6); > @@ -168,9 +169,9 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) > s[i] ^= rs->alpha_to[rs_modnn(rs, tmp + (FCR + i) * j)]; > } > > - /* Calc s[i] = s[i] / alpha^(v + i) */ > + /* Calc syn[i] = s[i] / alpha^(v + i) */ > for (i = 0; i < NROOTS; i++) { > - if (syn[i]) > + if (s[i]) > syn[i] = rs_modnn(rs, rs->index_of[s[i]] + (NN - FCR - i)); > } > /* Call the decoder library */
On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote: > The variable 'syn' was being used uninitialized. Also > fixed incorrect use of syn[] vs s[]. > > Tested on powerpc board with 64MB DOC2000. > --- > > I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header > mismatches causing a failure to detect the DOC device partitions. Tracing > through, I saw this variable being used uninitialized and I suspect > incorrectly also. > > I do not really understand how the ecc/syndrome code works, so I do not > know if this patch is the correct solution, but it did make my problem > go away... > > CC: Thomas Gleixner as I believe he may have written this function initially. > > drivers/mtd/nand/diskonchip.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c > index a5bf9ff..7da2321 100644 > --- a/drivers/mtd/nand/diskonchip.c > +++ b/drivers/mtd/nand/diskonchip.c > @@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) > uint8_t parity; > uint16_t ds[4], s[5], tmp, errval[8], syn[4]; > > + memset(syn, 0, sizeof(syn)); I also do not know the math of this stuff, but this change is not needed ... > /* Convert the ecc bytes into words */ > ds[0] = ((ecc[4] & 0xff) >> 0) | ((ecc[5] & 0x03) << 8); > ds[1] = ((ecc[5] & 0xfc) >> 2) | ((ecc[2] & 0x0f) << 6); > @@ -168,9 +169,9 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) > s[i] ^= rs->alpha_to[rs_modnn(rs, tmp + (FCR + i) * j)]; > } > > - /* Calc s[i] = s[i] / alpha^(v + i) */ > + /* Calc syn[i] = s[i] / alpha^(v + i) */ > for (i = 0; i < NROOTS; i++) { > - if (syn[i]) > + if (s[i]) > syn[i] = rs_modnn(rs, rs->index_of[s[i]] + (NN - FCR - i)); > } ... after this change, right?
On Tue, 2010-06-29 at 06:46 +0300, Artem Bityutskiy wrote: > On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote: > > The variable 'syn' was being used uninitialized. Also > > fixed incorrect use of syn[] vs s[]. > > > > Tested on powerpc board with 64MB DOC2000. > > --- > > > > I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header > > mismatches causing a failure to detect the DOC device partitions. Tracing > > through, I saw this variable being used uninitialized and I suspect > > incorrectly also. > > > > I do not really understand how the ecc/syndrome code works, so I do not > > know if this patch is the correct solution, but it did make my problem > > go away... > > > > CC: Thomas Gleixner as I believe he may have written this function initially. > > > > drivers/mtd/nand/diskonchip.c | 5 +++-- > > 1 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c > > index a5bf9ff..7da2321 100644 > > --- a/drivers/mtd/nand/diskonchip.c > > +++ b/drivers/mtd/nand/diskonchip.c > > @@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) > > uint8_t parity; > > uint16_t ds[4], s[5], tmp, errval[8], syn[4]; > > > > + memset(syn, 0, sizeof(syn)); > > I also do not know the math of this stuff, but this change is not > needed ... Sorry, ignore this, I'll put this patch to my l2-mtd-2.6 tree and let dwmw2 decide whether it is good or not :-)
On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote: > The variable 'syn' was being used uninitialized. Also > fixed incorrect use of syn[] vs s[]. > > Tested on powerpc board with 64MB DOC2000. > --- > > I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header > mismatches causing a failure to detect the DOC device partitions. Tracing > through, I saw this variable being used uninitialized and I suspect > incorrectly also. > > I do not really understand how the ecc/syndrome code works, so I do not > know if this patch is the correct solution, but it did make my problem > go away... > > CC: Thomas Gleixner as I believe he may have written this function initially. Please, always add Signed-off-by for kernel patches. I added it for you.
On 29/06/10 16:29, Artem Bityutskiy wrote: > On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote: >> The variable 'syn' was being used uninitialized. Also >> fixed incorrect use of syn[] vs s[]. >> >> Tested on powerpc board with 64MB DOC2000. >> --- >> >> I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header >> mismatches causing a failure to detect the DOC device partitions. Tracing >> through, I saw this variable being used uninitialized and I suspect >> incorrectly also. >> >> I do not really understand how the ecc/syndrome code works, so I do not >> know if this patch is the correct solution, but it did make my problem >> go away... >> >> CC: Thomas Gleixner as I believe he may have written this function initially. > > Please, always add Signed-off-by for kernel patches. I added it for you. > Thanks. I guess I expected to be told that I was smoking something and that my DOC error disappearing was unrelated... Mark
On Tue, 2010-06-29 at 16:47 +1000, Mark Ware wrote: > On 29/06/10 16:29, Artem Bityutskiy wrote: > > On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote: > >> The variable 'syn' was being used uninitialized. Also > >> fixed incorrect use of syn[] vs s[]. > >> > >> Tested on powerpc board with 64MB DOC2000. > >> --- > >> > >> I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header > >> mismatches causing a failure to detect the DOC device partitions. Tracing > >> through, I saw this variable being used uninitialized and I suspect > >> incorrectly also. > >> > >> I do not really understand how the ecc/syndrome code works, so I do not > >> know if this patch is the correct solution, but it did make my problem > >> go away... > >> > >> CC: Thomas Gleixner as I believe he may have written this function initially. > > > > Please, always add Signed-off-by for kernel patches. I added it for you. > > > > Thanks. I guess I expected to be told that I was smoking something and that > my DOC error disappearing was unrelated... Note, I did not really review your patch - the MTD maintainer will take a look at it later, probably.
diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c index a5bf9ff..7da2321 100644 --- a/drivers/mtd/nand/diskonchip.c +++ b/drivers/mtd/nand/diskonchip.c @@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) uint8_t parity; uint16_t ds[4], s[5], tmp, errval[8], syn[4]; + memset(syn, 0, sizeof(syn)); /* Convert the ecc bytes into words */ ds[0] = ((ecc[4] & 0xff) >> 0) | ((ecc[5] & 0x03) << 8); ds[1] = ((ecc[5] & 0xfc) >> 2) | ((ecc[2] & 0x0f) << 6); @@ -168,9 +169,9 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) s[i] ^= rs->alpha_to[rs_modnn(rs, tmp + (FCR + i) * j)]; } - /* Calc s[i] = s[i] / alpha^(v + i) */ + /* Calc syn[i] = s[i] / alpha^(v + i) */ for (i = 0; i < NROOTS; i++) { - if (syn[i]) + if (s[i]) syn[i] = rs_modnn(rs, rs->index_of[s[i]] + (NN - FCR - i)); } /* Call the decoder library */