Attention is currently required from: Anastasia Klimchuk, Malcolm Boyes, Nikolai Artemiev, Stefan Reinauer.
Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82259?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Anastasia Klimchuk, Verified+1 by build bot (Jenkins)
Change subject: flashchips: Add support for Boya B25Q64AS
......................................................................
flashchips: Add support for Boya B25Q64AS
The B25Q64AS has been tested by ch341a programmer: read, write, erase
Datasheet: https://archive.org/details/1912111437-boyamicro-by-25-q-64-assig-c-383793
Change-Id: I05ecf2b118902db974544d86e023a348912371dd
Signed-off-by: boyesm <boyesmalcolm(a)gmail.com>
---
M flashchips.c
M include/flashchips.h
2 files changed, 39 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/82259/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/82259?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I05ecf2b118902db974544d86e023a348912371dd
Gerrit-Change-Number: 82259
Gerrit-PatchSet: 2
Gerrit-Owner: Malcolm Boyes <boyesmalcolm(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Malcolm Boyes <boyesmalcolm(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Paul Menzel, ZhiYuanNJ.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email )
Change subject: ch347_spi: Add driver support for CH347F packaging
......................................................................
Patch Set 7:
(1 comment)
File ch347_spi.c:
PS2:
> Done
Yes, please move the code for the device speed settings to a new commit and push another patch to Gerrit.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82193?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 7
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: ZhiYuanNJ
Gerrit-Comment-Date: Sun, 12 May 2024 17:26:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ZhiYuanNJ
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82162?usp=email )
Change subject: flashrom_udev.rules: Add rule for CH347
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Nicholas, just wanted to check, is this patch ready, can I submit?
Yes, this patch can be submitted.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82162?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ia83fa08f6d7c2f449b1a5c0c387c6d4368b99e3a
Gerrit-Change-Number: 82162
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 12 May 2024 15:59: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: Nikolai Artemiev, Stefan Reinauer, Victor Lim.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82332?usp=email )
Change subject: flashchips: Add (or update) chip models <list models>
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/82332/comment/a4cc4023_cc188528 :
PS1, Line 7: Add (or update) chip models <list models>
For this commit you can be more specific, because you know what model you adding. So instead the title (title is first line) should be:
`flashchips: Add support for GD25LF128E`
The rest of commit message is all good, keep it.
Patchset:
PS1:
Great, you are getting closer, this patch is almost there! I added two comments what you need to do.
If you have questions, you can reply to comments (Reply button on the comment and then Reply blue button on the top).
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/82332/comment/5af4ddef_435fc060 :
PS1, Line 6506: .model_id = GIGADEVICE_GD25LF128E,
This macro should be added in the same patch as chip definition.
So you need to add to this commit a change in include/flashchips.h , one line adding this macro for this model ID.
I can see you made CB:82190 with all the model IDs added, but you will need to add IDs in the same patch where the chip definition is added.
As an example, if you remember the other patch that was discussed on the mailing list: CB:58025. It's an example, but you can see it adds two chip definitions in flashchips.c, and two model IDs in flashchips.h.
In this patch, you will have one definition and one new model ID for it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82332?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I71fdc7ea1aea69d14db6af3bac2da3e7bee8abbe
Gerrit-Change-Number: 82332
Gerrit-PatchSet: 1
Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Sun, 12 May 2024 13:33:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Urja Rannikko.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/19616?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: Add a generic 27C256 definition (32k UV EPROM)
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Is anyone still interested in this patch? (and it's on staging branch)
--
To view, visit https://review.coreboot.org/c/flashrom/+/19616?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Change-Id: Ie726ff252bc398d78e76bfb5ed01359341b734f3
Gerrit-Change-Number: 19616
Gerrit-PatchSet: 1
Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Philippe Mathieu-Daudé <f4bug(a)amsat.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Comment-Date: Sun, 12 May 2024 11:27:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: David Hendricks.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/19859?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: flashchips: use 4BA direct erase functions for MX25L256
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Is anyone still interested in this patch? (and it's on staging branch)
--
To view, visit https://review.coreboot.org/c/flashrom/+/19859?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Change-Id: Ib3694eee96b5c082924fd3a9f63ea92830f896ce
Gerrit-Change-Number: 19859
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Comment-Date: Sun, 12 May 2024 11:26:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82231?usp=email )
Change subject: flashrom-tester: Include flashrom/src/cmd.rs tests in Cargo workspace
......................................................................
flashrom-tester: Include flashrom/src/cmd.rs tests in Cargo workspace
Ensure ChromeOS ebuild (ecargo_test) runs all unit tests, including
those under flashrom/src/cmd.rs which were previously being skipped due
to not being in the default Cargo workspace.
By adding flashrom/ to the [workspace] section of Cargo.toml, these
tests will now be consistently included when building and testing
flashrom-tester on ChromeOS.
References:
* ebuild of flashrom-tester: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/…
* ecarg_test: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/…
BUG=b:338962302
TEST=(ChromeOS)
FEATURES=`test emerge-corsola flashrom-tester`
Could see tests like `cmd::tests::decode_io_opt ... ok`
TEST=(UPSTREAM)
1. Build flashrom by `meson`
2. Build bindings/rust/libflashrom by `cargo build`
3. Build util/flashrom_tester by
`cargo build`
`cargo test --workspace`
Could see tests like `cmd::tests::decode_io_opt ... ok`
Change-Id: Ic23bc35592e6d7d8dd24c71630ea9a2eb2d58573
Signed-off-by: Hsuan Ting Chen <roccochen(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/82231
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M util/flashrom_tester/Cargo.toml
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
Angel Pons: Looks good to me, but someone else must approve
Anastasia Klimchuk: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml
index a76a5b4..50c73dd 100644
--- a/util/flashrom_tester/Cargo.toml
+++ b/util/flashrom_tester/Cargo.toml
@@ -15,6 +15,9 @@
name = "flashrom_tester"
required-features = ["cli"]
+[workspace]
+members = [".", "flashrom"]
+
[dependencies]
atty = "0.2"
built = { version = "0.5", features = ["chrono"] }
--
To view, visit https://review.coreboot.org/c/flashrom/+/82231?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ic23bc35592e6d7d8dd24c71630ea9a2eb2d58573
Gerrit-Change-Number: 82231
Gerrit-PatchSet: 5
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com>
Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81548?usp=email )
Change subject: erasure_layout: don't copy region buffers if they're null/zero-size
......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/81548/comment/fd0cc90d_526bab2a :
PS2, Line 10: the pointer is null
I think from the meaning of the patch, you meant to say
`the pointer is non-null`
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/81548/comment/7680b945_70a9f658 :
PS1, Line 263: old_start_buf = (uint8_t *)malloc(old_start - region_start);
: if (!old_start_buf) {
: msg_cerr("Not enough memory!\n");
: ret = -1;
: goto _end;
: }
: old_end_buf = (uint8_t *)malloc(region_end - old_end);
: if (!old_end_buf) {
: msg_cerr("Not enough memory!\n");
: ret = -1;
: goto _end;
: }
> Sorry it took longer than I expected. […]
Great! this is what I was thinking about! I am resolving this comment.
I added few more other comments though :)
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/81548/comment/dd093a22_69ec2718 :
PS2, Line 263: size_t old_start_len
This can be const, but also
maybe rename it into `start_buf_len` ?
I think that "old" suffix doesn't really explain anything. What do you think?
https://review.coreboot.org/c/flashrom/+/81548/comment/47062ded_d9a3f306 :
PS2, Line 264: size_t old_end_len
And this can be const and `end_buf_len`
--
To view, visit https://review.coreboot.org/c/flashrom/+/81548?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I99aad4119f9c11bea75e3419926cc833bc1f68c5
Gerrit-Change-Number: 81548
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Sun, 12 May 2024 11:00:24 +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: Nicholas Chin.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82162?usp=email )
Change subject: flashrom_udev.rules: Add rule for CH347
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Nicholas, just wanted to check, is this patch ready, can I submit?
--
To view, visit https://review.coreboot.org/c/flashrom/+/82162?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ia83fa08f6d7c2f449b1a5c0c387c6d4368b99e3a
Gerrit-Change-Number: 82162
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Sun, 12 May 2024 08:04:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment