Re: [PATCH 16/17] mtd: rawnand: cafe: Handle non-32bit aligned reads/writes

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hi Boris,

Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Mon, 27 Apr
2020 10:20:26 +0200:

> The spec says the write/read buffers should be filled/read 32bits at a
> time. While most of the time the reads/writes are aligned on 4 bytes,
> we should make the implementation more robust to non-usual NAND
> operations.

Well, if I didn't get confused by the previous changes, I think these
are used by ->exec_op() and at boot time there are 2B (ID), 3B (I
don't remember) and 5B (full ID) accesses that are performed. These are
not 32-bit aligned.

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> ---
>  drivers/mtd/nand/raw/cafe_nand.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
> index 26495085f285..955dc78c57be 100644
> --- a/drivers/mtd/nand/raw/cafe_nand.c
> +++ b/drivers/mtd/nand/raw/cafe_nand.c
> @@ -184,10 +184,18 @@ static void cafe_write_buf(struct nand_chip *chip, const void *buf,
>  {
>  	struct cafe_priv *cafe = nand_get_controller_data(chip);
>  
> -	if (cafe->usedma)
> +	if (cafe->usedma) {

Are you sure usedma will not be triggered in the case of unaligned
accesses? I didn't check but it could be nice to verify that this flag
is only set by read/write_page() helpers for instance (which only
handle 32-bit aligned data).

>  		memcpy(cafe->dmabuf, buf, len);
> -	else
> -		memcpy_toio(cafe->mmio + CAFE_NAND_WRITE_DATA, buf, len);
> +	} else {
> +		memcpy_toio(cafe->mmio + CAFE_NAND_WRITE_DATA, buf,
> +			    len & ~0x3);
> +		if (len & 0x3) {
> +			u32 tmp = 0;
> +
> +			memcpy(&tmp, buf + (len & ~0x3), len & 0x3);
> +			cafe_writel(cafe, tmp, NAND_WRITE_DATA);
> +		}
> +	}
>  
>  	dev_dbg(&cafe->pdev->dev, "Copy 0x%x bytes to write buffer.\n",	len);
>  }
> @@ -196,10 +204,17 @@ static void cafe_read_buf(struct nand_chip *chip, void *buf, unsigned int len)
>  {
>  	struct cafe_priv *cafe = nand_get_controller_data(chip);
>  
> -	if (cafe->usedma)
> +	if (cafe->usedma) {
>  		memcpy(buf, cafe->dmabuf, len);
> -	else
> -		memcpy_fromio(buf, cafe->mmio + CAFE_NAND_READ_DATA, len);
> +	} else {
> +		memcpy_fromio(buf, cafe->mmio + CAFE_NAND_READ_DATA,
> +			      len & ~0x3);
> +		if (len & 0x3) {
> +			u32 tmp = cafe_readl(cafe, NAND_READ_DATA);
> +
> +			memcpy(buf + (len & ~0x3), &tmp, len & 0x3);
> +		}
> +	}
>  
>  	dev_dbg(&cafe->pdev->dev, "Copy 0x%x bytes from read buffer.\n", len);
>  }

Reviewed-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>

Thanks,
Miquèl

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