Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/76853?usp=email )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)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: Ie04a5e2325aab94bffb276675be3fa4a88c6e134
Signed-off-by: xianzheng <xianzheng(a)mxic.com.cn>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/76853
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M flashchips.c
M include/flashchips.h
2 files changed, 2 insertions(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Anastasia Klimchuk: Looks good to me, approved
diff --git a/flashchips.c b/flashchips.c
index 065ebf2..83264ef 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 8645054..dfe9ec6 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/+/76853?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: Ie04a5e2325aab94bffb276675be3fa4a88c6e134
Gerrit-Change-Number: 76853
Gerrit-PatchSet: 6
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-MessageType: merged
Attention is currently required from: Aarya, Alexander Goncharov, Sam McNally.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77747?usp=email )
Change subject: erasure_layout: Fix double invocation of erasers
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
I did some more investigation and traced when this double-erase started:
initial logic which erased everything once is here:
https://review.coreboot.org/c/flashrom/+/66104/67 (that's around end of last year)
line flashrom.c#1174
and here second invocation was added:
https://review.coreboot.org/c/flashrom/+/66104/95..96
the code moved to a separate file, so two invocations are at:
erasure_layout.c#314 and #322
At head the two invocations are at erasure_layout.c#331 and #342
Now in my patch here I wrapped second invocation into a conditional, so that it is not invoked always, but only if something left to erase. I don't know whether it is a realistic scenario, and that's what you were saying:
> ideally there should nothing be left once the above loop at line 309 finishes
--
To view, visit https://review.coreboot.org/c/flashrom/+/77747?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: I92351eba0fd29114ce98b4a839358e92d176af28
Gerrit-Change-Number: 77747
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Fri, 15 Sep 2023 06:17:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77779?usp=email )
Change subject: meson.build: Upgrade minimum Meson version to 0.56.0
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Patchset:
PS1:
> I recalled there was a moment when we tried to upgrade the version but then reverted back. […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/77779?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: I53c6c42c27a58734742e3dce3cdbde4c65b89a90
Gerrit-Change-Number: 77779
Gerrit-PatchSet: 2
Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(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: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-Attention: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Comment-Date: Fri, 15 Sep 2023 05:50:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Anton Samsonov, Thomas Heijligen.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77779?usp=email )
Change subject: meson.build: Upgrade minimum Meson version to 0.56.0
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/77779?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: I53c6c42c27a58734742e3dce3cdbde4c65b89a90
Gerrit-Change-Number: 77779
Gerrit-PatchSet: 2
Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(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: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Comment-Date: Fri, 15 Sep 2023 00:40:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Alexander Goncharov, Sam McNally.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77747?usp=email )
Change subject: erasure_layout: Fix double invocation of erasers
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> Yes that condition wouldn't be called, because iirc the erase addresses are extend d to match the bl […]
Yes, the block was erased twice, each block was erased twice. I discovered this because the test in CB:67535 is recording each invocation.
I had a hypothesis that is because line #342 (in the base patch) is invoking `erasefn(flashctx, start_addr, block_len)` but the loop above already processed the area of memory from start_addr and up to start_addr+block_len.
So I tried to fix this and made this patch, with this patch erasers started being invoked only once.
When I say condition never invoked I mean condition on line #345 in patchset3.
But now I want to make sure I haven't missed anything, maybe there is some corner case? I added a test case with very irregular layout regions, but it still works :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/77747?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: I92351eba0fd29114ce98b4a839358e92d176af28
Gerrit-Change-Number: 77747
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Thu, 14 Sep 2023 13:31:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Sam McNally.
Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77747?usp=email )
Change subject: erasure_layout: Fix double invocation of erasers
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> Also to add here: I actually have never seen the condition being called `if (last_addr < start_addr […]
Yes that condition wouldn't be called, because iirc the erase addresses are extend d to match the block boundaries. So ideally there should nothing be left once the above loop at line 309 finishes.
also I'm not sure if understand the double eraser invocation correctly, is it that a block is getting erased twice?
--
To view, visit https://review.coreboot.org/c/flashrom/+/77747?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: I92351eba0fd29114ce98b4a839358e92d176af28
Gerrit-Change-Number: 77747
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Thu, 14 Sep 2023 12:48:57 +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: Aarya, Alexander Goncharov, Sam McNally.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77747?usp=email )
Change subject: erasure_layout: Fix double invocation of erasers
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> This is my attempt to fix the double invocation of every eraser, what do you think about it? I am no […]
Also to add here: I actually have never seen the condition being called `if (last_addr < start_addr + block_len)`
When I say "never" I mean that I tries all test cases from next patch CB:67535 and also ran with dummy `flashrom -p dummy:emulate=W25Q128FV -E` and I commented out the "full chip" erasers for this chip, so that algo need to go into a loop and not erase whole chip at once.
--
To view, visit https://review.coreboot.org/c/flashrom/+/77747?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: I92351eba0fd29114ce98b4a839358e92d176af28
Gerrit-Change-Number: 77747
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Thu, 14 Sep 2023 12:23:13 +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: Aarya, Edward O'Callaghan, Nikolai Artemiev, Simon Buhrow, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67535?usp=email )
Change subject: tests: Unit tests for erase function selection algo
......................................................................
Patch Set 13:
(1 comment)
Patchset:
PS13:
I added test cases #7 and #15 with very irregular layout
--
To view, visit https://review.coreboot.org/c/flashrom/+/67535?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: I8f3fdefb76e71f6f8dc295d9dead5f38642aace7
Gerrit-Change-Number: 67535
Gerrit-PatchSet: 13
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Simon Buhrow
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 14 Sep 2023 12:18:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Edward O'Callaghan, Nikolai Artemiev, Simon Buhrow, Thomas Heijligen.
Hello Aarya, Edward O'Callaghan, Simon Buhrow, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67535?usp=email
to look at the new patch set (#13).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: tests: Unit tests for erase function selection algo
......................................................................
tests: Unit tests for erase function selection algo
The test checks that algorithm for erase functions selection
works and there are no regressions.
Specifically, test contains an array of test cases. Each case
initialises a given initial state of the memory for the mock chip,
and layout regions on the chip, and then performs erase and write
operations.
At the end of operation, test asserts the following:
- the state of mock chip memory is as expected, i.e. properly erased
or written
- erase blocks are invoked in expected order and expected number
of them
- chip operation (erase or write) returned 0.
Change-Id: I8f3fdefb76e71f6f8dc295d9dead5f38642aace7
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
A tests/erase_func_algo.c
M tests/meson.build
M tests/tests.c
M tests/tests.h
4 files changed, 683 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/35/67535/13
--
To view, visit https://review.coreboot.org/c/flashrom/+/67535?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: I8f3fdefb76e71f6f8dc295d9dead5f38642aace7
Gerrit-Change-Number: 67535
Gerrit-PatchSet: 13
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Simon Buhrow
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset