[PATCH 1/2] mtd: nand: mxs: add helper to calculate the page size

[PATCH 1/2] mtd: nand: mxs: add helper to calculate the page size

[Marco Felsch]

Since commit 51061a9884a3 ("mtd: nand: nand_mxs: Add subpage read
support") the ecc_read_page and ecc_write_page page_size can be differ.
Because during ecc_read_page the page_size() is based on the ecc
calculation. Unlike to the ecc_write_page() which uses the nand
writesize and oobsize information.

This commit factor out the page size calculation during ecc_read_page to
a helper function. So we can reuse it during ecc_write_page().

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/mtd/nand/nand_mxs.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/nand_mxs.c b/drivers/mtd/nand/nand_mxs.c
index c3b07aa3f6..762c751fa7 100644
--- a/drivers/mtd/nand/nand_mxs.c
+++ b/drivers/mtd/nand/nand_mxs.c
@@ -688,6 +688,25 @@ static uint8_t mxs_nand_read_byte(struct mtd_info *mtd)
 	return buf;
 }
 
+/*
+ * Determine the page size including user data + ecc data
+ */
+static unsigned int mxs_nand_get_page_size(struct mtd_info *mtd,
+					   unsigned int bytes)
+{
+	unsigned int eccstrength, nchunks, pagesize;
+
+	eccstrength = mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize);
+
+	bytes = roundup(bytes, MXS_NAND_CHUNK_DATA_CHUNK_SIZE);
+	nchunks = mxs_nand_ecc_chunk_cnt(bytes);
+	pagesize =  MXS_NAND_METADATA_SIZE;
+	pagesize += MXS_NAND_CHUNK_DATA_CHUNK_SIZE * nchunks;
+	pagesize += DIV_ROUND_UP(13 * eccstrength * nchunks, 8);
+
+	return pagesize;
+}
+
 static void mxs_nand_config_bch(struct mtd_info *mtd, int readlen)
 {
 	struct nand_chip *nand = mtd->priv;
@@ -728,15 +747,11 @@ 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;
-
-	eccstrength = mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize);
+	int i, ret, readtotal, nchunks;
 
 	readlen = roundup(readlen, MXS_NAND_CHUNK_DATA_CHUNK_SIZE);
 	nchunks = mxs_nand_ecc_chunk_cnt(readlen);
-	readtotal =  MXS_NAND_METADATA_SIZE;
-	readtotal += MXS_NAND_CHUNK_DATA_CHUNK_SIZE * nchunks;
-	readtotal += DIV_ROUND_UP(13 * eccstrength * nchunks, 8);
+	readtotal = mxs_nand_get_page_size(mtd, readlen);
 
 	mxs_nand_config_bch(mtd, readtotal);
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] mtd: nand: mxs: sync page_size information

[Marco Felsch]

Since commit 51061a9884a3 ("mtd: nand: nand_mxs: Add subpage read
support") the ecc_read_page and ecc_write_page page_size can be differ
if the calculation is not writesize+oobsize. So during write more bytes
are passed trough the bch than during the read operation.

It seems that there are no problems for typical nand flashes using the
"normal" layout e.g. 2048+64 (user+oob) but if it gets a bit atypical
e.g. 2048+128 (user+oob) this leads into ecc errors.

Anyway the linux driver uses the 'struct bch_geometry' to share all
required nand information during read/write/bch_setup. So we should do
it here too by passing the same amount of bytes to the bch during a
write and read operation. Also we have to fix the flashlayout setup to
set the correct page size.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/mtd/nand/nand_mxs.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nand_mxs.c b/drivers/mtd/nand/nand_mxs.c
index 762c751fa7..fd4b784f7e 100644
--- a/drivers/mtd/nand/nand_mxs.c
+++ b/drivers/mtd/nand/nand_mxs.c
@@ -747,7 +747,7 @@ 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;
+	int i, ret, readtotal, nchunks, pagesize;
 
 	readlen = roundup(readlen, MXS_NAND_CHUNK_DATA_CHUNK_SIZE);
 	nchunks = mxs_nand_ecc_chunk_cnt(readlen);
@@ -922,7 +922,8 @@ static int __mxs_nand_ecc_read_page(struct mtd_info *mtd, struct nand_chip *nand
 rtn:
 	mxs_nand_return_dma_descs(nand_info);
 
-	mxs_nand_config_bch(mtd, mtd->writesize + mtd->oobsize);
+	pagesize = mxs_nand_get_page_size(mtd, mtd->writesize);
+	mxs_nand_config_bch(mtd, pagesize);
 
 	return ret ? ret : max_bitflips;
 }
@@ -960,11 +961,14 @@ static int mxs_nand_ecc_write_page(struct mtd_info *mtd,
 	struct mxs_nand_info *nand_info = nand->priv;
 	struct mxs_dma_desc *d;
 	uint32_t channel = nand_info->dma_channel_base + nand_info->cur_chip;
+	unsigned int writelen;
 	int ret = 0;
 
 	memcpy(nand_info->data_buf, buf, mtd->writesize);
 	memcpy(nand_info->oob_buf, nand->oob_poi, mtd->oobsize);
 
+	writelen = mxs_nand_get_page_size(mtd, mtd->writesize);
+
 	/* Handle block mark swapping. */
 	mxs_nand_swap_block_mark(mtd, nand_info->data_buf, nand_info->oob_buf);
 
@@ -987,7 +991,7 @@ static int mxs_nand_ecc_write_page(struct mtd_info *mtd,
 		GPMI_ECCCTRL_ENABLE_ECC |
 		GPMI_ECCCTRL_ECC_CMD_ENCODE |
 		GPMI_ECCCTRL_BUFFER_MASK_BCH_PAGE;
-	d->cmd.pio_words[3] = (mtd->writesize + mtd->oobsize);
+	d->cmd.pio_words[3] = writelen;
 	d->cmd.pio_words[4] = (dma_addr_t)nand_info->data_buf;
 	d->cmd.pio_words[5] = (dma_addr_t)nand_info->oob_buf;
 
@@ -1235,6 +1239,7 @@ static int mxs_nand_scan_bbt(struct mtd_info *mtd)
 	struct nand_chip *nand = mtd->priv;
 	struct mxs_nand_info *nand_info = nand->priv;
 	void __iomem *bch_regs = nand_info->bch_base;
+	unsigned int pagesize;
 	int ret;
 
 	/* Reset BCH. Don't use SFTRST on MX23 due to Errata #2847 */
@@ -1243,7 +1248,8 @@ static int mxs_nand_scan_bbt(struct mtd_info *mtd)
 	if (ret)
 		return ret;
 
-	mxs_nand_config_bch(mtd, mtd->writesize + mtd->oobsize);
+	pagesize = mxs_nand_get_page_size(mtd, mtd->writesize);
+	mxs_nand_config_bch(mtd, pagesize);
 
 	/* Set *all* chip selects to use layout 0 */
 	writel(0, bch_regs + BCH_LAYOUTSELECT);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand: mxs: sync page_size information

[Sascha Hauer]

Hi Marco,

On Fri, Jul 26, 2019 at 11:34:00AM +0200, Marco Felsch wrote:
> Since commit 51061a9884a3 ("mtd: nand: nand_mxs: Add subpage read
> support") the ecc_read_page and ecc_write_page page_size can be differ
> if the calculation is not writesize+oobsize. So during write more bytes
> are passed trough the bch than during the read operation.
> 
> It seems that there are no problems for typical nand flashes using the
> "normal" layout e.g. 2048+64 (user+oob) but if it gets a bit atypical
> e.g. 2048+128 (user+oob) this leads into ecc errors.

What problems did you see exactly? I am using boards here with 2k/128b
page size and they seem to work properly, also when interacting with Linux.

Sascha

-- 
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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand: mxs: sync page_size information

[Marco Felsch]

Hi Sascha,

On 19-08-05 12:17, Sascha Hauer wrote:
> Hi Marco,
> 
> On Fri, Jul 26, 2019 at 11:34:00AM +0200, Marco Felsch wrote:
> > Since commit 51061a9884a3 ("mtd: nand: nand_mxs: Add subpage read
> > support") the ecc_read_page and ecc_write_page page_size can be differ
> > if the calculation is not writesize+oobsize. So during write more bytes
> > are passed trough the bch than during the read operation.
> > 
> > It seems that there are no problems for typical nand flashes using the
> > "normal" layout e.g. 2048+64 (user+oob) but if it gets a bit atypical
> > e.g. 2048+128 (user+oob) this leads into ecc errors.
> 
> What problems did you see exactly? I am using boards here with 2k/128b
> page size and they seem to work properly, also when interacting with Linux.

You are right, the problem was caused by a kernel bug.. which is fixed
now.

Anyway IMHO it seems to be right to write as many bytes as are read. So
do you will accept the patches if I adapt the commit message(s).

Regards,
  Marco

> 
> Sascha
> 
> -- 
> 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 |
> 

-- 
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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox