[PATCH] flashrom: Fix probe_spi_res failover

[Carl-Daniel Hailfinger]

The existing check in probe_spi_res() was right for SPI controllers
which support all commands, but may not exist.
For controllers which support only a subset of commands, it will fail in
unexpected ways.

The new logic checks if RDID could be issued and its return values made
sense. In that case, RES probing is not performed. Otherwise, we try RES.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-spi_res_rdid_failover/spi.c
===================================================================
--- flashrom-spi_res_rdid_failover/spi.c	(revision 3773)
+++ flashrom-spi_res_rdid_failover/spi.c	(working copy)
@@ -160,15 +160,13 @@
    unsigned char readarr[3];
    uint32_t model_id;

-	if (spi_rdid(readarr, 3))
-		/* We couldn't issue RDID, it's pointless to try RES. */
+	/* Check if RDID was successful and did not return 0xff 0xff 0xff.
+	 * In that case, RES is pointless.
+	 */
+	if (!spi_rdid(readarr, 3) && ((readarr[0] != 0xff) ||
+	    (readarr[1] != 0xff) || (readarr[2] != 0xff)))
    	return 0;

-	/* Check if RDID returns 0xff 0xff 0xff, then we use RES. */
-	if ((readarr[0] != 0xff) || (readarr[1] != 0xff) ||
-	    (readarr[2] != 0xff))
-		return 0;
-
    if (spi_res(readarr))
    	return 0;

[FENG Yu Ning]

On Fri, Nov 28, 2008 at 1:32 AM, Carl-Daniel Hailfinger
 wrote:
> The existing check in probe_spi_res() was right for SPI controllers
> which support all commands, but may not exist.
> For controllers which support only a subset of commands, it will fail in
> unexpected ways.

sometimes the controller supports that command, but the command is
1. not in the opmenu, and we cannot change it (spi conf locked down)
2. the slave spi flash chip does not support rdid.
for example, my ich7 + sst25lf040a (both 1. and 2.)

There is also BBAR issue in the probing process and more. spi_res only
reads from address 0x0, which could be below BBA. And there is
read-only opmenu index difference with our hardcoded opcodes table.
They are other problems to attack against seperately, though.

Obviousely ack from me is not decisive, but still
Acked-by: FENG yu ning

[Carl-Daniel Hailfinger]

On 27.11.2008 19:37, FENG Yu Ning wrote:
> On Fri, Nov 28, 2008 at 1:32 AM, Carl-Daniel Hailfinger
>  wrote:
> > The existing check in probe_spi_res() was right for SPI controllers
> > which support all commands, but may not exist.
> > For controllers which support only a subset of commands, it will fail in
> > unexpected ways.
>
> sometimes the controller supports that command, but the command is
> not in the opmenu, and we cannot change it (spi conf locked down)
> the slave spi flash chip does not support rdid.
>
> for example, my ich7 + sst25lf040a (both 1. and 2.)
>
> There is also BBAR issue in the probing process and more. spi_res only
> reads from address 0x0, which could be below BBA. And there is
> read-only opmenu index difference with our hardcoded opcodes table.
> They are other problems to attack against seperately, though.
>
> Obviousely ack from me is not decisive, but still
> Acked-by: FENG yu ning

Thanks, r3774.

Regards,
Carl-Daniel