diff mbox series

[v2,13/19] mtd: rawnand: cafe: Don't split things when reading/writing a page

Message ID 20200505101353.1776394-14-boris.brezillon@collabora.com
State Changes Requested
Delegated to: Miquel Raynal
Headers show
Series mtd: rawnand: cafe: Convert to exec_op() (and more) | expand

Commit Message

Boris Brezillon May 5, 2020, 10:13 a.m. UTC 
Calling nand_read_page_op(pagesize)/nand_prog_page_begin_op(pagesize)
and expecting to get a pagesize+oobsize read from/written to the
read/write buffer is fragile and only works because of hacks done
in cmdfunc(). Let's read/write the page in one go, using the page
cache buffer as a bounce buffer instead of relying on guesses done
in cafe_nand_cmdfunc().

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Tested-by: Lubomir Rintel <lkundrak@v3.sk>
---
Changes in v2:
* Add T-b
* None
---
 drivers/mtd/nand/raw/cafe_nand.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Miquel Raynal May 10, 2020, 9:49 p.m. UTC | #1 
Boris Brezillon <boris.brezillon@collabora.com> wrote on Tue,  5 May
2020 12:13:47 +0200:

> Calling nand_read_page_op(pagesize)/nand_prog_page_begin_op(pagesize)
> and expecting to get a pagesize+oobsize read from/written to the
> read/write buffer is fragile and only works because of hacks done
> in cmdfunc(). Let's read/write the page in one go, using the page
> cache buffer as a bounce buffer instead of relying on guesses done
> in cafe_nand_cmdfunc().
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Tested-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> Changes in v2:
> * Add T-b
> * None
> ---
>  drivers/mtd/nand/raw/cafe_nand.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
> index f7e537836d55..36685bee9d83 100644
> --- a/drivers/mtd/nand/raw/cafe_nand.c
> +++ b/drivers/mtd/nand/raw/cafe_nand.c
> @@ -470,6 +470,7 @@ static int cafe_nand_read_page(struct nand_chip *chip, uint8_t *buf,
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct cafe_priv *cafe = nand_get_controller_data(chip);
> +	void *pagebuf = nand_get_data_buf(chip);
>  	unsigned int max_bitflips = 0;
>  	u32 ecc_result, status;
>  
> @@ -477,8 +478,11 @@ static int cafe_nand_read_page(struct nand_chip *chip, uint8_t *buf,
>  		cafe_readl(cafe, NAND_ECC_RESULT),
>  		cafe_readl(cafe, NAND_ECC_SYN_REG(0)));
>  
> -	nand_read_page_op(chip, page, 0, buf, mtd->writesize);
> -	chip->legacy.read_buf(chip, chip->oob_poi, mtd->oobsize);
> +	nand_read_page_op(chip, page, 0, pagebuf,
> +			  mtd->writesize + mtd->oobsize);

Shouldn't accesses to the OOB area be under conditions like if
(oob_required)? Unless of course reading the OOB like this is mandatory
in order to get the ECC engine working - I did not check.
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
index f7e537836d55..36685bee9d83 100644
--- a/drivers/mtd/nand/raw/cafe_nand.c
+++ b/drivers/mtd/nand/raw/cafe_nand.c
@@ -470,6 +470,7 @@  static int cafe_nand_read_page(struct nand_chip *chip, uint8_t *buf,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct cafe_priv *cafe = nand_get_controller_data(chip);
+	void *pagebuf = nand_get_data_buf(chip);
 	unsigned int max_bitflips = 0;
 	u32 ecc_result, status;
 
@@ -477,8 +478,11 @@  static int cafe_nand_read_page(struct nand_chip *chip, uint8_t *buf,
 		cafe_readl(cafe, NAND_ECC_RESULT),
 		cafe_readl(cafe, NAND_ECC_SYN_REG(0)));
 
-	nand_read_page_op(chip, page, 0, buf, mtd->writesize);
-	chip->legacy.read_buf(chip, chip->oob_poi, mtd->oobsize);
+	nand_read_page_op(chip, page, 0, pagebuf,
+			  mtd->writesize + mtd->oobsize);
+
+	if (buf != pagebuf)
+		memcpy(buf, pagebuf, mtd->writesize);
 
 	ecc_result = cafe_readl(cafe, NAND_ECC_RESULT);
 	status = CAFE_FIELD_GET(NAND_ECC_RESULT, STATUS, ecc_result);
@@ -640,15 +644,17 @@  static int cafe_nand_write_page(struct nand_chip *chip,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct cafe_priv *cafe = nand_get_controller_data(chip);
+	void *pagebuf = nand_get_data_buf(chip);
 	int ret;
 
-	nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize);
-	chip->legacy.write_buf(chip, chip->oob_poi, mtd->oobsize);
+	if (pagebuf != buf)
+		memcpy(pagebuf, buf, mtd->writesize);
 
 	/* Set up ECC autogeneration */
 	cafe->ctl2 |= CAFE_NAND_CTRL2_AUTO_WRITE_ECC;
 
-	ret = nand_prog_page_end_op(chip);
+	ret = nand_prog_page_op(chip, page, 0, pagebuf,
+				mtd->writesize + mtd->oobsize);
 
 	/*
 	 * And clear it before returning so that following write operations