Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31793
Change subject: drivers/spi: Instrument erase operation ......................................................................
drivers/spi: Instrument erase operation
Verify console output contains the new string.
BS: BS_DEV_INIT times (us): entry 0 run 72771 exit 0 Finalize devices... Devices finalized
APIC 00: ** Enter AmdInitLate [00020004] SB800: sb_Late_Post AmdInitLate() returned AGESA_SUCCESS APIC 00: Heap in SystemMem (4) at 0x10000014 APIC 00: ** Exit AmdInitLate [00020004]
APIC 00: ** Enter AmdS3Save [0002000b] Manufacturer: ef SF: Detected W25Q32_V with sector size 0x1000, total 0x400000 SF: Successfully erased 4096 bytes @ 0xffff1000 in 97754 usecs Manufacturer: ef SF: Detected W25Q32_V with sector size 0x1000, total 0x400000 SF: Successfully erased 4096 bytes @ 0xffff0000 in 121777 usecs AmdS3Save() returned AGESA_SUCCESS APIC 00: Heap in SystemMem (4) at 0x10000014 APIC 00: ** Exit AmdS3Save [0002000b] BS: BS_POST_DEVICE times (us): entry 0 run 3 exit 228371
Change-Id: I4eb8a850d7263bc3c800d97655f0b37623b78562 Signed-off-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/drivers/spi/spi_flash.c 1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/31793/1
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index d1f5227..7f64968 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -198,6 +198,9 @@ u32 start, end, erase_size; int ret = -1; u8 cmd[4]; + struct stopwatch sw; + + stopwatch_init(&sw);
erase_size = flash->sector_size; if (offset % erase_size || len % erase_size) { @@ -234,7 +237,8 @@ goto out; }
- printk(BIOS_DEBUG, "SF: Successfully erased %zu bytes @ %#x\n", len, start); + printk(BIOS_DEBUG, "SF: Successfully erased %zu bytes @ %#x in %ld usecs\n", + len, start, stopwatch_duration_usecs(&sw));
out: return ret;
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31793 )
Change subject: drivers/spi: Instrument erase operation ......................................................................
Patch Set 1:
Julius, as an ARM device is failing, could you please comment?
``` LINK cbfs/fallback/bootblock.debug arm-eabi-ld.bfd: Bootblock exceeded its allotted size! (28K) arm-eabi-ld.bfd: verstage overlaps the previous region! make[1]: *** [src/arch/arm/Makefile.inc:70: /cb-build/coreboot-gerrit.0/GOOGLE_SMAUG/cbfs/fallback/bootblock.debug] Error 1 make[1]: Leaving directory '/home/coreboot/slave-root/workspace/coreboot-gerrit' ```
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31793 )
Change subject: drivers/spi: Instrument erase operation ......................................................................
Patch Set 1:
Like it says, bootblock runs out of space. From what I remember, SMAUG had some 150 bytes room, failed during those console loglevel changes in similar fashion.
BTW: There is code adding timestamps around every AmdXX() call. Besides, I don't think there is much you can do here, SPI erase is pretty much a busy wait-poll-loop and that W25Q part may have 45ms(typ) to 400ms(max) sectore erase time.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31793 )
Change subject: drivers/spi: Instrument erase operation ......................................................................
Patch Set 1:
Yeah, what Kyösti said. I think half a dozen floating CLs have run into that by now but it looks like none of them have merged (with a fix) yet. You can just change the tegra210 memlayout to move one more KB from TIMESTAMP to BOOTBLOCK, for example.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31793?usp=email )
Change subject: drivers/spi: Instrument erase operation ......................................................................
Abandoned
-
Julius Werner (Code Review) -
Kyösti Mälkki (Code Review)
-
Paul Menzel (Code Review)
-
Stefan Reinauer (Code Review)