Hi Sascha, On Fri, Oct 21, 2016 at 09:18:08AM +0200, Sascha Hauer wrote: > On Thu, Oct 20, 2016 at 03:40:07PM +0200, Christian Hemp wrote: > > The calculation of readtotal must be bit alligend. If not the bch core > > finds bit flips in every page, because readtotal is too small. > > This bug was mostly introduced since commit "51061a9 mtd: nand: nand_mxs: > > Add subpage read support". > > Is this somehow related to: > > http://lists.infradead.org/pipermail/linux-mtd/2016-June/068243.html > > Or is this another topic? > > Sascha Yes, this patch fixes the same wrong assumption as in the patch above. In the above patch on the kernel list the assumption is that every block of chunk+eccbits is byte aligned on the flash. So if the eccbits are not divisible by 8, there are unused bits in the last byte of every block of chunk+eccbits. That's not the case. In fact the second, third,... chunk will not be byte aligned on the flash anymore. The blocks are bit shifted. AFAIR, the GPMI/ECC engine operates at the bit level (which is a pain to deal with BTW), and is only requiring a byte alignment on the total number of ECC bits. So here, DIV_ROUND_UP(18 * 13 * 4, 8) = 117, which fits in the 118 bytes (128 bytes - 10 bytes of 'metadata'). [Source: http://lists.infradead.org/pipermail/linux-mtd/2016-June/068244.html] In the patch below the line > > - ecc_parity_size = 13 * eccstrength / 8; is the culprit. If the eccbits per chunk are not divisible by 8, a rounding error occurs. Since C integer division truncates the fractional part, the byte size in 'ecc_parity_size' underestimates the real size of the eccbits on the flash. The correct solution is to calculate all sizes in bits and then round up to the next byte. So the code only overestimates the required read size. I can only agree to Boris statement: AFAIR, the GPMI/ECC engine operates at the bit level (which is a pain to deal with BTW), Dealing with bit aligned stuff is really awful. Mit freundlichen Grüßen / Kind regards, Stefan Lengfeld > > > > > Tested with: > > nand: NAND device: Manufacturer ID: 0x01, Chip ID: 0xd3 (AMD/Spansion > > S34ML08G2), 1024MiB, page size: 2048, OOB size: 128 > > > > nand: NAND device: Manufacturer ID: 0x2c, Chip ID: 0xdc (Micron > > MT29F4G08ABADAWP), 512MiB, page size: 2048, OOB size: 64 > > > > nand: NAND device: Manufacturer ID: 0xec, Chip ID: 0xd3 (Samsung NAND > > 1GiB 3,3V 8-bit), 1024MiB, page size: 2048, OOB size: 64 > > > > Signed-off-by: Christian Hemp <c.hemp@xxxxxxxxx> > > Signed-off-by: Stefan Lengfeld <s.lengfeld@xxxxxxxxx> > > --- > > drivers/mtd/nand/nand_mxs.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/mtd/nand/nand_mxs.c b/drivers/mtd/nand/nand_mxs.c > > index fe955e8..cba0bee 100644 > > --- a/drivers/mtd/nand/nand_mxs.c > > +++ b/drivers/mtd/nand/nand_mxs.c > > @@ -728,15 +728,15 @@ static int __mxs_nand_ecc_read_page(struct mtd_info *mtd, struct nand_chip *nand > > uint32_t corrected = 0, failed = 0; > > uint8_t *status; > > unsigned int max_bitflips = 0; > > - int i, ret, readtotal, nchunks, eccstrength, ecc_parity_size; > > + int i, ret, readtotal, nchunks, eccstrength; > > > > eccstrength = mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize); > > > > readlen = roundup(readlen, MXS_NAND_CHUNK_DATA_CHUNK_SIZE); > > nchunks = mxs_nand_ecc_chunk_cnt(readlen); > > - ecc_parity_size = 13 * eccstrength / 8; > > - readtotal = MXS_NAND_METADATA_SIZE + > > - (MXS_NAND_CHUNK_DATA_CHUNK_SIZE + ecc_parity_size) * nchunks; > > + readtotal = MXS_NAND_METADATA_SIZE; > > + readtotal += MXS_NAND_CHUNK_DATA_CHUNK_SIZE * nchunks; > > + readtotal += DIV_ROUND_UP(13 * eccstrength * nchunks, 8); > > > > mxs_nand_config_bch(mtd, readtotal); > > > > -- > > 1.9.1 > > > > > > _______________________________________________ > > barebox mailing list > > barebox@xxxxxxxxxxxxxxxxxxx > > http://lists.infradead.org/mailman/listinfo/barebox > > > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > _______________________________________________ > barebox mailing list > barebox@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/barebox _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
