Attention is currently required from: Anastasia Klimchuk, Angel Pons, Neil Armstrong, Thomas Heijligen.
Anastasia Klimchuk has uploaded a new patch set (#5) to the change originally created by Neil Armstrong. ( https://review.coreboot.org/c/flashrom/+/50263?usp=email )
The following approvals got outdated and were removed:
Code-Review+2 by Angel Pons, Verified+1 by build bot (Jenkins)
Change subject: flashchips: add definition of the XT25F02E SPI NOR flash
......................................................................
flashchips: add definition of the XT25F02E SPI NOR flash
This adds definition of the XT25F02E 2MBit SPI NOR Flash
from XTX Technology Limited.
Tested (Probe, Erase, Write, Read) with a VL805 USB3.0 bridge.
Datasheet:
https://datasheet.lcsc.com/lcsc/2006091008_XTX-XT25F02EDTIGT_C596313.pdf
Signed-off-by: Neil Armstrong <narmstrong(a)baylibre.com>
Change-Id: I295633c448c05520e4a6aa09c08bd7c9f2346d54
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashchips.c
M include/flashchips.h
2 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/50263/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/50263?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I295633c448c05520e4a6aa09c08bd7c9f2346d54
Gerrit-Change-Number: 50263
Gerrit-PatchSet: 5
Gerrit-Owner: Neil Armstrong <narmstrong(a)baylibre.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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: Neil Armstrong <narmstrong(a)baylibre.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/66438?usp=email )
Change subject: flashchips: Add EON EN25T80 and EN25T80
......................................................................
Abandoned
Contributor left, we have no datasheets and no testing information
--
To view, visit https://review.coreboot.org/c/flashrom/+/66438?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie62a08bed242b5b006e4bb2f86eb70c871bd5ed9
Gerrit-Change-Number: 66438
Gerrit-PatchSet: 1
Gerrit-Owner: Shreeyash <shreeyash335(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: abandon
Name of user not set #1005084 has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/77581?usp=email )
Change subject: flashchips: Add support for MXIC MX25U25643G
......................................................................
flashchips: Add support for MXIC MX25U25643G
It is similar to the MX25U25635F and shares its RDID.
Tested by ch341a programmer : read, write and erase.
Datasheet is available at the following URL:
https://www.mxic.com.tw/en-us/products/NOR-Flash/Serial-NOR-Flash/Pages/speā¦
Change-Id: Id6c4254a02800f04c091009d19a628435fc20fce
Signed-off-by: xianzheng <xianzheng(a)mxic.com.cn>
---
M flashchips.c
M include/flashchips.h
2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/77581/1
diff --git a/flashchips.c b/flashchips.c
index ce68e95..b0092e1 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -10207,7 +10207,7 @@
{
.vendor = "Macronix",
- .name = "MX25U25635F",
+ .name = "MX25U25635F/MX25U25643G",
.bustype = BUS_SPI,
.manufacture_id = MACRONIX_ID,
.model_id = MACRONIX_MX25U25635F,
diff --git a/include/flashchips.h b/include/flashchips.h
index 6e191eb..dbe7cdb 100644
--- a/include/flashchips.h
+++ b/include/flashchips.h
@@ -529,7 +529,7 @@
#define MACRONIX_MX25U3235E 0x2536 /* Same as MX25U6435F */
#define MACRONIX_MX25U6435E 0x2537 /* Same as MX25U6435F */
#define MACRONIX_MX25U12835E 0x2538 /* Same as MX25U12835F */
-#define MACRONIX_MX25U25635F 0x2539
+#define MACRONIX_MX25U25635F 0x2539 /* Same as MX25U25643G */
#define MACRONIX_MX25U51245G 0x253a
#define MACRONIX_MX25L3235D 0x5E16 /* MX25L3225D/MX25L3235D/MX25L3237D */
#define MACRONIX_MX25L6495F 0x9517
--
To view, visit https://review.coreboot.org/c/flashrom/+/77581?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id6c4254a02800f04c091009d19a628435fc20fce
Gerrit-Change-Number: 77581
Gerrit-PatchSet: 1
Gerrit-Owner: Name of user not set #1005084
Gerrit-MessageType: newchange
Attention is currently required from: Name of user not set #1005084, Nikolai Artemiev, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77563?usp=email )
Change subject: flashchips: Add support for MXIC MX25U25643G
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
Patchset:
PS1:
xianzheng, seems like you made a mistake, you shouldn't be removing lines in a patch like this
--
To view, visit https://review.coreboot.org/c/flashrom/+/77563?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibff2780dcb86ffb7790e0204385e6350f136f21d
Gerrit-Change-Number: 77563
Gerrit-PatchSet: 1
Gerrit-Owner: Name of user not set #1005084
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Name of user not set #1005084
Gerrit-Comment-Date: Fri, 01 Sep 2023 04:41:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Vasily Galkin.
Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77530?usp=email )
Change subject: flashchips: Add WP features for Winbond W25X20
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/77530?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I82c0cc52ca2a78d27f513234cc12d3e09d8905a5
Gerrit-Change-Number: 77530
Gerrit-PatchSet: 1
Gerrit-Owner: Vasily Galkin
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Vasily Galkin
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 31 Aug 2023 13:05:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Alexander Goncharov, Anton Samsonov.
Anton Samsonov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email )
Change subject: Remove dependency on C23 __has_include()
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Thanks for the patch! Can you please provide what compiler and version you used to test this patch? [ā¦]
In my understanding, this patch may be needed for all compilers based on legacy versions of EDG frontend, such as version 4.14, which is somewhat onĀ par with GCCĀ 5.5, for example. Those still happen to be used in embedded / certified systems.
Newer EDG versions like 5.1, which is somewhat onĀ par with GCCĀ 7.3, and of course the most recent versions like 6.4, which is somewhat onĀ par withĀ GCCĀ 11.3, do not need this patch already. But those versions are not available on all target platforms.
Do you think such details should be present in commit message?
--
To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6
Gerrit-Change-Number: 77089
Gerrit-PatchSet: 1
Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Thu, 31 Aug 2023 10:23:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: comment
Attention is currently required from: Anton Samsonov.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email )
Change subject: Remove dependency on C23 __has_include()
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Thanks for the patch! Can you please provide what compiler and version you used to test this patch? It would be great to include that in the commit message.
--
To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6
Gerrit-Change-Number: 77089
Gerrit-PatchSet: 1
Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Comment-Date: Thu, 31 Aug 2023 07:15:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nikolai Artemiev has submitted this change. ( https://review.coreboot.org/c/flashrom/+/75991?usp=email )
Change subject: flashrom: only perform WP unlock for write/erase operations
......................................................................
flashrom: only perform WP unlock for write/erase operations
Don't unlock using WP for read/verify operations because WP will only
disable write locks. Most chips don't have read locks anyway, but some
do, so we still call the chip's unlock function for read/verify
operations.
Unconditionally unlocking using WP slows down flashrom significantly
with some programmers, particularly linux_mtd due to inefficiency in the
current kernel MTD interface.
BUG=b:283779258
BRANCH=none
TEST=`ninja test`
TEST=`flashrom -{r,w,E,v}` on strongbad
TEST=`flashrom --wp-enable; flashrom -{w,E}` on strongbad
Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/75991
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M flashrom.c
1 file changed, 41 insertions(+), 11 deletions(-)
Approvals:
build bot (Jenkins): Verified
Edward O'Callaghan: Looks good to me, approved
diff --git a/flashrom.c b/flashrom.c
index b6e5cf8..630c69d 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -2052,24 +2052,49 @@
return 0;
}
-static int unlock_flash_wp(struct flashctx *const flash)
+static int unlock_flash_wp(struct flashctx *const flash,
+ const bool write_it, const bool erase_it)
+
{
- /* Save original WP state to be restored later */
- if (save_initial_flash_wp(flash))
+ int ret = 0;
+
+ /* WP only disables write protection, so only use WP to unlock
+ * for write/erase operations.
+ *
+ * For read/verify operations, we still call the chip's unlock
+ * function, which may disable read locks if the chip has them.
+ */
+ if (!write_it && !erase_it) {
+ msg_cdbg("Skipping writeprotect-based unlocking for read/verify operations.\n");
return -1;
+ }
+
+ /* Save original WP state to be restored later */
+ if (save_initial_flash_wp(flash)) {
+ ret = -1;
+ goto warn_out;
+ }
/* Disable WP */
struct flashrom_wp_cfg *unlocked_wp_cfg;
- if (flashrom_wp_cfg_new(&unlocked_wp_cfg) != FLASHROM_WP_OK)
- return -1;
+ if (flashrom_wp_cfg_new(&unlocked_wp_cfg) != FLASHROM_WP_OK) {
+ ret = -1;
+ goto warn_out;
+ }
flashrom_wp_set_range(unlocked_wp_cfg, 0, 0);
flashrom_wp_set_mode(unlocked_wp_cfg, FLASHROM_WP_MODE_DISABLED);
- enum flashrom_wp_result ret = flashrom_wp_write_cfg(flash, unlocked_wp_cfg);
+ if (flashrom_wp_write_cfg(flash, unlocked_wp_cfg) != FLASHROM_WP_OK) {
+ ret = -1;
+ }
flashrom_wp_cfg_release(unlocked_wp_cfg);
- return (ret == FLASHROM_WP_OK) ? 0 : -1;
+warn_out:
+ if (ret)
+ msg_cerr("Failed to unlock flash status reg with wp support.\n");
+
+ return ret;
}
int prepare_flash_access(struct flashctx *const flash,
@@ -2092,14 +2117,19 @@
/* Initialize chip_restore_fn_count before chip unlock calls. */
flash->chip_restore_fn_count = 0;
- /* Given the existence of read locks, we want to unlock for read, erase and write. */
int ret = 1;
if (flash->chip->decode_range != NO_DECODE_RANGE_FUNC ||
(flash->mst->buses_supported & BUS_PROG && flash->mst->opaque.wp_write_cfg)) {
- ret = unlock_flash_wp(flash);
- if (ret)
- msg_cerr("Failed to unlock flash status reg with wp support.\n");
+ ret = unlock_flash_wp(flash, write_it, erase_it);
}
+ /*
+ * Fall back to chip unlock function if we haven't already successfully
+ * unlocked using WP (e.g. WP unlocking failed, chip had no WP support,
+ * WP was skipped for read/verify ops).
+ *
+ * Given the existence of read locks, we want to unlock for read,
+ * erase, write, and verify.
+ */
blockprotect_func_t *bp_func = lookup_blockprotect_func_ptr(flash->chip);
if (ret && bp_func)
bp_func(flash);
--
To view, visit https://review.coreboot.org/c/flashrom/+/75991?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1
Gerrit-Change-Number: 75991
Gerrit-PatchSet: 5
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: merged
Alexander Goncharov has submitted this change. ( https://review.coreboot.org/c/flashrom/+/77293?usp=email )
Change subject: doc: Fix nesting of About flashrom group of menu items
......................................................................
doc: Fix nesting of About flashrom group of menu items
Adding the title to About flashrom index page allows the engine
to recognise it as a group with a list of menu items inside, which
is as expected.
Without the title on the index page, all menu items inside About
flashrom are inlined into the menu.
Change-Id: I595acc282a536a6d5fa26cf2f8d18dbe549f9716
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/77293
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Alexander Goncharov <chat(a)joursoir.net>
---
M doc/about_flashrom/index.rst
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Alexander Goncharov: Looks good to me, approved
Edward O'Callaghan: Looks good to me, approved
diff --git a/doc/about_flashrom/index.rst b/doc/about_flashrom/index.rst
index 11d845f..de36fc2 100644
--- a/doc/about_flashrom/index.rst
+++ b/doc/about_flashrom/index.rst
@@ -1,3 +1,6 @@
+About flashrom
+==============
+
.. toctree::
:maxdepth: 1
--
To view, visit https://review.coreboot.org/c/flashrom/+/77293?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I595acc282a536a6d5fa26cf2f8d18dbe549f9716
Gerrit-Change-Number: 77293
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
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-MessageType: merged