Re: [flashrom] [PATCH 0/5] check_trans and probe_spi_rdid_edi

[Carl-Daniel Hailfinger]

Am 03.10.2012 06:13 schrieb Stefan Tauner:
> This patch set enables us to figure out if transactions will succeed
> without executing them, which allows for a refactoring of
> probe_spi_rdid_generic() which in consequence makes it possible to add
> probe_spi_rdid_edi().

I think this is a good idea. Review below and in each individual patch.
> The infrastructure code as a whole, but especially the ichspi code
> is heavily modified. Please be aware of that if you test it on
> mainboards.
>
> Stefan Tauner (5):
>   Refactor ichspi and prepare it for check_trans().
>   Prepare wbsio_spi.c for check_trans().
>   Introduce check_trans().
>   Generify probe_spi_rdid_generic() and add probe_spi_rdid_edi().
>   Let the dummy programmer emulate SPI payload limitations.
>
> This is the output of the dummy programmer to show the new behavior:
>
> flashrom v0.9.6.1-r1611 on Linux 3.2.0-31-generic (x86_64)
> flashrom is free software, get the source code at http://www.flashrom.org
>
> flashrom was built with libpci 3.1.8, GCC 4.6.3, little endian
> Command line (3 args): ./flashrom -p dummy:bus=SPI,spi_prog=wbsio,emulate=M25P10.RES -V
> Calibrating delay loop... OS timer resolution is 1 usecs, 1456M loops per second, 10 myus = 10 us, 100 myus = 127 us, 1000 myus = 1023 us, 10000 myus = 10024 us, 4 myus = 10 us, OK.
> Initializing dummy programmer
> Requested buses are: SPI
> Enabling support for SPI flash.
> Emulating ST M25P10.RES SPI flash chip (RES, page write)
> Using SPI payload limitations of the wbsio programmer.
> Filling fake flash chip with 0xff, size 131072
> The following protocols are supported: SPI.
> Probing for AMIC A25L05PT, 64 kB: 3 byte RDID not supported on this SPI controller.

This should have been "4 byte RDID..."
> Probing for AMIC A25L05PU, 64 kB: 3 byte RDID not supported on this SPI controller.
> [...]
> Probing for Spansion S25FL064A, 8192 kB: 3 byte RDID not supported on this SPI controller.
> Probing for SST SST25LF040A, 512 kB: probe_spi_res2: id1 0x10, id2 0x10
> Probing for SST SST25LF080A, 1024 kB: probe_spi_res2: id1 0x10, id2 0x10
> Probing for SST SST25VF010, 128 kB: probe_spi_rems: id1 0xff, id2 0xff
> Probing for SST SST25VF016B, 2048 kB: 3 byte RDID not supported on this SPI controller.
> Probing for SST SST25VF032B, 4096 kB: 3 byte RDID not supported on this SPI controller.
> Probing for SST SST25VF064C, 8192 kB: 3 byte RDID not supported on this SPI controller.
> Probing for SST SST25VF040, 512 kB: probe_spi_rems: id1 0xff, id2 0xff
> Probing for SST SST25VF040B, 512 kB: 3 byte RDID not supported on this SPI controller.
> Probing for SST SST25VF040B.REMS, 512 kB: probe_spi_rems: id1 0xff, id2 0xff
> Probing for SST SST25VF080B, 1024 kB: 3 byte RDID not supported on this SPI controller.
> [...]
> Probing for ST M25P05-A, 64 kB: 3 byte RDID not supported on this SPI controller.
> Probing for ST M25P05, 64 kB: probe_spi_res1: id 0x10
> Chip status register is 00
> Found ST flash chip "M25P05" (64 kB, SPI) on dummy.
> Probing for ST M25P10-A, 128 kB: 3 byte RDID not supported on this SPI controller.
> Probing for ST M25P10, 128 kB: probe_spi_res1: id 0x10
> Chip status register is 00
> Found ST flash chip "M25P10" (128 kB, SPI) on dummy.
> Probing for ST M25P20, 256 kB: 3 byte RDID not supported on this SPI controller.
> Probing for ST M25P40, 512 kB: 3 byte RDID not supported on this SPI controller.
> Probing for ST M25P40-old, 512 kB: probe_spi_res1: id 0x10
> Probing for ST M25P80, 1024 kB: 3 byte RDID not supported on this SPI controller.
> [...]
> Probing for Winbond W25X64, 8192 kB: 3 byte RDID not supported on this SPI controller.
> Probing for Unknown SFDP-capable chip, 0 kB: No SFDP signature found.
> Probing for AMIC unknown AMIC SPI chip, 0 kB: 3 byte RDID not supported on this SPI controller.
> [...]
> Probing for Generic unknown SPI chip (RDID), 0 kB: 3 byte RDID not supported on this SPI controller.
> Probing for Generic unknown SPI chip (REMS), 0 kB: probe_spi_rems: id1 0xff, id2 0xff
> Multiple flash chips were detected: "M25P05", "M25P10"
> Please specify which chip to use with the -c <chipname> option.

Regards,
Carl-Daniel

[PATCH 0/5] check_trans and probe_spi_rdid_edi

[Stefan Tauner]

On Mon, 15 Oct 2012 23:17:44 +0200
Carl-Daniel Hailfinger  wrote:
> > Probing for AMIC A25L05PT, 64 kB: 3 byte RDID not supported on this SPI controller.
>
> This should have been "4 byte RDID..."

well, when one expects a chip-centric message here (and one probably
does, even when the controller is mentioned), then yes. but with my
patches that part of the code does not know how many vendor bytes are
needed to be read so it checks for down to the minimum of 3 bytes (and
prints that then). it was a compromise to make it more generic without
more boiler plate code (such as probe_spi_rdid4()). you will see the
pros and cons of the approach when you look at the 4th patch more
closely. one possible solution for the message itself is to print "RDID
with >=i bytes not supported...". in the long term the SPI probing will
probably/hopefully change a lot anyway, then this message is no longer
relevant anyway.

i'll wait for reviews of the complete set before doing any further
changes.