[PATCH 0/2] spi: spi-mem: odd byte condition changes in dtr_supports_op
From: Dhruva Gole d-gole@ti.com
Currently if one tries to read an odd filesize from a flash in OSPI DTR Mode then the transaction just straightaway returns failure without even attempting a read.
Base on comments from a series a while back: https://lore.kernel.org/u-boot/20221025062036.383460-1-d-gole@ti.com/
where Pratyush highlights the risk of on writes an extra byte goes to the flash. Hence this series makes sure that we perform this ODD Byte check only if the transactions are "non-read" related ie. only write related.
An alternative solution was suggested here: https://patchwork.ozlabs.org/project/linux-mtd/patch/20210531181757.19458-6-...
However the code is in linux kernel where it's a full fledge OS running with presumambly enough DDR RAM and we dont really have memory contraints as tight as in a bootloader. The part where the above solution does
... tmp_buf = kmalloc(PAGE_SIZE, GFP_KERNEL); ...
This is something we do not have the luxury of doing at bootloader stage where the RAM is very limited. In TI K3 AM625 SOC's case we have the same code running on R5 SPL Stage where memory is extremely limited and we can't really afford to allocate some at runtime as and when we want.
These patches have been tested on AM625 SK EVM platform and check for odd and even byte reads, updates and also full bootup from OSPI Flash till U-Boot prompt:
... U-Boot SPL 2023.04-rc2-00040-gbe9fd01af6 (Mar 01 2023 - 11:30:52 +0530) SYSFW ABI: 3.1 (firmware rev 0x0008 '8.5.3--v08.05.03 (Chill Capybar') Trying to boot from SPI Starting ATF on ARM64 core...
NOTICE: BL31: v2.7(release):v2.7.0-359-g1309c6c805-dirty NOTICE: BL31: Built : 11:48:12, Dec 14 2022 I/TC: I/TC: OP-TEE version: 3.19.0-15-gd6c5d0037 (gcc version 9.2.1 20191025 (GNU Toolchain for the A-profile Architecture 9.2-2019.12 (arm-9.10))) #1 Wed Dec 14 11:52:03 UTC 2022 aarch64 I/TC: WARNING: This OP-TEE configuration might be insecure! I/TC: WARNING: Please check https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html I/TC: Primary CPU initializing I/TC: SYSFW ABI: 3.1 (firmware rev 0x0008 '8.5.3--v08.05.03 (Chill Capybar') I/TC: HUK Initialized I/TC: Activated SA2UL device I/TC: Fixing SA2UL firewall owner for GP device I/TC: Enabled firewalls for SA2UL TRNG device I/TC: SA2UL TRNG initialized I/TC: SA2UL Drivers initialized I/TC: Primary CPU switching to normal world boot
U-Boot SPL 2023.04-rc2-00040-gbe9fd01af6 (Mar 01 2023 - 11:31:30 +0530) SYSFW ABI: 3.1 (firmware rev 0x0008 '8.5.3--v08.05.03 (Chill Capybar') Trying to boot from SPI
U-Boot 2023.04-rc2-00040-gbe9fd01af6 (Mar 01 2023 - 11:31:30 +0530)
SoC: AM62X SR1.0 GP Model: Texas Instruments AM625 SK DRAM: 2 GiB Core: 50 devices, 20 uclasses, devicetree: separate MMC: mmc@fa10000: 0, mmc@fa00000: 1, mmc@fa20000: 2 Loading Environment from nowhere... OK In: serial@2800000 Out: serial@2800000 Err: serial@2800000 Net: eth0: ethernet@8000000port@1 Hit any key to stop autoboot: 0 => ...
Odd length reads: ... => sf probe SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB => fatload mmc 1 $loadaddr ti.gz 12285 bytes read in 8 ms (1.5 MiB/s) => sf update $loadaddr 0x700000 $filesize device 0 offset 0x700000, size 0x2ffd 12285 bytes written, 0 bytes skipped in 1.478s, speed 8494 B/s sf read 0x90000000 0x700000 $filesize device 0 offset 0x700000, size 0x2ffd SF: 12285 bytes @ 0x700000 Read: OK => cmp.b $loadaddr 0x90000000 $filesize # all bytes were same # ...
Cc: Vignesh Raghavendra vigneshr@ti.com Cc: Pratyush Yadav ptyadav@amazon.de Cc: Vaishnav Achath vaishnav.a@ti.com Cc: Nikhil M Jain n-jain1@ti.com
Dhruva Gole (2): spi: spi-mem: s/dummy/data buswidth check in dtr_supports_op() spi: spi-mem: perform odd len check only while writing data
drivers/spi/spi-mem.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
From: Dhruva Gole d-gole@ti.com
This should have been op->data.buswidth instead as we check for octal bus width for the data related ops Also add explanation for why there is checks for 8D even data bytes
Cc: Pratyush Yadav pratyush@kernel.org Reviewed-by: Pratyush Yadav ptyadav@amazon.de Tested-by: Nikhil M Jain n-jain1@ti.com Signed-off-by: Dhruva Gole d-gole@ti.com ---
This patch has no changes wrt previously sent patch: https://lore.kernel.org/u-boot/20230220054231.74367-1-d-gole@ti.com/
drivers/spi/spi-mem.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 8e8995fc53..57a36f31a5 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -181,8 +181,12 @@ bool spi_mem_dtr_supports_op(struct spi_slave *slave, if (op->dummy.nbytes && op->dummy.buswidth == 8 && op->dummy.nbytes % 2) return false;
+ /* + * Transactions of odd length do not make sense for 8D-8D-8D mode + * because a byte is transferred in just half a cycle. + */ if (op->data.dir != SPI_MEM_NO_DATA && - op->dummy.buswidth == 8 && op->data.nbytes % 2) + op->data.buswidth == 8 && op->data.nbytes % 2) return false;
return spi_mem_check_buswidth(slave, op);
From: Dhruva Gole d-gole@ti.com
in spi_mem_dtr_supports_op we have a check for allowing only even number of bytes to be r/w. Odd bytes writing can be a concern while writing data to a flash for example because 8 DTR mode doesn't support it. However, reading ODD Bytes even though may not be physically possible we can still allow for it because it will not have serious implications on any critical registers being overwritten since they are just reads.
Cc: Vaishnav Achath vaishnav.a@ti.com Cc: Pratyush Yadav pratyush@kernel.org Cc: Vignesh Raghavendra vigneshr@ti.com Tested-by: Nikhil M Jain n-jain1@ti.com Signed-off-by: Dhruva Gole d-gole@ti.com --- drivers/spi/spi-mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 57a36f31a5..b7eca58359 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -185,7 +185,7 @@ bool spi_mem_dtr_supports_op(struct spi_slave *slave, * Transactions of odd length do not make sense for 8D-8D-8D mode * because a byte is transferred in just half a cycle. */ - if (op->data.dir != SPI_MEM_NO_DATA && + if (op->data.dir != SPI_MEM_NO_DATA && op->data.dir != SPI_MEM_DATA_IN && op->data.buswidth == 8 && op->data.nbytes % 2) return false;
participants (1)
-
Nikhil M Jain