Attention is currently required from: Simon Buhrow, Nico Huber, Aarya, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 )
Change subject: flashrom.c: Add wrapper function to use the erase algorithm
......................................................................
Patch Set 91:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/66104/comment/2006480e_70b1a7f0
PS78, Line 6:
: flashrom.c: Add wrapper function to use the erase algorithm
:
: Add a function to call the erase algorithm.
> commit message is unrelated to content.
Doxygen and squash.
--
To view, visit https://review.coreboot.org/c/flashrom/+/66104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8
Gerrit-Change-Number: 66104
Gerrit-PatchSet: 91
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Simon Buhrow
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Fri, 23 Dec 2022 03:36:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Simon Buhrow, Aarya.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65844 )
Change subject: flashrom.c:Add a function to get list of sectors that need erasing
......................................................................
Patch Set 73:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/65844/comment/49bfac85_25e8e29c
PS73, Line 7: flashrom.c:Add a function to get list of sectors that need erasing
:
: Add a function that returns a list of sectors (as seen by the first
: erase function) that need erasing.
Doxygen documentation and squash.
--
To view, visit https://review.coreboot.org/c/flashrom/+/65844
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256
Gerrit-Change-Number: 65844
Gerrit-PatchSet: 73
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Simon Buhrow
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Dec 2022 03:34:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Aarya.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65642 )
Change subject: flashrom.c:Add function to align region to sector boundaries
......................................................................
Patch Set 74:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/65642/comment/15a1da24_d67f6188
PS74, Line 7: flashrom.c:Add function to align region to sector boundaries
:
: Add a function to align start and end address of the region (in struct
: walk_info) to some erase sector boundaries and modify the region start
: and end addresses to match nearest erase sector boundaries. This
: function will be used in the new algorithm for erase function selection.
just document the function with Doxygen and squash into the previous patch.
--
To view, visit https://review.coreboot.org/c/flashrom/+/65642
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I215ea4986aa23360fc65ff761f4e49c6069160ac
Gerrit-Change-Number: 65642
Gerrit-PatchSet: 74
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Simon Buhrow
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Dec 2022 03:32:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen, Paul Menzel, Aarya.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65879 )
Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks
......................................................................
Patch Set 54:
(9 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/65879/comment/bbb0fb7b_ab60580a
PS54, Line 11: So after use all the layouts as well as
: the list itself should be freed. The free_erase_layout function does
: that.
this part should be documented above the function in a Doxygen style comment. https://github.com/flashrom/flashrom/blob/master/flashrom.c#L555 is a example of Doxygen style comments.
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/65879/comment/3d1ba57e_cd2db943
PS54, Line 24: int
`unsigned int`
https://review.coreboot.org/c/flashrom/+/65879/comment/bab9c917_b22c99fe
PS54, Line 34: create_erase_layout
The O-time complexity of this function seems extremely large for something that says it just creates a layout? Can you add some Doxygen comments above the function and punctuate the function with some comments inside to allow for understanding the phases your algorithm is taking here please?
https://review.coreboot.org/c/flashrom/+/65879/comment/c9fe84b0_3e392952
PS54, Line 55: (
trivial: `e (`
https://review.coreboot.org/c/flashrom/+/65879/comment/6e8fdcd1_3efdca57
PS54, Line 64: )
: {
trivial: `) {` and `if (!ptr)` is canonical.
https://review.coreboot.org/c/flashrom/+/65879/comment/219fe8bf_68878343
PS54, Line 71: chipoff_t start_addr = 0;
does this belong to the outer block_count loop? should it be reset when the inner loop finishes for the next iteration of the outer loop?
https://review.coreboot.org/c/flashrom/+/65879/comment/3f0a0f51_2411aa78
PS54, Line 72: chipoff_t end_addr = 0;
delete and also don't unnecessarily initialise variables as you obscure compiler warnings.
https://review.coreboot.org/c/flashrom/+/65879/comment/4e5d6042_6cd915ef
PS54, Line 74: block_num < block_count
since `block_num` is initialised to zero above doesn't this predicate become `0 < block_count` and the block_count never decrements so this is a infinite loop?
https://review.coreboot.org/c/flashrom/+/65879/comment/e03cb7c1_fe1ecba0
PS54, Line 79: e
Keep minimal scope to variables so declare here,
`chipoff_t end_addr = start_addr + block->size - 1;`
--
To view, visit https://review.coreboot.org/c/flashrom/+/65879
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4
Gerrit-Change-Number: 65879
Gerrit-PatchSet: 54
Gerrit-Owner: 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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Dec 2022 03:15:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Rick Altherr.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/71206
to look at the new patch set (#3).
Change subject: flashchips: Remove FEATURE_4BA_WREN for MT25QL128 and mark as tested
......................................................................
flashchips: Remove FEATURE_4BA_WREN for MT25QL128 and mark as tested
Using both a Dediprog SF100 and a Bus Pirate, read and erase works
correctly on a MT25QL128 but writes were failing to take effect.
Currently, the entry in flashchips.c indicates that this device supports
4-byte addressing. Micron's datasheet indicates that it does not.
After removing FEATURE_4BA_WREN from feature_bits, both SF100 and
Bus Pirate were able to successfully read, erase, and write a
MT25QL128 so also marking as tested.
Change-Id: I6341456c722840a413bd2c51fe9a78bbda5cdbab
Signed-off-by: Rick Altherr <kc8apf(a)kc8apf.net>
---
M flashchips.c
1 file changed, 20 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/71206/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/71206
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6341456c722840a413bd2c51fe9a78bbda5cdbab
Gerrit-Change-Number: 71206
Gerrit-PatchSet: 3
Gerrit-Owner: Rick Altherr <kc8apf(a)kc8apf.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Rick Altherr <kc8apf(a)kc8apf.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Rick Altherr.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/71206
to look at the new patch set (#2).
Change subject: flashchips: Remove FEATURE_4BA_WREN for MT25QL128 and mark as tested
......................................................................
flashchips: Remove FEATURE_4BA_WREN for MT25QL128 and mark as tested
Using both a Dediprog SF100 and a Bus Pirate, read and erase works
correctly on a MT25QL128 but writes were failing to take effect.
Currently, the entry in flashchips.c indicates that this device supports
4-byte addressing. Micron's datasheet indicates that it does not.
After removing FEATURE_4BA_WREN from feature_bits, both SF100 and
Bus Pirate were able to successfully read, erase, and write a
MT25QL128 so also marking as tested.
Change-Id: I6341456c722840a413bd2c51fe9a78bbda5cdbab
Signed-off-by: Rick Altherr <kc8apf(a)kc8apf.net>
---
M flashchips.c
1 file changed, 20 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/71206/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/71206
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6341456c722840a413bd2c51fe9a78bbda5cdbab
Gerrit-Change-Number: 71206
Gerrit-PatchSet: 2
Gerrit-Owner: Rick Altherr <kc8apf(a)kc8apf.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Rick Altherr <kc8apf(a)kc8apf.net>
Gerrit-MessageType: newpatchset
Rick Altherr has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71206 )
Change subject: flashchips: Remove 4-byte addressing flag for MT25QL128 and mark as tested
......................................................................
flashchips: Remove 4-byte addressing flag for MT25QL128 and mark as tested
Using both a Dediprog SF100 and a Bus Pirate, read and erase works correctly
on a MT25QL128 but writes were failing to take effect. Currently, the entry
in flashchips.c indicates that this device supports 4-byte addressing.
Micron's datasheet indicates that it does not. After removing the 4-byte
addressing feature flag, both SF100 and Bus Pirate were able to successfully
read, erase, and write a MT25QL128 so also marking as tested.
Change-Id: I6341456c722840a413bd2c51fe9a78bbda5cdbab
Signed-off-by: Rick Altherr <kc8apf(a)kc8apf.net>
---
M flashchips.c
1 file changed, 19 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/71206/1
diff --git a/flashchips.c b/flashchips.c
index 9818eac..d303eb9 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -11997,8 +11997,8 @@
.page_size = 256,
/* supports SFDP */
/* OTP: 64B total; read 0x4B, write 0x42 */
- .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN,
- .tested = TEST_UNTESTED,
+ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP,
+ .tested = TEST_PREW,
.probe = PROBE_SPI_RDID,
.probe_timing = TIMING_ZERO,
.block_erasers =
--
To view, visit https://review.coreboot.org/c/flashrom/+/71206
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6341456c722840a413bd2c51fe9a78bbda5cdbab
Gerrit-Change-Number: 71206
Gerrit-PatchSet: 1
Gerrit-Owner: Rick Altherr <kc8apf(a)kc8apf.net>
Gerrit-MessageType: newchange
Attention is currently required from: Thomas Heijligen, Name of user not set #1004601.
Peter Stuge has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70571 )
Change subject: serial: Add Darwin/macOS support for custom and >230400 baudrates
......................................................................
Patch Set 7:
(2 comments)
Patchset:
PS7:
Thanks, glad to drop the change in between!
Now I just don't understand why this change still has a Merge Conflict.
File serial.c:
https://review.coreboot.org/c/flashrom/+/70571/comment/3710e293_59145c48
PS4, Line 78: #if !IS_DARWIN
> I would prefer the variant in use_custom_baud(). […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/70571
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3e6b88d2b4c2a130b16456752681fd9f807bf6f0
Gerrit-Change-Number: 70571
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Stuge <peter(a)stuge.se>
Gerrit-Reviewer: Name of user not set #1004601
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: Name of user not set #1004601
Gerrit-Comment-Date: Thu, 22 Dec 2022 17:16:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Peter Stuge <peter(a)stuge.se>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment