linux-kernel - Re: [PATCH] Revert "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: <878f66c3-83a4-4f83-88d8-ea2917d82d76@linaro.org>
Date: Wed, 15 Jan 2025 10:29:25 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Pratyush Yadav <pratyush@...nel.org>, Michael Walle <mwalle@...nel.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>,
 Cheng Ming Lin <linchengming884@...il.com>,
 Alexander Stein <alexander.stein@...tq-group.com>
Cc: linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org,
 alvinzhou@...c.com.tw, leoyu@...c.com.tw,
 Cheng Ming Lin <chengminglin@...c.com.tw>
Subject: Re: [PATCH] Revert "mtd: spi-nor: core: replace dummy buswidth from
 addr to data"



On 1/15/25 10:16 AM, Pratyush Yadav wrote:
> This reverts commit 98d1fb94ce75f39febd456d6d3cbbe58b6678795. The commit

"The commit" on a new paragraph please

> uses data nbits instead of addr nbits for dummy phase. This causes a
> regression for all boards where spi-tx-bus-width is smaller than
> spi-rx-bus-width. It is a common pattern for boards to have
> spi-tx-bus-width == 1 and spi-rx-bus-width > 1. The regression causes
> all reads with a dummy phase to become unavailable for such boards,
> leading to a usually slower 0-dummy-cycle read being selected.
> 
> Most controllers' supports_op hooks call spi_mem_default_supports_op().
> In spi_mem_default_supports_op(), spi_mem_check_buswidth() is called to
> check if the buswidths for the op can actually be supported by the
> board's wiring. This wiring information comes from (among other things)
> the spi-{tx,rx}-bus-width DT properties. Based on these properties,
> SPI_TX_* or SPI_RX_* flags are set by of_spi_parse_dt().
> spi_mem_check_buswidth() then uses these flags to make the decision
> whether an op can be supported by the board's wiring (in a way,
> indirectly checking against spi-{rx,tx}-bus-width).
> 
> Now the tricky bit here is that spi_mem_check_buswidth() does:
> 
> 	if (op->dummy.nbytes &&
> 	    spi_check_buswidth_req(mem, op->dummy.buswidth, true))
> 		return false;
> 
> The true argument to spi_check_buswidth_req() means the op is treated as
> a TX op. For a board that has say 1-bit TX and 4-bit RX, a 4-bit dummy
> TX is considered as unsupported, and the op gets rejected.
> 
> The commit being reverted uses the data buswidth for dummy buswidth. So
> for reads, the RX buswidth gets used for the dummy phase, uncovering
> this issue. In reality, a dummy phase is neither RX nor TX. As the name
> suggests, these are just dummy cycles that send or receive no data, and
> thus don't really need to have any buswidth at all.
> 
> Ideally, dummy phases should not be checked against the board's wiring
> capabilities at all, and should only be sanity-checked for having a sane
> buswidth value. Since we are now at rc7 and such a change might
> introduce many unexpected bugs, revert the commit for now. It can be
> sent out later along with the spi_mem_check_buswidth() fix.
> 
> Reported-by: Alexander Stein <alexander.stein@...tq-group.com>
> Closes: https://lore.kernel.org/linux-mtd/3342163.44csPzL39Z@steina-w/

fixes tag please

with that:
Reviewed-by: Tudor Ambarus <tudor.ambarus@...aro.org>