On Tue, Jan 14 2025, Alexander Stein wrote:
Hi Tudor,
Am Dienstag, 14. Januar 2025, 14:26:47 CET schrieb Tudor Ambarus:
On 1/14/25 12:57 PM, Alexander Stein wrote:
Hello everyone,
Hi,
Am Dienstag, 12. November 2024, 08:52:42 CET schrieb Cheng Ming Lin:
From: Cheng Ming Lin chengminglin@mxic.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@vger.kernel.org Reviewd-by: Pratyush Yadav pratyush@kernel.org Signed-off-by: Cheng Ming Lin chengminglin@mxic.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?
My guess is that 1S-1S-4S is stripped out in spi_nor_spimem_adjust_hwcaps(). Maybe the controller has some limitation in nxp_fspi_supports_op(). Would you add some prints, and check these chunks of code?
Thanks for the fast response. I was able to track it down. Eventually the buswidth check in spi_check_buswidth_req fails. For command 0x3c: Before revert:
mode: 0x800, buswidth: 2
After revert
mode: 0x800, buswidth: 1
The mode is set to SPI_RX_QUAD. Thus the check for dummy buswidth fails now that data_nbits are used now.
For command 0x6c it's similar but op->dummy.buswidth is 4 now.
It boils down that there are SPI controllers which have
spi-tx-bus-width = <1>; spi-rx-bus-width = <4>;
set in their DT nodes.
So it seems this combination is not supported.
No, this check is wrong. See https://lore.kernel.org/linux-mtd/20241112075242.174010-1-linchengming884@gm...