[coreboot] [PATCH] flashrom: Check for failed SPI command execution

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> Check for failed SPI command execution in flashrom. Although SPI itself
> does not have a mechanism to signal command failure, the SPI host may be
> unable to send a given command over the wire due to security or hardware
> limitations. The current code ignores these mechanisms completely and
> simply assumes almost every command succeeds. Complain if SPI command
> execution fails.
>
> Since locked down Intel chipsets (like the one we had problems with
> earlier) only allow a small subset of commands, find the common subset
> of commands between the chipset and the ROM in the chip erase case. That
> is accomplished by the new spi_chip_erase_60_c7() which can be used for
> chips supporting both 0x60 and 0xc7 chip erase commands.
>
> Both parts of the patch address problems seen in the real world. The
> increased verbosity for the error case will help us diagnose and address
> problems better.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>
>   


>  	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
>  		sleep(1);
>  	return 0;
> @@ -290,19 +304,44 @@
>  int spi_chip_erase_c7(struct flashchip *flash)
>  {
>  	const unsigned char cmd[JEDEC_CE_C7_OUTSIZE] = { JEDEC_CE_C7 };
> +	int result;
>  
> -	spi_disable_blockprotect();
> -	spi_write_enable();
> +	result = spi_disable_blockprotect();
> +	if (result) {
> +		printf_debug("spi_disable_blockprotect failed\n");
> +		return result;
> +	}
> +	result = spi_write_enable();
> +	if (result) {
> +		printf_debug("spi_write_enable failed\n");
> +		return result;
> +	}
>   
This code is duplicated in a couple of places. Is it worth factoring it out?

> +int spi_chip_erase_60_c7(struct flashchip *flash)
> +{
> +	int result;
> +	result = spi_chip_erase_60(flash);
> +	if (result) {
> +		printf_debug("spi_chip_erase_60 failed, trying c7\n");
> +		result = spi_chip_erase_c7(flash);
> +	}
> +	return result;
> +}
>   
I don't particularly like this. Maybe we should have spi_chip_erase()
try all different erase functions in a row, and check whether the erase
was actually successfull (all bytes 0xff)?

Otherwise: Acked-by: Stefan Reinauer <stepan at coresystems.de>


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866