linux-kernel - Re: [PATCH] mtd: spi-nor: core: replace dummy buswidth from addr to data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <437e9c87-fbae-4402-858c-ce6de0a992c7@linaro.org>
Date: Mon, 11 Nov 2024 10:18:11 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Cheng Ming Lin <linchengming884@...il.com>, pratyush@...nel.org,
 mwalle@...nel.org, miquel.raynal@...tlin.com, richard@....at,
 vigneshr@...com, linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Cc: alvinzhou@...c.com.tw, leoyu@...c.com.tw,
 Cheng Ming Lin <chengminglin@...c.com.tw>
Subject: Re: [PATCH] mtd: spi-nor: core: replace dummy buswidth from addr to
 data



On 11/7/24 9:30 AM, Cheng Ming Lin wrote:
> From: Cheng Ming Lin <chengminglin@...c.com.tw>
> 
> The default dummy cycle for Macronix SPI NOR flash in Octal Output
> Read Mode(1-1-8) is 20.
> 
> Currently, the dummy buswidth is set according to the address bus width.
> In the 1-1-8 mode, this means the dummy buswidth is 1. When converting
> dummy cycles to bytes, this results in 20 x 1 / 8 = 2 bytes, causing the
> host to read data 4 cycles too early.
> 
> Since the protocol data buswidth is always greater than or equal to the
> address buswidth. Setting the dummy buswidth to match the data buswidth
> increases the likelihood that the dummy cycle-to-byte conversion will be
> divisible, preventing the host from reading data prematurely.

This is still very wrong and the `fix` is working just by chance.
Consider what happens when one requires 10 dummy cycles. BTW, does this
fix a real problem, or it's just a theoretical fix?

I don't like that we're chasing our tails here. The fix is to pass
directly cycles instead of bytes and update MTD and SPI to use cycles. I
had an attempt to fix this in the past, but I can't allocate time to
respin. Are you interested in taking over and fixing MTD and SPI?

> Signed-off-by: Cheng Ming Lin <chengminglin@...c.com.tw>
> ---
>  drivers/mtd/spi-nor/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f9c189ed7353..c7aceaa8a43f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -89,7 +89,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>  		op->addr.buswidth = spi_nor_get_protocol_addr_nbits(proto);
>  
>  	if (op->dummy.nbytes)
> -		op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
> +		op->dummy.buswidth = spi_nor_get_protocol_data_nbits(proto);
>  
>  	if (op->data.nbytes)
>  		op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);