Change in flashrom[master]: WIP: spi25_statusreg.c: restore SR contents at flashrom exit
Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/48778 )
Change subject: WIP: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
WIP: spi25_statusreg.c: restore SR contents at flashrom exit
spi_disable_blockprotect_generic() now uses register_chip_restore() to reset the chip's status register at flashrom exit.
Imported from cros flashrom at `b170dd4e1d5c33b169c5`
Change-Id: If2f0e73518d40519b7569f627c90a34c364df47c Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M spi25_statusreg.c 1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/48778/1
diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 34f9ad4..5dda912 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -108,6 +108,12 @@ return readarr[0]; }
+static int spi_restore_status(struct flashctx *flash, uint8_t status) +{ + msg_cdbg("restoring chip status (0x%02x)\n", status); + return spi_write_status_register(flash, status); +} + /* A generic block protection disable. * Tests if a protection is enabled with the block protection mask (bp_mask) and returns success otherwise. * Tests if the register bits are locked with the lock_mask (lock_mask). @@ -139,6 +145,9 @@ return 0; }
+ /* restore status register content upon exit */ + register_chip_restore(spi_restore_status, flash, status); + msg_cdbg("Some block protection in effect, disabling... "); if ((status & lock_mask) != 0) { msg_cdbg("\n\tNeed to disable the register lock first... ");
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48778
to look at the new patch set (#2).
Change subject: WIP: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
WIP: spi25_statusreg.c: restore SR contents at flashrom exit
register_chip_restore() provides a general mechanism for restoring a chip's state at flashrom exit; it can be used whenever the SR needs to be changed temporarily to perform some operation and then be changed back after the operation is complete. The only current current use case is in s25f.c, which changes the SR's sector layout bits so that entire flash accessible.
This patch uses the chip restore functionality to reset changes to the status register made by spi_disable_blockprotect_generic(). This is probably the safer than leaving the SR bits modified, as the flash chip will usually be accessed by some external controller that may make unknown assumptions about the SR.
Imported from cros flashrom at `b170dd4e1d5c33b169c5`
Change-Id: If2f0e73518d40519b7569f627c90a34c364df47c Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M spi25_statusreg.c 1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/48778/2
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48778
to look at the new patch set (#3).
Change subject: WIP: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
WIP: spi25_statusreg.c: restore SR contents at flashrom exit
register_chip_restore() provides a general mechanism for restoring a chip's state at flashrom exit; it can be used whenever the SR needs to be changed temporarily to perform some operation and changed back after the operation is complete. The only current current use case is in s25f.c, which changes the SR's sector layout bits so that entire flash accessible.
This patch uses the chip restore functionality to reset changes to the status register made by spi_disable_blockprotect_generic(). This is probably the safer than leaving the SR bits modified, as the flash chip will usually be accessed by some external controller that may make unknown assumptions about the SR.
Imported from cros flashrom at `b170dd4e1d5c33b169c5`
Change-Id: If2f0e73518d40519b7569f627c90a34c364df47c Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M spi25_statusreg.c 1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/48778/3
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48778
to look at the new patch set (#4).
Change subject: WIP: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
WIP: spi25_statusreg.c: restore SR contents at flashrom exit
register_chip_restore() provides a general mechanism for restoring a chip's state at flashrom exit; it can be used whenever the SR needs to be changed temporarily to perform some operation and changed back after the operation is complete. The only current current use case is in s25f.c, which changes the SR's sector layout bits so that entire flash accessible.
This patch uses the chip restore functionality to reset changes to the status register made by spi_disable_blockprotect_generic(). This is probably the safest approach, as the flash chip will usually be accessed by some external controller that may make unknown assumptions about state of the SR.
Imported from cros flashrom at `b170dd4e1d5c33b169c5`
Change-Id: If2f0e73518d40519b7569f627c90a34c364df47c Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M spi25_statusreg.c 1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/48778/4
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48778
to look at the new patch set (#5).
Change subject: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
spi25_statusreg.c: restore SR contents at flashrom exit
register_chip_restore() provides a general mechanism for restoring a chip's state at flashrom exit; it can be used whenever the SR needs to be changed temporarily to perform some operation and changed back after the operation is complete. The only current current use case is in s25f.c, which changes the SR's sector layout bits so that entire flash accessible.
This patch uses the chip restore functionality to reset changes to the status register made by spi_disable_blockprotect_generic(). This is probably the safest approach, as the flash chip will usually be accessed by some external controller that may make unknown assumptions about state of the SR.
Imported from cros flashrom at `b170dd4e1d5c33b169c5`
Change-Id: If2f0e73518d40519b7569f627c90a34c364df47c Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M spi25_statusreg.c 1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/48778/5
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48778 )
Change subject: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
Patch Set 5: Code-Review+2
(2 comments)
https://review.coreboot.org/c/flashrom/+/48778/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/48778/5//COMMIT_MSG@11 PS5, Line 11: . ` upon the second run of Flashrom.` I guess is what you were saying here?
https://review.coreboot.org/c/flashrom/+/48778/5/spi25_statusreg.c File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/48778/5/spi25_statusreg.c@148 PS5, Line 148: ` in finalize_flash_access(). */`
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48778
to look at the new patch set (#6).
Change subject: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
spi25_statusreg.c: restore SR contents at flashrom exit
register_chip_restore() provides a general mechanism for restoring a chip's state at flashrom exit; it can be used whenever the SR needs to be changed temporarily to perform some operation and changed back after the operation is complete. The only current current use case is in s25f.c, which changes the SR's sector layout bits so that entire flash accessible.
This patch uses the chip restore functionality to reset changes to the status register made by spi_disable_blockprotect_generic(). This should help to ensure consistency across multiple runs of flashrom and make it easier to predict how a specific operation will change the flash.
Imported from cros flashrom at `b170dd4e1d5c33b169c5`
Change-Id: If2f0e73518d40519b7569f627c90a34c364df47c Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M spi25_statusreg.c 1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/48778/6
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48778 )
Change subject: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/flashrom/+/48778/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/48778/5//COMMIT_MSG@11 PS5, Line 11: .
` upon the second run of Flashrom. […]
I think I meant something like other flashing tools or hardware flash controllers might expect a specific status register configuration, but those probably wouldn't care about the block protect bits too much. I've replaced that sentence with some better reasons for the change.
https://review.coreboot.org/c/flashrom/+/48778/5/spi25_statusreg.c File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/48778/5/spi25_statusreg.c@148 PS5, Line 148:
` in finalize_flash_access(). […]
Done
Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48778 )
Change subject: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
Patch Set 6: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/48778 )
Change subject: spi25_statusreg.c: restore SR contents at flashrom exit ......................................................................
spi25_statusreg.c: restore SR contents at flashrom exit
register_chip_restore() provides a general mechanism for restoring a chip's state at flashrom exit; it can be used whenever the SR needs to be changed temporarily to perform some operation and changed back after the operation is complete. The only current current use case is in s25f.c, which changes the SR's sector layout bits so that entire flash accessible.
This patch uses the chip restore functionality to reset changes to the status register made by spi_disable_blockprotect_generic(). This should help to ensure consistency across multiple runs of flashrom and make it easier to predict how a specific operation will change the flash.
Imported from cros flashrom at `b170dd4e1d5c33b169c5`
Change-Id: If2f0e73518d40519b7569f627c90a34c364df47c Signed-off-by: Nikolai Artemiev nartemiev@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/48778 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M spi25_statusreg.c 1 file changed, 9 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 34f9ad4..a0b0fcf 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -108,6 +108,12 @@ return readarr[0]; }
+static int spi_restore_status(struct flashctx *flash, uint8_t status) +{ + msg_cdbg("restoring chip status (0x%02x)\n", status); + return spi_write_status_register(flash, status); +} + /* A generic block protection disable. * Tests if a protection is enabled with the block protection mask (bp_mask) and returns success otherwise. * Tests if the register bits are locked with the lock_mask (lock_mask). @@ -139,6 +145,9 @@ return 0; }
+ /* Restore status register content upon exit in finalize_flash_access(). */ + register_chip_restore(spi_restore_status, flash, status); + msg_cdbg("Some block protection in effect, disabling... "); if ((status & lock_mask) != 0) { msg_cdbg("\n\tNeed to disable the register lock first... ");
-
Edward O'Callaghan (Code Review) -
Nikolai Artemiev (Code Review)