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 <k...@prevas.dk> > Signed-off-by: Rasmus Villemoes <rasmus.villem...@prevas.dk>
Applied to u-boot/master, thanks! -- Tom
signature.asc
Description: PGP signature