[PATCH resend 4/5] mpc8xxx_spi: always use 8-bit characters, don't read or write garbage

[Tom Rini]

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 at 7000:0 dout 0FB53618 din 00000000 bitlen 8
> mpc8xxx_spi_xfer: slave spi at 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 at 7000:0 dout 0FB536E8 din 00000000 bitlen 40
> mpc8xxx_spi_xfer: slave spi at 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 at 7000:0 dout 0FB53848 din 00000000 bitlen 40
> mpc8xxx_spi_xfer: slave spi at 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 at 7000:0 dout 0FB536E8 din 00000000 bitlen 40
> mpc8xxx_spi_xfer: slave spi at 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 at 7000:0 dout 0FB536E8 din 00000000 bitlen 40
> mpc8xxx_spi_xfer: slave spi at 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 at prevas.dk>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200331/8c677885/attachment.sig>