Attention is currently required from: Nikolai Artemiev, Kapil Porwal, Edward O'Callaghan, Anastasia Klimchuk.
Hello build bot (Jenkins), Nikolai Artemiev, Subrata Banik, Edward O'Callaghan, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70549
to look at the new patch set (#7).
Change subject: flashchips.c: Add reg_bits for W25Q256JW_DTR
......................................................................
flashchips.c: Add reg_bits for W25Q256JW_DTR
Add reg_bits for W25Q256JW_DTR as per the datasheet.
BUG=b:263410331
TEST=Verified on google/rex.
w/o this patch:
Failed to get WP status: WP operations are not implemented for this chip
w/ this patch:
flashrom -p internal --wp-range 0x0,0x2000000
flashrom -p internal --wp-enable
flashrom -p internal --wp-status
flashrom -p internal -E <---- failed to erase the flash as WP (which is
expected)
Signed-off-by: Kapil Porwal <kapilporwal(a)google.com>
Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367
---
M flashchips.c
1 file changed, 38 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/70549/7
--
To view, visit https://review.coreboot.org/c/flashrom/+/70549
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367
Gerrit-Change-Number: 70549
Gerrit-PatchSet: 7
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nikolai Artemiev, Kapil Porwal, Edward O'Callaghan, Anastasia Klimchuk.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70549 )
Change subject: flashchips.c: Add reg_bits for W25Q256JW_DTR
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/70549/comment/2d4cf085_81ebe98b
PS6, Line 14: Steps:
> WP commands cannot be executed without this CL. The tool throws below error -
> `Failed to get WP status: WP operations are not implemented for this chip`
may be something like this
w/o this patch:
Failed to get WP status: WP operations are not implemented for this chip
w/ this patch:
flashrom -p internal --wp-range 0x0,0x2000000
flashrom -p internal --wp-enable
flashrom -p internal --wp-status
flashrom -p internal -E <---- failed to erase the flash as WP (which is expected)
--
To view, visit https://review.coreboot.org/c/flashrom/+/70549
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367
Gerrit-Change-Number: 70549
Gerrit-PatchSet: 6
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 Feb 2023 14:00:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Artemiev, Subrata Banik, Edward O'Callaghan, Anastasia Klimchuk.
Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70549 )
Change subject: flashchips.c: Add reg_bits for W25Q256JW_DTR
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/70549/comment/839f30e6_1dc896e2
PS6, Line 14: Steps:
> w/ and w/o this CL test would have been better
WP commands cannot be executed without this CL. The tool throws below error -
`Failed to get WP status: WP operations are not implemented for this chip`
--
To view, visit https://review.coreboot.org/c/flashrom/+/70549
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367
Gerrit-Change-Number: 70549
Gerrit-PatchSet: 6
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 Feb 2023 13:11:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Artemiev, Kapil Porwal, Edward O'Callaghan, Anastasia Klimchuk.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70549 )
Change subject: flashchips.c: Add reg_bits for W25Q256JW_DTR
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/70549/comment/022f6e5b_bb571663
PS6, Line 14: Steps:
w/ and w/o this CL test would have been better
--
To view, visit https://review.coreboot.org/c/flashrom/+/70549
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367
Gerrit-Change-Number: 70549
Gerrit-PatchSet: 6
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 Feb 2023 13:08:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Artemiev, Subrata Banik, Edward O'Callaghan, Anastasia Klimchuk.
Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70549 )
Change subject: flashchips.c: Add reg_bits for W25Q256JW_DTR
......................................................................
Patch Set 5:
(1 comment)
This change is ready for review.
Patchset:
PS5:
I have verified this CL on google/rex. Please review.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70549
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367
Gerrit-Change-Number: 70549
Gerrit-PatchSet: 5
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 Feb 2023 12:52:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72408 )
Change subject: dummyflasher: use new API to register shutdown function
......................................................................
Patch Set 3:
(2 comments)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/72408/comment/f356e0ba_4b39433e
PS2, Line 79: uint8_t refs_cnt;
> Can you add a comment on this: to explain that dummyflasher can register multiple masters and they a […]
Done
https://review.coreboot.org/c/flashrom/+/72408/comment/a596b066_5a29ee0f
PS2, Line 1416: data->refs_cnt += ((dummy_buses_supported & BUS_PROG) != BUS_NONE);
: data->refs_cnt += ((dummy_buses_supported & BUS_NONSPI) != BUS_NONE);
: data->refs_cnt += ((dummy_buses_supported & BUS_SPI) != BUS_NONE);
:
: #if 0
: data->refs_cnt += (dummy_buses_supported & BUS_PROG) ? 1 : 0;
: data->refs_cnt += (dummy_buses_supported & BUS_NONSPI) ? 1 : 0;
: data->refs_cnt += (dummy_buses_supported & BUS_SPI) ? 1 : 0;
: #endif
> Another thing could be to add `&& !ret` to every condition. […]
Your last idea is exactly what we need. Thank you! Here we go :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/72408
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea
Gerrit-Change-Number: 72408
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 Feb 2023 08:37:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Alexander Goncharov.
Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/72408
to look at the new patch set (#3).
Change subject: dummyflasher: use new API to register shutdown function
......................................................................
dummyflasher: use new API to register shutdown function
This allows masters to register shutdown function in *_master
struct, which means there is no need to call register_shutdown in init
function, since this call is now a part of register_*_master.
A dummy programmer can register masters for multiple buses that share a
programmer's data (a pointer to struct emu_data) with each other. To
avoid unexpected memory freeing by shutdown function, we need to keep
track of how many buses are using the shared resource. Use the
reference counting technique to achieve this.
TEST=ninja test
Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
---
M dummyflasher.c
1 file changed, 43 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/72408/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/72408
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea
Gerrit-Change-Number: 72408
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Edward O'Callaghan.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71834 )
Change subject: internal: Move parallel logic into internal_par implementation
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
File internal_par.c:
https://review.coreboot.org/c/flashrom/+/71834/comment/d242db81_c9d35e53
PS1, Line 16:
> Actually it is a independent programmer and probably _should_ be registered just by itself.
Oh wow I didn't realise this :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/71834
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Idabeceb59a36680f5fbb45d3ee4bd5dbf837373b
Gerrit-Change-Number: 71834
Gerrit-PatchSet: 2
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 Feb 2023 02:10:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment