On Tue, Feb 11, 2020 at 03:20:25PM +0000, Rasmus Villemoes wrote:
There are a few problems with the current driver.
First, it unconditionally reads from dout/writes to din whether or not those pointers are NULL. So for example a simple "sf probe" ends up writing four bytes at address 0:
=> md.l 0x0 8 00000000: 45454545 45454545 05050505 05050505 EEEEEEEE........ 00000010: 00000000 00000000 07070707 07070707 ................ => sf probe 0 mpc8xxx_spi_xfer: slave spi@7000:0 dout 0FB53618 din 00000000 bitlen 8 mpc8xxx_spi_xfer: slave spi@7000:0 dout 00000000 din 0FB536B8 bitlen 48 SF: Detected s25sl032p with page size 256 Bytes, erase size 64 KiB, total 4 MiB => md.l 0x0 8 00000000: ff000000 45454545 05050505 05050505 ....EEEE........ 00000010: 00000000 00000000 07070707 07070707 ................
(here I've change the first debug statement to a printf, and made it print the din/dout pointers rather than the uints they point at).
Second, as we can also see above, it always writes a full 32 bits, even if a smaller amount was requested. So for example
=> mw.l $loadaddr 0xaabbccdd 8 => md.l $loadaddr 8 02000000: aabbccdd aabbccdd aabbccdd aabbccdd ................ 02000010: aabbccdd aabbccdd aabbccdd aabbccdd ................ => sf read $loadaddr 0x400 6 device 0 offset 0x400, size 0x6 mpc8xxx_spi_xfer: slave spi@7000:0 dout 0FB536E8 din 00000000 bitlen 40 mpc8xxx_spi_xfer: slave spi@7000:0 dout 00000000 din 02000000 bitlen 48 SF: 6 bytes @ 0x400 Read: OK => sf read 0x02000010 0x400 8 device 0 offset 0x400, size 0x8 mpc8xxx_spi_xfer: slave spi@7000:0 dout 0FB53848 din 00000000 bitlen 40 mpc8xxx_spi_xfer: slave spi@7000:0 dout 00000000 din 02000010 bitlen 64 SF: 8 bytes @ 0x400 Read: OK => md.l $loadaddr 8 02000000: 45454545 45450000 aabbccdd aabbccdd EEEEEE.......... 02000010: 45454545 45454545 aabbccdd aabbccdd EEEEEEEE........
Finally, when the bitlen is 24 mod 32 (e.g. requesting to read 3 or 7 bytes), the last three bytes and up being the wrong ones, since the driver does a full 32 bit read and then shifts the wrong byte out:
=> mw.l $loadaddr 0xaabbccdd 4 => md.l $loadaddr 4 02000000: aabbccdd aabbccdd aabbccdd aabbccdd ................ => sf read $loadaddr 0x444 10 device 0 offset 0x444, size 0x10 mpc8xxx_spi_xfer: slave spi@7000:0 dout 0FB536E8 din 00000000 bitlen 40 mpc8xxx_spi_xfer: slave spi@7000:0 dout 00000000 din 02000000 bitlen 128 SF: 16 bytes @ 0x444 Read: OK => md.l $loadaddr 4 02000000: 552d426f 6f742032 3031392e 30342d30 U-Boot 2019.04-0 => mw.l $loadaddr 0xaabbccdd 4 => sf read $loadaddr 0x444 0xb device 0 offset 0x444, size 0xb mpc8xxx_spi_xfer: slave spi@7000:0 dout 0FB536E8 din 00000000 bitlen 40 mpc8xxx_spi_xfer: slave spi@7000:0 dout 00000000 din 02000000 bitlen 88 SF: 11 bytes @ 0x444 Read: OK => md.l $loadaddr 4 02000000: 552d426f 6f742032 31392e00 aabbccdd U-Boot 219......
Fix all of that by always using a character size of 8, and reject transfers that are not a whole number of bytes. While it ends being more work for the CPU, we're mostly bounded by the speed of the SPI bus, and we avoid writing to the mode register in every loop.
Based on work by Klaus H. Sørensen.
Cc: Klaus H. Sorensen khso@prevas.dk Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Applied to u-boot/master, thanks!