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

Tudor Ambarus
2025-01-15T10:29:29Z

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@ew.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@linaro.org>