Uwe Poeche has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34384 )
Change subject: include/spi-generic: clean up FLASH TIMEOUTs ......................................................................
include/spi-generic: clean up FLASH TIMEOUTs
This patch provides as follow up of patch 983a2ef65c an cleanup of defines for FLASH TIMEOUTs and the following necessary changes in the source files which use these defines.
Change-Id: Ibc4beda2660a83fd5f0ed325b2ee3148c6d96639 Signed-off-by: Uwe Poeche uwe.poeche@siemens.com --- M src/drivers/spi/adesto.c M src/drivers/spi/amic.c M src/drivers/spi/atmel.c M src/drivers/spi/eon.c M src/drivers/spi/gigadevice.c M src/drivers/spi/macronix.c M src/drivers/spi/spansion.c M src/drivers/spi/spi_flash.c M src/drivers/spi/sst.c M src/drivers/spi/stmicro.c M src/drivers/spi/winbond.c M src/include/spi-generic.h M src/southbridge/intel/common/spi.c 13 files changed, 29 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/34384/1
diff --git a/src/drivers/spi/adesto.c b/src/drivers/spi/adesto.c index 4e1043e..c74fd70 100644 --- a/src/drivers/spi/adesto.c +++ b/src/drivers/spi/adesto.c @@ -181,7 +181,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/amic.c b/src/drivers/spi/amic.c index b580dc3..6e1234b 100644 --- a/src/drivers/spi/amic.c +++ b/src/drivers/spi/amic.c @@ -155,7 +155,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/atmel.c b/src/drivers/spi/atmel.c index 58a2862..ac7f0d9 100644 --- a/src/drivers/spi/atmel.c +++ b/src/drivers/spi/atmel.c @@ -137,7 +137,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/eon.c b/src/drivers/spi/eon.c index f3cf70e..c6fdba1 100644 --- a/src/drivers/spi/eon.c +++ b/src/drivers/spi/eon.c @@ -270,7 +270,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) { printk(BIOS_WARNING, "SF: EON Page Program timeout\n"); goto out; diff --git a/src/drivers/spi/gigadevice.c b/src/drivers/spi/gigadevice.c index 1ff594a..71433cc 100644 --- a/src/drivers/spi/gigadevice.c +++ b/src/drivers/spi/gigadevice.c @@ -212,7 +212,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index 5a97b8f..a41e96f 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -249,7 +249,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
diff --git a/src/drivers/spi/spansion.c b/src/drivers/spi/spansion.c index 4a241ba..c3a071e 100644 --- a/src/drivers/spi/spansion.c +++ b/src/drivers/spi/spansion.c @@ -264,7 +264,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index a81306e..cfa500e 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -232,7 +232,8 @@ if (ret) goto out;
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PAGE_ERASE_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PAGE_ERASE_TIMEOUT_MS); if (ret) goto out; } diff --git a/src/drivers/spi/sst.c b/src/drivers/spi/sst.c index abe3f2a..429afa0 100644 --- a/src/drivers/spi/sst.c +++ b/src/drivers/spi/sst.c @@ -179,7 +179,7 @@ if (ret) return ret;
- return spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + return spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT_MS); }
static int sst_write_256(const struct spi_flash *flash, u32 offset, size_t len, @@ -239,7 +239,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
@@ -294,7 +295,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
diff --git a/src/drivers/spi/stmicro.c b/src/drivers/spi/stmicro.c index 6625764..98f6e4e 100644 --- a/src/drivers/spi/stmicro.c +++ b/src/drivers/spi/stmicro.c @@ -331,7 +331,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 9e9bb00..3e0a266 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -326,7 +326,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index 93704f8..ffd3d2d 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -20,10 +20,9 @@ * is a problem (and well your system already is broken), so err on the side * of caution in case we're dealing with slower SPI buses and/or processors. */ -#define CONF_SYS_HZ 100 -#define SPI_FLASH_PROG_TIMEOUT (2 * CONF_SYS_HZ) -#define SPI_FLASH_PAGE_ERASE_TIMEOUT (5 * CONF_SYS_HZ) -#define SPI_FLASH_SECTOR_ERASE_TIMEOUT (10 * CONF_SYS_HZ) +#define SPI_FLASH_PROG_TIMEOUT_MS 200 +#define SPI_FLASH_PAGE_ERASE_TIMEOUT_MS 500 +#define SPI_FLASH_SECTOR_ERASE_TIMEOUT_MS 1000
#include <commonlib/region.h> #include <stdint.h> diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 6fece4f..3244808 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -726,7 +726,7 @@ u32 start, end, erase_size; int ret; uint16_t hsfc; - unsigned int timeout = 1000 * SPI_FLASH_SECTOR_ERASE_TIMEOUT; + unsigned int timeout = 1000 * SPI_FLASH_SECTOR_ERASE_TIMEOUT_MS;
erase_size = flash->sector_size; if (offset % erase_size || len % erase_size) {
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34384 )
Change subject: include/spi-generic: clean up FLASH TIMEOUTs ......................................................................
Patch Set 1: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@7 PS1, Line 7: include/spi-generic: clean up FLASH TIMEOUTs Maybe:
include/spi-generic: Append unit to time-out macro names
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: 983a2ef65c The hash will change, when the commit is committed to the master branch.
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: an a
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: follow up follow-up
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: This patch provides as follow up of patch 983a2ef65c an cleanup of : defines for FLASH TIMEOUTs and the following necessary changes in the : source files which use these defines. With the updated commit message, I’d just remove the description.
Hello Patrick Rudolph, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34384
to look at the new patch set (#2).
Change subject: include/spi-generic: Append unit to macro names ......................................................................
include/spi-generic: Append unit to macro names
This patch appends a unit (milliseconds) to time-out macro names for better understanding the code which is using the macros.
Change-Id: Ibc4beda2660a83fd5f0ed325b2ee3148c6d96639 Signed-off-by: Uwe Poeche uwe.poeche@siemens.com --- M src/drivers/spi/adesto.c M src/drivers/spi/amic.c M src/drivers/spi/atmel.c M src/drivers/spi/eon.c M src/drivers/spi/gigadevice.c M src/drivers/spi/macronix.c M src/drivers/spi/spansion.c M src/drivers/spi/spi_flash.c M src/drivers/spi/sst.c M src/drivers/spi/stmicro.c M src/drivers/spi/winbond.c M src/include/spi-generic.h M src/southbridge/intel/common/spi.c 13 files changed, 29 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/34384/2
Uwe Poeche has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34384 )
Change subject: include/spi-generic: Append unit to macro names ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@7 PS1, Line 7: include/spi-generic: clean up FLASH TIMEOUTs
Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: follow up
follow-up
Ack
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: 983a2ef65c
The hash will change, when the commit is committed to the master branch.
Ack
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: an
a
Ack
https://review.coreboot.org/c/coreboot/+/34384/1//COMMIT_MSG@9 PS1, Line 9: This patch provides as follow up of patch 983a2ef65c an cleanup of : defines for FLASH TIMEOUTs and the following necessary changes in the : source files which use these defines.
With the updated commit message, I’d just remove the description.
Done
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34384 )
Change subject: include/spi-generic: Append unit to macro names ......................................................................
Patch Set 2: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34384 )
Change subject: include/spi-generic: Append unit to macro names ......................................................................
include/spi-generic: Append unit to macro names
This patch appends a unit (milliseconds) to time-out macro names for better understanding the code which is using the macros.
Change-Id: Ibc4beda2660a83fd5f0ed325b2ee3148c6d96639 Signed-off-by: Uwe Poeche uwe.poeche@siemens.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34384 Reviewed-by: Werner Zeh werner.zeh@siemens.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/spi/adesto.c M src/drivers/spi/amic.c M src/drivers/spi/atmel.c M src/drivers/spi/eon.c M src/drivers/spi/gigadevice.c M src/drivers/spi/macronix.c M src/drivers/spi/spansion.c M src/drivers/spi/spi_flash.c M src/drivers/spi/sst.c M src/drivers/spi/stmicro.c M src/drivers/spi/winbond.c M src/include/spi-generic.h M src/southbridge/intel/common/spi.c 13 files changed, 29 insertions(+), 18 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Werner Zeh: Looks good to me, approved
diff --git a/src/drivers/spi/adesto.c b/src/drivers/spi/adesto.c index 4e1043e..c74fd70 100644 --- a/src/drivers/spi/adesto.c +++ b/src/drivers/spi/adesto.c @@ -181,7 +181,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/amic.c b/src/drivers/spi/amic.c index b580dc3..6e1234b 100644 --- a/src/drivers/spi/amic.c +++ b/src/drivers/spi/amic.c @@ -155,7 +155,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/atmel.c b/src/drivers/spi/atmel.c index 58a2862..ac7f0d9 100644 --- a/src/drivers/spi/atmel.c +++ b/src/drivers/spi/atmel.c @@ -137,7 +137,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/eon.c b/src/drivers/spi/eon.c index f3cf70e..c6fdba1 100644 --- a/src/drivers/spi/eon.c +++ b/src/drivers/spi/eon.c @@ -270,7 +270,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) { printk(BIOS_WARNING, "SF: EON Page Program timeout\n"); goto out; diff --git a/src/drivers/spi/gigadevice.c b/src/drivers/spi/gigadevice.c index 1ff594a..71433cc 100644 --- a/src/drivers/spi/gigadevice.c +++ b/src/drivers/spi/gigadevice.c @@ -212,7 +212,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index 5a97b8f..a41e96f 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -249,7 +249,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
diff --git a/src/drivers/spi/spansion.c b/src/drivers/spi/spansion.c index 4a241ba..c3a071e 100644 --- a/src/drivers/spi/spansion.c +++ b/src/drivers/spi/spansion.c @@ -264,7 +264,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index a81306e..cfa500e 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -232,7 +232,8 @@ if (ret) goto out;
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PAGE_ERASE_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PAGE_ERASE_TIMEOUT_MS); if (ret) goto out; } diff --git a/src/drivers/spi/sst.c b/src/drivers/spi/sst.c index abe3f2a..429afa0 100644 --- a/src/drivers/spi/sst.c +++ b/src/drivers/spi/sst.c @@ -179,7 +179,7 @@ if (ret) return ret;
- return spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + return spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT_MS); }
static int sst_write_256(const struct spi_flash *flash, u32 offset, size_t len, @@ -239,7 +239,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
@@ -294,7 +295,8 @@ break; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) break;
diff --git a/src/drivers/spi/stmicro.c b/src/drivers/spi/stmicro.c index 6625764..98f6e4e 100644 --- a/src/drivers/spi/stmicro.c +++ b/src/drivers/spi/stmicro.c @@ -331,7 +331,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 9e9bb00..3e0a266 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -326,7 +326,8 @@ goto out; }
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + ret = spi_flash_cmd_wait_ready(flash, + SPI_FLASH_PROG_TIMEOUT_MS); if (ret) goto out;
diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index 93704f8..ffd3d2d 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -20,10 +20,9 @@ * is a problem (and well your system already is broken), so err on the side * of caution in case we're dealing with slower SPI buses and/or processors. */ -#define CONF_SYS_HZ 100 -#define SPI_FLASH_PROG_TIMEOUT (2 * CONF_SYS_HZ) -#define SPI_FLASH_PAGE_ERASE_TIMEOUT (5 * CONF_SYS_HZ) -#define SPI_FLASH_SECTOR_ERASE_TIMEOUT (10 * CONF_SYS_HZ) +#define SPI_FLASH_PROG_TIMEOUT_MS 200 +#define SPI_FLASH_PAGE_ERASE_TIMEOUT_MS 500 +#define SPI_FLASH_SECTOR_ERASE_TIMEOUT_MS 1000
#include <commonlib/region.h> #include <stdint.h> diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 6fece4f..3244808 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -726,7 +726,7 @@ u32 start, end, erase_size; int ret; uint16_t hsfc; - unsigned int timeout = 1000 * SPI_FLASH_SECTOR_ERASE_TIMEOUT; + unsigned int timeout = 1000 * SPI_FLASH_SECTOR_ERASE_TIMEOUT_MS;
erase_size = flash->sector_size; if (offset % erase_size || len % erase_size) {
-
Martin Roth (Code Review) -
Paul Menzel (Code Review)
-
Uwe Poeche (Code Review)
-
Werner Zeh (Code Review)