Mary Ruthven has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43527 )
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
raiden_debug_spi: add param for a custom reset setup
Cr50 asserts EC_RST_L to flash the AP. Asserting EC_RST_L breaks flashing the AP on some boards. This change adds a custom_rst arg to tell cr50 not to assert any system reset signals during flash AP setup.
BUG=b:154885210 BRANCH=none TEST=manual
sudo flashrom -p raiden_debug_spi:target=EC -r
sudo flashrom -p raiden_debug_spi:target=AP -r
sudo flashrom -p raiden_debug_spi:target=AP,custom_rst=true -r
sudo flashrom -p raiden_debug_spi:target=AP,custom_rst=bogus -r
sudo flashrom -p raiden_debug_spi -r
Signed-off-by: Mary Ruthven mruthven@chromium.org Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 --- M raiden_debug_spi.c 1 file changed, 30 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/43527/1
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index b55307a..6b3468e 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -136,17 +136,17 @@ };
enum raiden_debug_spi_request { - RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, - RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, - RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, - RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, - RAIDEN_DEBUG_SPI_REQ_ENABLE_H1 = 0x0004, - RAIDEN_DEBUG_SPI_REQ_RESET = 0x0005, - RAIDEN_DEBUG_SPI_REQ_BOOT_CFG = 0x0006, - RAIDEN_DEBUG_SPI_REQ_SOCKET = 0x0007, - RAIDEN_DEBUG_SPI_REQ_SIGNING_START = 0x0008, - RAIDEN_DEBUG_SPI_REQ_SIGNING_SIGN = 0x0009, - + RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, + RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, + RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, + RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, + RAIDEN_DEBUG_SPI_REQ_ENABLE_H1 = 0x0004, + RAIDEN_DEBUG_SPI_REQ_RESET = 0x0005, + RAIDEN_DEBUG_SPI_REQ_BOOT_CFG = 0x0006, + RAIDEN_DEBUG_SPI_REQ_SOCKET = 0x0007, + RAIDEN_DEBUG_SPI_REQ_SIGNING_START = 0x0008, + RAIDEN_DEBUG_SPI_REQ_SIGNING_SIGN = 0x0009, + RAIDEN_DEBUG_SPI_REQ_ENABLE_AP_CUSTOM = 0x000a, };
#define PACKET_HEADER_SIZE (2) @@ -452,6 +452,23 @@ return 0; }
+static int get_ap_request_type(void) +{ + int ap_request = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP; + char *custom_rst_str = extract_programmer_param("custom_rst"); + if (custom_rst_str) { + if (!strcasecmp(custom_rst_str, "true")) + ap_request = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP_CUSTOM; + else { + msg_perr("Invalid custom rst param: %s\n", + custom_rst_str); + ap_request = -1; + } + } + free(custom_rst_str); + return ap_request; +} + static int get_target(void) { int request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE; @@ -459,7 +476,7 @@ char *target_str = extract_programmer_param("target"); if (target_str) { if (!strcasecmp(target_str, "ap")) - request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP; + request_enable = get_ap_request_type(); else if (!strcasecmp(target_str, "ec")) request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_EC; else { @@ -468,6 +485,7 @@ } } free(target_str); + msg_pinfo("Raiden target: %d\n", request_enable);
return request_enable; }
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/43527
to look at the new patch set (#2).
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
raiden_debug_spi: add param for a custom reset setup
Cr50 asserts EC_RST_L to flash the AP. Asserting EC_RST_L breaks flashing the AP on some boards. This change adds a custom_rst arg to tell cr50 not to assert any system reset signals during flash AP setup.
BUG=b:154885210 BRANCH=none TEST=manual
sudo flashrom -p raiden_debug:target=EC -r
sudo flashrom -p raiden_debug:target=AP -r
sudo flashrom -p raiden_debug:target=AP,custom_rst=true -r
sudo flashrom -p raiden_debug:target=AP,custom_rst=bogus -r
sudo flashrom -p raiden_debug -r
Signed-off-by: Mary Ruthven mruthven@chromium.org Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 --- M raiden_debug_spi.c 1 file changed, 30 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/43527/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/43527
to look at the new patch set (#3).
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
raiden_debug_spi: add param for a custom reset setup
Cr50 asserts EC_RST_L to flash the AP. Asserting EC_RST_L breaks flashing the AP on some boards. This change adds a custom_rst arg to tell cr50 not to assert any system reset signals during flash AP setup.
BUG=b:154885210 BRANCH=none TEST=manual
flashrom -p raiden_debug:target=EC -r ec.bin
flashrom -p raiden_debug:target=AP -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=true -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=inv -r ap.bin
flashrom -p raiden_debug -r base.bin
Signed-off-by: Mary Ruthven mruthven@chromium.org Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 --- M raiden_debug_spi.c 1 file changed, 30 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/43527/3
Brian Nemec has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43527 )
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
Patch Set 3: Code-Review+1
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43527 )
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG@9 PS3, Line 9: Cr50 `The Chromium TPM Cr50 asserts...`
Others may not know what a Cr50 is outside the Google ecosystem.
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG@10 PS3, Line 10: some boards. `some Chromium boards` also would be nice to elaborate here a little more, breaks how?
One possible suggestion for someone without context:
``` The Chromium TPM Cr50 asserts EC_RST_L while flashing the AP however this doesn't work on some Chromium boards depending on EC configuration. This adds a custom_rst spi master parameter to tell the Cr50 not to assert any system reset signals during a AP flashing. ```
https://review.coreboot.org/c/flashrom/+/43527/3/raiden_debug_spi.c File raiden_debug_spi.c:
https://review.coreboot.org/c/flashrom/+/43527/3/raiden_debug_spi.c@458 PS3, Line 458: tract_programmer_param("custom_rst"); Any new parameters should be documented in the man page as well.
Andrey Pronin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43527 )
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG@9 PS3, Line 9: Cr50
`The Chromium TPM Cr50 asserts...` […]
It's technically not a TPM, and haven't seen it being called a "chromium tpm" before. If cr50 doesn't work, we can go with "H1 firmware" or "Titan C firmware" (both are valid names for the chip, one older but still used, one newer marketing-sounding) or "GSC firmware"
Hello Brian Nemec, build bot (Jenkins), Brian Nemec, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/43527
to look at the new patch set (#4).
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
raiden_debug_spi: add param for a custom reset setup
The default Cr50 behavior is to assert EC_RST_L before programming the AP. Some Chromium devices don't power the AP flash when the EC is in reset. These boards can't flash the AP with the current CCD behavior. This change adds a custom_rst raiden_debug param to tell Cr50 not to assert EC_RST_L or touch any reset signals while flashing the AP. Users will need to configure the reset signals before running the flashrom command.
BUG=b:154885210 BRANCH=none TEST=manual
flashrom -p raiden_debug:target=EC -r ec.bin
flashrom -p raiden_debug:target=AP -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=true -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=inv -r ap.bin
flashrom -p raiden_debug -r base.bin
Signed-off-by: Mary Ruthven mruthven@chromium.org Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 --- M raiden_debug_spi.c 1 file changed, 30 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/43527/4
Hello Brian Nemec, build bot (Jenkins), Brian Nemec, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/43527
to look at the new patch set (#5).
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
raiden_debug_spi: add param for a custom reset setup
GSC firmware asserts EC_RST_L before programming the AP with raiden_debug:target=AP.Some Chromium devices don't power the AP flash when the EC is in reset. These boards can't flash the AP with the current CCD behavior. This change adds a custom_rst raiden_debug param to tell Cr50 not to assert EC_RST_L or touch any reset signals while flashing the AP. Users will need to configure the reset signals before running the flashrom command.
BUG=b:154885210 BRANCH=none TEST=manual
flashrom -p raiden_debug:target=EC -r ec.bin
flashrom -p raiden_debug:target=AP -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=true -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=inv -r ap.bin
flashrom -p raiden_debug -r base.bin
Signed-off-by: Mary Ruthven mruthven@chromium.org Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 --- M raiden_debug_spi.c 1 file changed, 30 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/43527/5
Mary Ruthven has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43527 )
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG@9 PS3, Line 9: Cr50
It's technically not a TPM, and haven't seen it being called a "chromium tpm" before. […]
Done
https://review.coreboot.org/c/flashrom/+/43527/3//COMMIT_MSG@10 PS3, Line 10: some boards.
`some Chromium boards` also would be nice to elaborate here a little more, breaks how? […]
Done
https://review.coreboot.org/c/flashrom/+/43527/3/raiden_debug_spi.c File raiden_debug_spi.c:
https://review.coreboot.org/c/flashrom/+/43527/3/raiden_debug_spi.c@458 PS3, Line 458: tract_programmer_param("custom_rst");
Any new parameters should be documented in the man page as well.
Brian will add a raiden_debug section to the man page
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43527 )
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
Patch Set 6: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/43527 )
Change subject: raiden_debug_spi: add param for a custom reset setup ......................................................................
raiden_debug_spi: add param for a custom reset setup
GSC firmware asserts EC_RST_L before programming the AP with raiden_debug:target=AP.Some Chromium devices don't power the AP flash when the EC is in reset. These boards can't flash the AP with the current CCD behavior. This change adds a custom_rst raiden_debug param to tell Cr50 not to assert EC_RST_L or touch any reset signals while flashing the AP. Users will need to configure the reset signals before running the flashrom command.
BUG=b:154885210 BRANCH=none TEST=manual
flashrom -p raiden_debug:target=EC -r ec.bin
flashrom -p raiden_debug:target=AP -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=true -r ap.bin
flashrom -p raiden_debug:target=AP,custom_rst=inv -r ap.bin
flashrom -p raiden_debug -r base.bin
Signed-off-by: Mary Ruthven mruthven@chromium.org Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 Reviewed-on: https://review.coreboot.org/c/flashrom/+/43527 Reviewed-by: Edward O'Callaghan quasisec@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M raiden_debug_spi.c 1 file changed, 30 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index f167a3a..bd7c054 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -399,17 +399,17 @@ };
enum raiden_debug_spi_request { - RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, - RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, - RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, - RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, - RAIDEN_DEBUG_SPI_REQ_ENABLE_H1 = 0x0004, - RAIDEN_DEBUG_SPI_REQ_RESET = 0x0005, - RAIDEN_DEBUG_SPI_REQ_BOOT_CFG = 0x0006, - RAIDEN_DEBUG_SPI_REQ_SOCKET = 0x0007, - RAIDEN_DEBUG_SPI_REQ_SIGNING_START = 0x0008, - RAIDEN_DEBUG_SPI_REQ_SIGNING_SIGN = 0x0009, - + RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, + RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, + RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, + RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, + RAIDEN_DEBUG_SPI_REQ_ENABLE_H1 = 0x0004, + RAIDEN_DEBUG_SPI_REQ_RESET = 0x0005, + RAIDEN_DEBUG_SPI_REQ_BOOT_CFG = 0x0006, + RAIDEN_DEBUG_SPI_REQ_SOCKET = 0x0007, + RAIDEN_DEBUG_SPI_REQ_SIGNING_START = 0x0008, + RAIDEN_DEBUG_SPI_REQ_SIGNING_SIGN = 0x0009, + RAIDEN_DEBUG_SPI_REQ_ENABLE_AP_CUSTOM = 0x000a, };
/* @@ -1407,6 +1407,23 @@ return 0; }
+static int get_ap_request_type(void) +{ + int ap_request = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP; + char *custom_rst_str = extract_programmer_param("custom_rst"); + if (custom_rst_str) { + if (!strcasecmp(custom_rst_str, "true")) + ap_request = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP_CUSTOM; + else { + msg_perr("Invalid custom rst param: %s\n", + custom_rst_str); + ap_request = -1; + } + } + free(custom_rst_str); + return ap_request; +} + static int get_target(void) { int request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE; @@ -1414,7 +1431,7 @@ char *target_str = extract_programmer_param("target"); if (target_str) { if (!strcasecmp(target_str, "ap")) - request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP; + request_enable = get_ap_request_type(); else if (!strcasecmp(target_str, "ec")) request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_EC; else { @@ -1423,6 +1440,7 @@ } } free(target_str); + msg_pinfo("Raiden target: %d\n", request_enable);
return request_enable; }