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 <[EMAIL PROTECTED]>
>
>
> 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 <[EMAIL PROTECTED]>
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: [EMAIL PROTECTED] • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
