linux-kernel - Re: [PATCH v2 1/1] 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: <3342163.44csPzL39Z@steina-w>
Date: Tue, 14 Jan 2025 13:57:59 +0100
From: Alexander Stein <alexander.stein@...tq-group.com>
To: tudor.ambarus@...aro.org, 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>, stable@...r.kernel.org, Cheng Ming Lin <linchengming884@...il.com>
Subject: Re: [PATCH v2 1/1] mtd: spi-nor: core: replace dummy buswidth from addr to data

Hello everyone,

Am Dienstag, 12. November 2024, 08:52:42 CET schrieb Cheng Ming Lin:
> 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.
> 
> Fixes: 0e30f47232ab5 ("mtd: spi-nor: add support for DTR protocol")
> Cc: stable@...r.kernel.org
> Reviewd-by: Pratyush Yadav <pratyush@...nel.org>
> 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);
> 

I just noticed this commit caused a regression on my i.MX8M Plus based board,
detected using git bisect.
DT: arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts
Starting with this patch read is only 1S-1S-1S, before it was
1S-1S-4S.

before:
> cat /sys/kernel/debug/spi-nor/spi0.0/params
> name            mt25qu512a
> id              20 bb 20 10 44 00
> size            64.0 MiB
> write size      1
> page size       256
> address nbytes  4
> flags           HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_4BIT_BP
> | HAS_SR_BP3_BIT6 | SOFT_RESET
> 
> opcodes
> 
>  read           0x6c
>  
>   dummy cycles  8
>  
>  erase          0xdc
>  program        0x12
>  8D extension   none
> 
> protocols
> 
>  read           1S-1S-4S
>  write          1S-1S-1S
>  register       1S-1S-1S
> 
> erase commands
> 
>  21 (4.00 KiB) [1]
>  dc (64.0 KiB) [3]
>  c7 (64.0 MiB)
> 
> sector map
> 
>  region (in hex)   | erase mask | overlaid
>  ------------------+------------+----------
>  00000000-03ffffff |     [   3] | no

after:
> cat /sys/kernel/debug/spi-nor/spi0.0/params
> name            mt25qu512a
> id              20 bb 20 10 44 00
> size            64.0 MiB
> write size      1
> page size       256
> address nbytes  4
> flags           HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_4BIT_BP
> | HAS_SR_BP3_BIT6 | SOFT_RESET
> 
> opcodes
> 
>  read           0x13
>  
>   dummy cycles  0
>  
>  erase          0xdc
>  program        0x12
>  8D extension   none
> 
> protocols
> 
>  read           1S-1S-1S
>  write          1S-1S-1S
>  register       1S-1S-1S
> 
> erase commands
> 
>  21 (4.00 KiB) [1]
>  dc (64.0 KiB) [3]
>  c7 (64.0 MiB)
> 
> sector map
> 
>  region (in hex)   | erase mask | overlaid
>  ------------------+------------+----------
>  00000000-03ffffff |     [   3] | no

AFAICT the patch seems sane, so it probably just uncovered another
problem already lurking somewhere deeper.
Given the HW similarity I expect imx8mn and imx8mm based platforms to be
affected as well.
Reverting this commit make the read to be 1S-1S-4S again.
Any ideas ow to tackling down this problem?

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/