Change in flashrom[master]: ichspi: print transaction error in debug mode
Jonathan Zhang has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49001 ) Change subject: ichspi: print transaction error in debug mode ...................................................................... ichspi: print transaction error in debug mode When "--fmap" option is used, flashrom does binary search in flash for fmap. When it reads flash region that is locked, transaction error happens. Print transaction error in debug mode, so that following message: Transaction error between offset 0x02cb4000 and 0x02cb4007 (= 0x02cb4000 + 7)! shows up only when "-V" is specified. Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Signed-off-by: Jonathan Zhang <jonzhang@fb.com> --- M ichspi.c 1 file changed, 7 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/49001/1 diff --git a/ichspi.c b/ichspi.c index 4209d60..6dde7b2 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1311,11 +1311,17 @@ if (hsfs & HSFS_FCERR) { addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask; - msg_perr("Transaction error between offset 0x%08x and " + + /* + * If the partition is locked, this transaction error happens. + * Print in verbose mode to avoid excessive messages. + */ + msg_cdbg("Transaction error between offset 0x%08x and " "0x%08x (= 0x%08x + %d)!\n", addr, addr + len - 1, addr, len - 1); prettyprint_ich9_reg_hsfs(hsfs, ich_gen); prettyprint_ich9_reg_hsfc(REGREAD16(ICH9_REG_HSFC), ich_gen); + return 1; } return 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/49001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Gerrit-Change-Number: 49001 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jonzhang@fb.com> Gerrit-MessageType: newchange
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49001 ) Change subject: ichspi: print transaction error in debug mode ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Gerrit-Change-Number: 49001 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jonzhang@fb.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: insomniac <insomniac@slackware.it> Gerrit-Comment-Date: Wed, 30 Dec 2020 08:39:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49001 ) Change subject: ichspi: print transaction error in debug mode ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/49001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Gerrit-Change-Number: 49001 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jonzhang@fb.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: insomniac <insomniac@slackware.it> Gerrit-Comment-Date: Wed, 30 Dec 2020 08:40:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49001 ) Change subject: ichspi: print transaction error in debug mode ...................................................................... Patch Set 1: Code-Review+1 Showing this as an error when --fmap is not used is useful however... -- To view, visit https://review.coreboot.org/c/flashrom/+/49001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Gerrit-Change-Number: 49001 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jonzhang@fb.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: insomniac <insomniac@slackware.it> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 30 Dec 2020 09:27:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49001 ) Change subject: ichspi: print transaction error in debug mode ...................................................................... Patch Set 1: Code-Review-1 Is there any reason to hide this error? It *is* an error. -- To view, visit https://review.coreboot.org/c/flashrom/+/49001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Gerrit-Change-Number: 49001 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jonzhang@fb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: insomniac <insomniac@slackware.it> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 01 Jan 2021 00:00:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49001 ) Change subject: ichspi: print transaction error in debug mode ...................................................................... Patch Set 1:
Patch Set 1: Code-Review-1
Is there any reason to hide this error? It *is* an error.
When "--fmap" option is used, flashrom does binary search in flash for fmap. It is expected that this binary search operation may try to read region that is locked; in this case, flashrom should skip this region without error message, but instead print such message only when "-V" is specified. Otherwise, user may think there is a legitimate error. For context, please check https://review.coreboot.org/cgit/flashrom.git/tree/fmap.c#n229. That being said, as ich_hwseq_wait_for_cycle_complete() is called for many other cases as well, that we do need transaction error to be treated as error. Is there a way to understand from the status register whether the region is locked? If so, we can treat it as warning only if the region is locked. If not, one method to narrow down is to treat transaction error as warning only when ich_hwseq_wait_for_cycle_complete() is called by ich_hwseq_read(), but not by ich_hwseq_erase() and ich_hwseq_write(). Feedback/suggestions are appreciated. -- To view, visit https://review.coreboot.org/c/flashrom/+/49001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Gerrit-Change-Number: 49001 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jonzhang@fb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: insomniac <insomniac@slackware.it> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 01 Jan 2021 04:08:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/49001?usp=email ) Change subject: ichspi: print transaction error in debug mode ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/flashrom/+/49001?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iebed1ae7f15b7f06b90a0b5947b953f7f0e1567d Gerrit-Change-Number: 49001 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: insomniac <insomniac@slackware.it> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org>
participants (5)
-
Anastasia Klimchuk (Code Review) -
Angel Pons (Code Review)
-
Arthur Heymans (Code Review)
-
Frans Hendriks (Code Review)
-
Jonathan Zhang (Code Review)