Re: [PATCH v4] mtd: rawnand: cadence: support 64-bit slave dma interface

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

Hi Valentin,

vkorenblit@sequans.com wrote on Tue, 18 Oct 2022 11:16:31 +0200:

> 32-bit accesses on 64-bit sdma trigger sdma_err in intr_status register.
> 
> Check dma capabilities before reading/writing from/to sdma interface.
> 
> Link: https://secure-web.cisco.com/1Zuwps3Jm-Qybygq3UAIg4d4S5heggZX1ow8KENGLYXasfYvc72QPuxCC1kbg9_YPoqVYqfCQVlokpkras6hyjajt6RxKCmlzNhUsTu6vdTgufT8upiLvVnbScXuhDTKV539Fk3pwIluw0CMUbizt16dze_fAdyNKJxwNbEzZIq8DIpD8kK9qUCTdhHSEXza7Ytk_EmwCBawUQFBNNV2CzKBaw-vx5t7gbuvlT1IL9WV4bA3nzUs4NLiN1TV2lBuyA28Duk2i-aouwPb5o3UwDHhHxyCQuGos0F-sSxMREQr2Uj4lgR8lfIpFbk6O8o7J3K2i8rXA4TqUYcRD4BxWWNk3nBHVJxW2oVqX6sjS3Vh32VxOnsoXhttu_3UFm3qziOiukq8jXcvoFPvD6kwJutnujcsJzeGzEBSBfUxSHdgZYFyBbHwqqBC298e3uUV-/https%3A%2F%2Flists.01.org%2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org%2Fthread%2F3NMACGIM5NDUBPXRT5RTBZON6LQE5A3B%2F

What happened to the link? I guess there is some security software
interaction here? Should we take the one from your v3?

> 
> Signed-off-by: Valentin Korenblit <vkorenblit@sequans.com>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> ---
> 
> Changes v1 -> v2:
> 	- Replaced ioread64_rep by cadence_nand_readsq (suggested by Arnd)
> 	- Replaced iowrite64_rep by cadence_nand_writesq (suggested by Arnd)
> 	- Do not try to access 64-bit sdma if __raw_readq/__raw_writeq are not defined
> 
> Changes v2 -> v3:
> 	- Use readsq/writesq on 64-bit architectures (Arnd)
> 	- Detect issue on init instead of 1st transfer
> 
> Changes v3 -> v4:
> 	- Updated link and reviewed tag (Arnd)
> ---
>  .../mtd/nand/raw/cadence-nand-controller.c    | 70 +++++++++++++++----
>  1 file changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/cadence-nand-controller.c b/drivers/mtd/nand/raw/cadence-nand-controller.c
> index 9dac3ca69d57..7661a5cf1883 100644
> --- a/drivers/mtd/nand/raw/cadence-nand-controller.c
> +++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
> @@ -1184,6 +1184,14 @@ static int cadence_nand_hw_init(struct cdns_nand_ctrl *cdns_ctrl)
>  	if (cadence_nand_read_bch_caps(cdns_ctrl))
>  		return -EIO;
>  
> +#ifndef CONFIG_64BIT
> +	if (cdns_ctrl->caps2.data_dma_width == 8) {
> +		dev_err(cdns_ctrl->dev,
> +			"cannot access 64-bit dma on !64-bit architectures");
> +		return -EIO;
> +	}
> +#endif
> +
>  	/*
>  	 * Set IO width access to 8.
>  	 * It is because during SW device discovering width access
> @@ -1882,17 +1890,36 @@ static int cadence_nand_read_buf(struct cdns_nand_ctrl *cdns_ctrl,
>  		return status;
>  
>  	if (!cdns_ctrl->caps1->has_dma) {
> -		int len_in_words = len >> 2;
> +		u8 data_dma_width = cdns_ctrl->caps2.data_dma_width;
> +
> +		int len_in_words = (data_dma_width == 4) ? len >> 2 : len >> 3;
>  
>  		/* read alingment data */
> -		ioread32_rep(cdns_ctrl->io.virt, buf, len_in_words);
> +		if (data_dma_width == 4)
> +			ioread32_rep(cdns_ctrl->io.virt, buf, len_in_words);
> +#ifdef CONFIG_64BIT
> +		else
> +			readsq(cdns_ctrl->io.virt, buf, len_in_words);
> +#endif
> +
>  		if (sdma_size > len) {
> +			int read_bytes = (data_dma_width == 4) ?
> +				len_in_words << 2 : len_in_words << 3;
> +
>  			/* read rest data from slave DMA interface if any */
> -			ioread32_rep(cdns_ctrl->io.virt, cdns_ctrl->buf,
> -				     sdma_size / 4 - len_in_words);
> +			if (data_dma_width == 4)
> +				ioread32_rep(cdns_ctrl->io.virt,
> +					     cdns_ctrl->buf,
> +					     sdma_size / 4 - len_in_words);
> +#ifdef CONFIG_64BIT
> +			else
> +				readsq(cdns_ctrl->io.virt, cdns_ctrl->buf,
> +				       sdma_size / 8 - len_in_words);
> +#endif
> +
>  			/* copy rest of data */
> -			memcpy(buf + (len_in_words << 2), cdns_ctrl->buf,
> -			       len - (len_in_words << 2));
> +			memcpy(buf + read_bytes, cdns_ctrl->buf,
> +			       len - read_bytes);
>  		}
>  		return 0;
>  	}
> @@ -1936,16 +1963,35 @@ static int cadence_nand_write_buf(struct cdns_nand_ctrl *cdns_ctrl,
>  		return status;
>  
>  	if (!cdns_ctrl->caps1->has_dma) {
> -		int len_in_words = len >> 2;
> +		u8 data_dma_width = cdns_ctrl->caps2.data_dma_width;
> +
> +		int len_in_words = (data_dma_width == 4) ? len >> 2 : len >> 3;
> +
> +		if (data_dma_width == 4)
> +			iowrite32_rep(cdns_ctrl->io.virt, buf, len_in_words);
> +#ifdef CONFIG_64BIT
> +		else
> +			writesq(cdns_ctrl->io.virt, buf, len_in_words);
> +#endif
>  
> -		iowrite32_rep(cdns_ctrl->io.virt, buf, len_in_words);
>  		if (sdma_size > len) {
> +			int written_bytes = (data_dma_width == 4) ?
> +				len_in_words << 2 : len_in_words << 3;
> +
>  			/* copy rest of data */
> -			memcpy(cdns_ctrl->buf, buf + (len_in_words << 2),
> -			       len - (len_in_words << 2));
> +			memcpy(cdns_ctrl->buf, buf + written_bytes,
> +			       len - written_bytes);
> +
>  			/* write all expected by nand controller data */
> -			iowrite32_rep(cdns_ctrl->io.virt, cdns_ctrl->buf,
> -				      sdma_size / 4 - len_in_words);
> +			if (data_dma_width == 4)
> +				iowrite32_rep(cdns_ctrl->io.virt,
> +					      cdns_ctrl->buf,
> +					      sdma_size / 4 - len_in_words);
> +#ifdef CONFIG_64BIT
> +			else
> +				writesq(cdns_ctrl->io.virt, cdns_ctrl->buf,
> +					sdma_size / 8 - len_in_words);
> +#endif
>  		}
>  
>  		return 0;


Thanks,
Miquèl

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