Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63724 )
Change subject: [WIP] meson: rework the programmer selection
......................................................................
Patch Set 2:
(4 comments)
File meson.build:
https://review.coreboot.org/c/flashrom/+/63724/comment/512ff217_b915376b
PS2, Line 98: # 'deps' : list[dep], # fallback: []
Could each dep list additional files that it requires? It looks like every programmer that depends on libpci also needs pcidev.c, so it might be simpler to have the libpci dep specify that pcidev.c be built.
https://review.coreboot.org/c/flashrom/+/63724/comment/e11d4653_fab49dcc
PS2, Line 418: p_x.get('deps',[])
deps is already defaulted to [] above, so this doesn't need to specify a fallback.
https://review.coreboot.org/c/flashrom/+/63724/comment/35091270_ac6ab336
PS2, Line 427: elif selected and available
Seems clearer to nest these, since it isn't immediately obvious that this is the same condition as above but minus `deps_found`
if selected and available
if deps_found
active = true
else
error(...)
`selected_all and available` below could do the same.
https://review.coreboot.org/c/flashrom/+/63724/comment/68edf339_ebacc37f
PS2, Line 464: if host_machine.system() in systems_serial_support
Could we avoid building this if no programmer needs serial? Might need a bit of rework to express it as a kind of dependency.
--
To view, visit https://review.coreboot.org/c/flashrom/+/63724
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib44b26e3748fc71f116184082b4aed0bb208b4c1
Gerrit-Change-Number: 63724
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-CC: Richard Hughes <richard(a)hughsie.com>
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-Comment-Date: Thu, 21 Apr 2022 00:25:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63724 )
Change subject: [WIP] meson: rework the programmer selection
......................................................................
Patch Set 2:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/flashrom/+/63724
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib44b26e3748fc71f116184082b4aed0bb208b4c1
Gerrit-Change-Number: 63724
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-CC: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 20 Apr 2022 19:42:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Richard Hughes, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Patrick Rudolph, Peter Marheine.
Daniel Campello 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 9:
(2 comments)
Patchset:
PS4:
> I think the context of adding support to programmers is useful to understand the needs of the framew […]
Ack
File 82802ab.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/8b1cdcaa_67209e22
PS8, Line 137: set_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len);
> Updating progress on every byte seems pretty costly. […]
Does it make sense to switch the callback method to take a `int percentage` instead of current and total and have the callback be called max 100 times instead on each byte update?
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 9
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: build bot (Jenkins) <no-reply(a)coreboot.org>
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-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Tue, 19 Apr 2022 17:21:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Richard Hughes, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Patrick Rudolph.
Daniel Campello has uploaded a new patch set (#9) to the change originally created by Richard Hughes. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
libflashrom: Return progress state to the library user
Projects using libflashrom like fwupd expect the user to wait for the
operation to complete. To avoid the user thinking the process has
"hung" or "got stuck" report back the progress complete of the erase,
write and read operations.
Include a test for the dummy spi25 device.
Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Signed-off-by: Richard Hughes <richard(a)hughsie.com>
---
M 82802ab.c
M at45db.c
M cli_classic.c
M cli_output.c
M dediprog.c
M en29lv640b.c
M flash.h
M it87spi.c
M jedec.c
M libflashrom.c
M libflashrom.h
M linux_mtd.c
M lspcon_i2c_spi.c
M realtek_mst_i2c_spi.c
M spi.c
M spi25.c
M sst28sf040.c
M tests/spi25.c
M tests/tests.c
M tests/tests.h
20 files changed, 145 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/49643/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 9
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: build bot (Jenkins) <no-reply(a)coreboot.org>
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-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60074 )
Change subject: build: Remove cli_classic need for internal symbols
......................................................................
Patch Set 9: Code-Review-1
(1 comment)
Patchset:
PS9:
`cli_classic` still needs the internal symbols of `libflashrom`. But we can use the static library to link against. Unit `cli_classic` uses only the `libflashrom.h` interface it's still a long way to go.
The code changes and commit message are unrelated to each other.
Yes, we could remove `flashrom.c` there. It's also part of `srcs`.
--
To view, visit https://review.coreboot.org/c/flashrom/+/60074
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib04293c14f53c8e798c7d3e35483067520668161
Gerrit-Change-Number: 60074
Gerrit-PatchSet: 9
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 19 Apr 2022 16:19:58 +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/+/60074
to look at the new patch set (#8).
Change subject: build: Remove cli_classic need for internal symbols
......................................................................
build: Remove cli_classic need for internal symbols
cli_classic should now only use the libflashrom interface.
BUG=b:208132085
TEST=`make && meson/ninja`.
Change-Id: Ib04293c14f53c8e798c7d3e35483067520668161
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M meson.build
1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/60074/8
--
To view, visit https://review.coreboot.org/c/flashrom/+/60074
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib04293c14f53c8e798c7d3e35483067520668161
Gerrit-Change-Number: 60074
Gerrit-PatchSet: 8
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset