Attention is currently required from: Edward O'Callaghan, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69517 )
Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
......................................................................
Patch Set 14:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/69517/comment/c9661551_f2c5e6ab
PS13, Line 1931: restore_flash_wp(flash, initial_wp_cfg);
> Indirectly calling `flashrom_wp_write_cfg` is intentional here? I guess it won't hurt, just looks w […]
Good point, I hadn't looked at this CL in a while and I thought we needed to restore in this case since the restore function won't be called if registration fails.
But this all happens before we disable flash protection in `unlock_flash_wp()` so we just need to free the reference to `initial_wp_cfg` like you said in your first comment.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69517
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f
Gerrit-Change-Number: 69517
Gerrit-PatchSet: 14
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Wed, 25 Jan 2023 00:34:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev.
Nikolai Artemiev has uploaded a new patch set (#14) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/69517 )
Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
......................................................................
flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
The full writeprotect implementation has proper support and
ability to unlock flash over spi25_statusreg.c. Therefore if
the required bits are available for the given chip prefer
proper writeprotect support instead of adhoc spi25_statusreg.c
helpers.
BUG=b:237485865
BRANCH=none
TEST=Tested on grunt DUT (prog: sb600spi, flash: W25Q128.W):
`flashrom --wp-range 0x0,0x1000000 \
flashrom --wp-status # Result: range=0x0,0x1000000 \
flashrom -w random.bin # Result: success \
flashrom -v random.bin # Result: success \
flashrom --wp-status # Result: range=0x0,0x1000000`
TEST=Tested that chips without WP support can still be unlocked
by deleting decode_range for W25Q128.W flashchip and
retesting on the grunt DUT.
Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f
CoAuthored-by: Nikolai Artemiev <nartemiev(a)google.com>
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M flashrom.c
1 file changed, 86 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/69517/14
--
To view, visit https://review.coreboot.org/c/flashrom/+/69517
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f
Gerrit-Change-Number: 69517
Gerrit-PatchSet: 14
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72393 )
Change subject: MAINTAINERS: Add Alexander Goncharov for bash completion
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/72393
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5cdae05a46dbd61060ff2b84accb9297334a6301
Gerrit-Change-Number: 72393
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Tue, 24 Jan 2023 22:45:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev.
Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69517 )
Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
......................................................................
Patch Set 13: Code-Review+1
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/69517/comment/aed41875_d43267f2
PS13, Line 1931: restore_flash_wp(flash, initial_wp_cfg);
Indirectly calling `flashrom_wp_write_cfg` is intentional here? I guess it won't hurt, just looks weird to read configuration and then write it back.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69517
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f
Gerrit-Change-Number: 69517
Gerrit-PatchSet: 13
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 24 Jan 2023 15:51:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/72160
to look at the new patch set (#2).
Change subject: ni845x_spi: refactor singleton states into reentrant pattern
......................................................................
ni845x_spi: refactor singleton states into reentrant pattern
Move global singleton states into a struct and store within
the spi_master data field for the life-time of the driver.
TOPIC=register_master_api
Change-Id: I45fcb8e20582cb0c532c4a9f0c78543a25f8d484
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
Ticket: https://ticket.coreboot.org/issues/391
---
M ni845x_spi.c
1 file changed, 75 insertions(+), 32 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/72160/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/72160
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I45fcb8e20582cb0c532c4a9f0c78543a25f8d484
Gerrit-Change-Number: 72160
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/72159
to look at the new patch set (#2).
Change subject: ni845x_spi: introduce a separate func for cleaning up
......................................................................
ni845x_spi: introduce a separate func for cleaning up
Shutdown function of programmer shouldn't be used as a clean-up
routine. This is due to the fact that the programmer's data is
passed there, ready only at the end of the initialization.
TOPIC=register_master_api
Change-Id: I8b32129b8d0dff9a7516973893fa4bf92f9e64db
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
Ticket: https://ticket.coreboot.org/issues/391
---
M ni845x_spi.c
1 file changed, 25 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/72159/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/72159
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8b32129b8d0dff9a7516973893fa4bf92f9e64db
Gerrit-Change-Number: 72159
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Anastasia Klimchuk.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72393 )
Change subject: MAINTAINERS: Add Alexander Goncharov for bash completion
......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/72393/comment/9b36e4a1_a895e785
PS1, Line 7: maintainers
> Uppercase: it's the name of the file.
Done
File MAINTAINERS:
https://review.coreboot.org/c/flashrom/+/72393/comment/7d4ac4f3_f43a03d1
PS1, Line 156: BASH COMPLETION
> Your entry should go first (abc order). […]
Got it! Done.
--
To view, visit https://review.coreboot.org/c/flashrom/+/72393
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5cdae05a46dbd61060ff2b84accb9297334a6301
Gerrit-Change-Number: 72393
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 24 Jan 2023 10:53:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Alexander Goncharov.
Hello Felix Singer, build bot (Jenkins), Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/72393
to look at the new patch set (#3).
Change subject: MAINTAINERS: Add Alexander Goncharov for bash completion
......................................................................
MAINTAINERS: Add Alexander Goncharov for bash completion
Change-Id: I5cdae05a46dbd61060ff2b84accb9297334a6301
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
---
M MAINTAINERS
1 file changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/72393/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/72393
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5cdae05a46dbd61060ff2b84accb9297334a6301
Gerrit-Change-Number: 72393
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Alexander Goncharov.
Hello Felix Singer, build bot (Jenkins), Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/72393
to look at the new patch set (#2).
Change subject: MAINTAINERS: Add Alexander Goncharov for bash completion
......................................................................
MAINTAINERS: Add Alexander Goncharov for bash completion
Change-Id: I5cdae05a46dbd61060ff2b84accb9297334a6301
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
---
M MAINTAINERS
1 file changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/72393/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/72393
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5cdae05a46dbd61060ff2b84accb9297334a6301
Gerrit-Change-Number: 72393
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: newpatchset