Attention is currently required from: Anastasia Klimchuk, Paul Menzel, ZhiYuanNJ.
Nicholas Chin has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email )
Change subject: ch347_spi: Add driver support for CH347F packaging
......................................................................
Patch Set 11:
(1 comment)
Patchset:
PS11:
> Nicholas, just checking, do you approve the patch? […]
The current code looks fine to me, and my CH347T still seems to work fine with it (I'm assuming ZhiYuanNJ tested it with a CH347F; I don't have that variant). I approve of it, but I did want someone else to take a look as I'm not very active in flashrom development/review.
--
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?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 11
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: ZhiYuanNJ
Gerrit-Comment-Date: Wed, 29 May 2024 03:11:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Paul Menzel, ZhiYuanNJ.
Anastasia Klimchuk has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email )
Change subject: ch347_spi: Add driver support for CH347F packaging
......................................................................
Patch Set 11:
(1 comment)
Patchset:
PS11:
Nicholas, just checking, do you approve the patch?
You did a great review, thank you so much!
--
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?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 11
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: ZhiYuanNJ
Gerrit-Comment-Date: Wed, 29 May 2024 02:49:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Nikolai Artemiev, Robert Marko, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change by Robert Marko. ( https://review.coreboot.org/c/flashrom/+/82676?usp=email )
Change subject: flashchips: Add XTX XT25F128B
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/82676?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I37084bd66bc7a8f93d6533ab0d67aa2528786299
Gerrit-Change-Number: 82676
Gerrit-PatchSet: 1
Gerrit-Owner: Robert Marko <robimarko(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: Robert Marko <robimarko(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 29 May 2024 02:19:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Paul Menzel, ZhiYuanNJ.
Nicholas Chin has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email )
Change subject: ch347_spi: Add driver support for CH347F packaging
......................................................................
Patch Set 11: Code-Review+1
--
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?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 11
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: Wed, 29 May 2024 02:07:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Aarya, Nikolai Artemiev, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email )
Change subject: Correct get_flash_region() to use inclusive upper bounds
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS6:
> I happened to be doing some other firmware work on the same machine I tested reading on and discover […]
Oh wow! So your patch not only removed tech debt, but also helped to find another bug! very cool :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec
Gerrit-Change-Number: 82496
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 29 May 2024 02:01:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Georg Gottleuber, Nikolai Artemiev, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change by Georg Gottleuber. ( https://review.coreboot.org/c/flashrom/+/82101?usp=email )
Change subject: flashchips.c: Add support for XM25RU256C
......................................................................
Patch Set 3:
(2 comments)
Patchset:
PS3:
> I double-checked with the datasheet: looks good. Only one additional ". […]
Is this for 4K sector? You can add it to the array of block_erasers, next to existing SPI_BLOCK_ERASE_20 entry. I saw this before, you can check there is a bunch of chip definitions in flashchips.c with both SPI_BLOCK_ERASE_21 and SPI_BLOCK_ERASE_20 defined.
If you will be adding 21, then if you can run erase once again on this chip, that would be perfect!
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/82101/comment/b9058c31_815e102b?us… :
PS3, Line 22118: TEST_UNTESTED
This probably slipped from a previous version of the patch? You already changed tested status to `TEST_OK_PREW` which is what you need (because you tested).
--
To view, visit https://review.coreboot.org/c/flashrom/+/82101?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I431474a662304d09438e274706d3fc9cfbbe0bd6
Gerrit-Change-Number: 82101
Gerrit-PatchSet: 3
Gerrit-Owner: Georg Gottleuber <ggo(a)tuxedo.de>
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: Georg Gottleuber <ggo(a)tuxedo.de>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 29 May 2024 01:59:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Georg Gottleuber <ggo(a)tuxedo.de>
Attention is currently required from: Aarya, Nikolai Artemiev.
Peter Marheine has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email )
Change subject: Correct get_flash_region() to use inclusive upper bounds
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS6:
> I tested reading flash on a yaviks board both with and without this patch, and it looks okay. […]
I happened to be doing some other firmware work on the same machine I tested reading on and discovered a `region.end - 1` in erase code that only exists in ChromeOS which breaks erase, but after patching that the whole read/erase/write process worked fine.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec
Gerrit-Change-Number: 82496
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 29 May 2024 01:13:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk, DZ, Nikolai Artemiev, Stefan Reinauer.
Bora Guvendik has posted comments on this change by Bora Guvendik. ( https://review.coreboot.org/c/flashrom/+/82626?usp=email )
Change subject: flashchips: add support for MX77U51250F chip
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> Bora, thank you for the patch! Could you please give a link to the datasheet for the chip? […]
Hi Anastasia, I think you need to request datasheet from Macronix, it is not available on their site yet.
https://www.mxic.com.tw/en-us/products/NOR-Flash/Serial-NOR-Flash/Pages/def…
--
To view, visit https://review.coreboot.org/c/flashrom/+/82626?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I2c2e94f01dc63f60cf636bc6afe1f033e2a6f83c
Gerrit-Change-Number: 82626
Gerrit-PatchSet: 3
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: DZ <danielzhang(a)mxic.com.cn>
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: DZ <danielzhang(a)mxic.com.cn>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 29 May 2024 00:35:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Alexander Goncharov has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82657?usp=email )
Change subject: add gcc-14 -Werror=calloc-transposed-args compatibility
......................................................................
add gcc-14 -Werror=calloc-transposed-args compatibility
gcc-14 added a new `-Wcalloc-transposed-args` warning. Documentation
says:
```
Warn about calls to allocation functions decorated with attribute
alloc_size with two arguments, which use sizeof operator as the earlier
size argument and don’t use it as the later size argument. This is a
coding style warning. The first argument to calloc is documented to be
number of elements in array, while the second argument is size of each
element, so calloc (n, sizeof (int)) is preferred over
calloc (sizeof (int), n).
```
Let's fix the existing occurrences.
Found-by: gcc v14.1.1 20240507
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
Change-Id: Icb9842fbc2fa6ad4cd9dc9384c19fd3741eadb2e
Reviewed-on: https://review.coreboot.org/c/flashrom/+/82657
Reviewed-by: Peter Marheine <pmarheine(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Robert Marko <robimarko(a)gmail.com>
Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr>
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M s25f.c
M spi25_statusreg.c
2 files changed, 2 insertions(+), 2 deletions(-)
Approvals:
Robert Marko: Looks good to me, but someone else must approve
Peter Marheine: Looks good to me, approved
Elyes Haouas: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Anastasia Klimchuk: Looks good to me, approved
diff --git a/s25f.c b/s25f.c
index dd15efc..b0f2438 100644
--- a/s25f.c
+++ b/s25f.c
@@ -291,7 +291,7 @@
s25fs_read_cr(flash, CR3NV_ADDR));
/* Restore CR3V when flashrom exits */
- uint8_t *data = calloc(sizeof(uint8_t), 1);
+ uint8_t *data = calloc(1, sizeof(uint8_t));
if (!data) {
msg_cerr("Out of memory!\n");
return 1;
diff --git a/spi25_statusreg.c b/spi25_statusreg.c
index 98988af..ceb2c77 100644
--- a/spi25_statusreg.c
+++ b/spi25_statusreg.c
@@ -310,7 +310,7 @@
}
/* Restore status register content upon exit in finalize_flash_access(). */
- uint8_t *data = calloc(sizeof(uint8_t), 1);
+ uint8_t *data = calloc(1, sizeof(uint8_t));
if (!data) {
msg_cerr("Out of memory!\n");
return 1;
--
To view, visit https://review.coreboot.org/c/flashrom/+/82657?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Icb9842fbc2fa6ad4cd9dc9384c19fd3741eadb2e
Gerrit-Change-Number: 82657
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Robert Marko <robimarko(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>