On 12/01/2023 10:06, Wolfram Sang wrote: > From: Cong Dang <cong.dang.xn@xxxxxxxxxxx> > > The dummy cycles value was wrongly calculated if dummy.buswidth > 1, > which affects QSPI, OSPI, HyperFlash on various SoCs. We're lucky in > Single SPI case since its dummy.buswidth equals to 1, so the result of > the division is unchanged > > This issue can be reproduced using something like the following commands > A. QSPI mode: Mount device with jffs2 format > jffs2: CLEANMARKER node found at 0x00000004, not first node in block (0x00000000) > > B. QSPI mode: Write data to mtd10, where mtd10 is a parition on SPI Flash > storage, defined properly in a device tree > > [Correct fragment, read from SPI Flash] > > root@v3x:~# echo "hello" > /dev/mtd10 > root@v3x:~# hexdump -C -n100 /dev/mtd10 > 00000000 68 65 6c 6c 6f 0a ff ff ff ff ff ff ff ff ff ff |hello...........| > 00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > > [Incorrect read of the same fragment: see the difference at offsets 0-3] > > root@v3x:~# echo "hello" > /dev/mtd10 > root@v3x:~# hexdump -C -n100 /dev/mtd10 > 00000000 00 00 00 00 68 65 6c 6c 6f 0a ff ff ff ff ff ff |....hello.......| > 00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > > As seen from the result, 4 NULL bytes were inserted before the test data. > Wrong calculation in rpcif_prepare() led to miss of some dummy cycle. A > division by bus width is redundant because it had been performed already > in spi-rpc-if.c::rpcif_spi_mem_prepare() > > Fix this by removing the redundant division. > > Fixes: ca7d8b980b67 ("memory: add Renesas RPC-IF driver") > Signed-off-by: Cong Dang <cong.dang.xn@xxxxxxxxxxx> > Signed-off-by: Hai Pham <hai.pham.ud@xxxxxxxxxxx> > Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > --- > > Sadly, I cannot test this patch myself because I don't have access to > hardware which uses a buswidth > 1 for the dummy read. However, from > code review, this patch makes sense. The division by buswidth is done > twice, once in the SPI driver and once in the RPC core. It should stay > only in the SPI driver. Any tests or further reviews on this? If not, I'll pick it up in few days. Best regards, Krzysztof