Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/64352 )
Change subject: util/shell.nix: Add libjaylink
......................................................................
util/shell.nix: Add libjaylink
libjaylink is packaged since NixOS 21.11 and it is out for many months
now. Thus, include the package libjaylink and remove comments.
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85
…
[View More]Reviewed-on: https://review.coreboot.org/c/flashrom/+/64352
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M util/shell.nix
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Paul Menzel: Looks good to me, but someone else must approve
Thomas Heijligen: Looks good to me, approved
diff --git a/util/shell.nix b/util/shell.nix
index d6ddf81..906e1fc 100644
--- a/util/shell.nix
+++ b/util/shell.nix
@@ -6,7 +6,7 @@
buildInputs = [
cmocka
libftdi1
-# libjaylink # Will be added in NixOS 21.11
+ libjaylink
libusb1
meson
ninja
--
To view, visit https://review.coreboot.org/c/flashrom/+/64352
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85
Gerrit-Change-Number: 64352
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
[View Less]
Attention is currently required from: Felix Singer, Richard Hughes, Daniel Campello, Angel Pons, Patrick Rudolph.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
Patch Set 22: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing …
[View More]mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 22
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-CC: Richard Hughes <hughsient(a)gmail.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 19 May 2022 16:14:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
[View Less]
Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64488 )
Change subject: [RFC] dummyflasher: Wire variable size feature via opaque infra
......................................................................
Patch Set 1:
(9 comments)
Patchset:
PS1:
If it works with the requirements, I think an opaque master is the way to go!
File …
[View More]dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/64488/comment/283d36d5_4d5f35bb
PS1, Line 129: return 0;
Seems unnecessary. If it's not our master, why should we get called?
https://review.coreboot.org/c/flashrom/+/64488/comment/1aeb6ade_2830ab2f
PS1, Line 133: * Once that happens, we need to have special hacks in functions:
For opaque masters this should never happen (they all have to fill the
size). Or if it ever happens, it would have to be part of an compatible
API change anyway.
https://review.coreboot.org/c/flashrom/+/64488/comment/72ad7e73_cf0a9114
PS1, Line 146: flash->chip->feature_bits = FEATURE_4BA_READ | FEATURE_4BA_WRITE;
These are SPI specific and wouldn't be needed if we don't mix opaque
and SPI functions. More about that below.
https://review.coreboot.org/c/flashrom/+/64488/comment/a8fbbb67_ceca2014
PS1, Line 153: for (i = 0; i < NUM_ERASEFUNCTIONS; i++) {
There is only one by definition (opaque entry in flashchips.c).
https://review.coreboot.org/c/flashrom/+/64488/comment/3d3c4644_901313d7
PS1, Line 865: .read = default_spi_read,
: .write = dummy_spi_write_256,
: .erase = spi_block_erase_c7,
It might make reasoning about the code much simpler to not re-use
the SPI functions.
Unless there are specific requirements, all an opaque dummy should
need is memcpy() for read/write and memset() for erase. Wrapping these
into three functions (e.g. dummy_opaque_read(), dummy_opaque_write(),
dummy_opaque_erase()) would add little boilerplate but make it clear
what is supposed to happen.
https://review.coreboot.org/c/flashrom/+/64488/comment/68971faf_fbc6af8f
PS1, Line 906: free(bustext);
Related to the very last comment below, we might want to update this.
AFAICS, emulation of masters and chips was kept separately so far.
Currently one can do odd things like this:
$ flashrom -p dummy:emulate=W25Q128FV,bus=parallel
...
No EEPROM/flash device found.
Seems stupid, but it's actually a test that the master infrastructure
works as expected :)
https://review.coreboot.org/c/flashrom/+/64488/comment/67e28289_d38d3c1d
PS1, Line 1269: rmst.spi = *s_mst;
: rmst.opaque = *o_mst;
It's not supposed to work like this. The .spi and .opaque fields were
originally part of a union, i.e. mutually exclusive. That's not visible
anymore because the code got a little obfuscated by CB:50246.
AIUI, register_master() is not supposed to be called by programmer drivers
directly. They should go through the specific register_*_master() functions.
I guess that your tests worked out shows that we actually don't need a SPI
master and register_opaque_master() should suffice.
If we need both masters, they can be registered individually.
https://review.coreboot.org/c/flashrom/+/64488/comment/7bca48fa_a5c1209d
PS1, Line 1346: } else {
It doesn't have to be mutually exclusive. We could just add all possible
masters (like done for NONSPI and SPI below). The probe function would be
responsible to check for `emu_chip == EMULATE_VARIABLE_SIZE`.
--
To view, visit https://review.coreboot.org/c/flashrom/+/64488
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I76402bfdf8b1a75489e4509fec92c9a777d0cf58
Gerrit-Change-Number: 64488
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Namyoon Woo <namyoon(a)google.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Thu, 19 May 2022 12:47:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[View Less]
Attention is currently required from: Felix Singer.
Jacek Naglak has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64440 )
Change subject: ft2232_spi.c: Add support for kt-link jtag interface
......................................................................
Patch Set 4:
(4 comments)
Patchset:
PS4:
Thanks
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/64440/comment/679d7838_34ad347f
PS1, Line 80: {FTDI_VID, KT_LINK_PID, OK, "Kristech", "…
[View More]KT-LINK"},
> Move this line to the other FTDIs
Done, thanks missed that.
https://review.coreboot.org/c/flashrom/+/64440/comment/e8e35c04_b99460e1
PS1, Line 671:
> remove space
Done
https://review.coreboot.org/c/flashrom/+/64440/comment/ad81f2b7_8e452310
PS1, Line 684:
> remove space
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/64440
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id58c675bc410ec3ef6d58603d13efc9ca53bb87c
Gerrit-Change-Number: 64440
Gerrit-PatchSet: 4
Gerrit-Owner: Jacek Naglak <jnaglak(a)tlen.pl>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Thu, 19 May 2022 10:28:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
[View Less]
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64499 )
Change subject: Add reviewers group to allow +1, -1 changes.
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
I'm not convinced that this is going to do everything that is desired. I suspect that permissions given by the 'all projects' group are still going to be active unless negated here. We can discuss that in the flashrom …
[View More]meeting and see if more changes are desired.
--
To view, visit https://review.coreboot.org/c/flashrom/+/64499
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: refs/meta/config
Gerrit-Change-Id: I6af85c58d9a8c37c8bf17181c83720332a474cc8
Gerrit-Change-Number: 64499
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com>
Gerrit-Comment-Date: Thu, 19 May 2022 05:50:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[View Less]
Martin L Roth has uploaded a new patch set (#2). ( https://review.coreboot.org/c/flashrom/+/64499 )
Change subject: Add reviewers group to allow +1, -1 changes.
......................................................................
Add reviewers group to allow +1, -1 changes.
Change-Id: I6af85c58d9a8c37c8bf17181c83720332a474cc8
---
M groups
M project.config
2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/64499/2
--
To …
[View More]view, visit https://review.coreboot.org/c/flashrom/+/64499
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: refs/meta/config
Gerrit-Change-Id: I6af85c58d9a8c37c8bf17181c83720332a474cc8
Gerrit-Change-Number: 64499
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com>
Gerrit-MessageType: newpatchset
[View Less]
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64499 )
Change subject: Review access change
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Just add the reviewers group to allow +1, -1 changes
--
To view, visit https://review.coreboot.org/c/flashrom/+/64499
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
…
[View More]Gerrit-Branch: refs/meta/config
Gerrit-Change-Id: I6af85c58d9a8c37c8bf17181c83720332a474cc8
Gerrit-Change-Number: 64499
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com>
Gerrit-Comment-Date: Thu, 19 May 2022 05:46:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[View Less]