Re: [PATCH 12/17] mtd: rawnand: cafe: Don't split things when reading/writing a page

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 27 Apr
2020 10:20:22 +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.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  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 31493a201a02..edf65197604b 100644
> --- a/drivers/mtd/nand/raw/cafe_nand.c
> +++ b/drivers/mtd/nand/raw/cafe_nand.c
> @@ -472,6 +472,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;
>  
> @@ -479,8 +480,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);
> @@ -642,15 +646,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


You are replacing ->read/write_buf() calls into memcpy() calls,
shouldn't this be cleaned before? or at least mentioned?

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/