Attention is currently required from: Anastasia Klimchuk.
Antonio Vázquez Blanco has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/85072?usp=email )
Change subject: Rename cli_classic.h to a more adequate cli_getop.h
......................................................................
Patch Set 2:
(2 comments)
File cli_getopt.c:
https://review.coreboot.org/c/flashrom/+/85072/comment/a729ace5_ec5fc221?us… :
PS1, Line 28: #include "cli_getopt.h"
> I noticed you put this line on the top, why? Usually the style we have is: first come includes in <> […]
Sorry, this is even subconscious for me. I do this because cli_getopt.h is the interface for cli_getopt.c, for this reason, including cli_getop.h first helps debug whether that header file is fully independent from any of the other includes in the "c" file.
After that, I usually try to include other project headers and finally system headers because this strategy helps with producing clean includes...
I've changed the use of quotes but left the include on the top. Please let me know if you prefer to change it. Thanks!
https://review.coreboot.org/c/flashrom/+/85072/comment/3330c52d_18658643?us… :
PS1, Line 35: // msg_gerr
> I noticed you are adding this comments for includes of flash. […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/flashrom/+/85072?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: Iaceeabedc26e93147d8122376d88e730aad1e355
Gerrit-Change-Number: 85072
Gerrit-PatchSet: 2
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 11 Nov 2024 08:39:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk, Antonio Vázquez Blanco.
Hello Anastasia Klimchuk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85072?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: Rename cli_classic.h to a more adequate cli_getop.h
......................................................................
Rename cli_classic.h to a more adequate cli_getop.h
The header only defines getop related stuff so it seems more intuitive
this way.
Change-Id: Iaceeabedc26e93147d8122376d88e730aad1e355
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
---
M cli_classic.c
M cli_getopt.c
R include/cli_getopt.h
3 files changed, 12 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/85072/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/85072?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Iaceeabedc26e93147d8122376d88e730aad1e355
Gerrit-Change-Number: 85072
Gerrit-PatchSet: 2
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84954?usp=email )
Change subject: doc: Add few sections to recent development doc
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
I decided to submit another larger patch first, because this one is easier to rebase with merge conflicts.
Did that, so now this one needs approval again.
Thank you!
--
To view, visit https://review.coreboot.org/c/flashrom/+/84954?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: Iedaca4a704c57c1db399c7888f743ad2961cbf9d
Gerrit-Change-Number: 84954
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 11 Nov 2024 02:50:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Anastasia Klimchuk, Peter Marheine.
Hello Peter Marheine, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/84954?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+2 by Peter Marheine, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: doc: Add few sections to recent development doc
......................................................................
doc: Add few sections to recent development doc
Change-Id: Iedaca4a704c57c1db399c7888f743ad2961cbf9d
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/release_notes/devel.rst
1 file changed, 78 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/84954/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/84954?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Iedaca4a704c57c1db399c7888f743ad2961cbf9d
Gerrit-Change-Number: 84954
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Matti Finder, Nikolai Artemiev, Sergii Dmytruk, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85075?usp=email )
Change subject: flashchips: Skip "WP untested" message for SFDP-capable chip
......................................................................
Patch Set 3:
(2 comments)
Patchset:
PS2:
> I wonder if `chip->tested. […]
1) Ignored in the meaning, not showing the message "flash has status untested for WP"?
2) Or ignored in the meaning, if it's set to OK, don't believe it because it can't be?
I think the first one, the message still useful: people see it and sometimes wonder and sometimes add support for WP. Apart from the very edge cases, like "generic" chip entries, I would keep the message (for now... but things can change).
The second option: I don't think there are such entries in flashchips at the moment. It has always been that the contributor who adds support also marks as tested.
Commit Message:
https://review.coreboot.org/c/flashrom/+/85075/comment/958ec65c_daeea200?us… :
PS2, Line 12: SDFP chip and what would you report?
> `SFDP`
Thank you! And I found the same typo in the commit message of the next patch too!
--
To view, visit https://review.coreboot.org/c/flashrom/+/85075?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: I7e945389895a8042df3aaae72bccf73405be8651
Gerrit-Change-Number: 85075
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Comment-Date: Sun, 10 Nov 2024 23:44:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Attention is currently required from: Anastasia Klimchuk, Matti Finder, Nikolai Artemiev, Stefan Reinauer.
Hello Matti Finder, Nikolai Artemiev, Sergii Dmytruk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85075?usp=email
to look at the new patch set (#3).
Change subject: flashchips: Skip "WP untested" message for SFDP-capable chip
......................................................................
flashchips: Skip "WP untested" message for SFDP-capable chip
This entry in the flashchips represent a "SFDP-capable chip" and
it doesn't make sense to show the message "WP operation has status
untested, please report this". The entry can cover any generic
SFDP chip and what would you report?
Secondly, the entry "SFDP-capable chip" does not currently support
WP operations anyway.
Going further, we will be working with SFDP way more, so this area
needs to be gradually upgraded.
Testing:
flashrom -p dummy:emulate=MX25L6436 -c "SFDP-capable chip" -r dump.rom
Change-Id: I7e945389895a8042df3aaae72bccf73405be8651
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashchips.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/85075/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/85075?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I7e945389895a8042df3aaae72bccf73405be8651
Gerrit-Change-Number: 85075
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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-Attention: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Anastasia Klimchuk, Matti Finder.
Hello Matti Finder, Peter Marheine, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85092?usp=email
to look at the new patch set (#3).
Change subject: sfdp: Update the message shown when SFDP-capable chip is detected
......................................................................
sfdp: Update the message shown when SFDP-capable chip is detected
Testing:
flashrom -p dummy:emulate=MX25L6436 -c "SFDP-capable chip"
Change-Id: If1a480ae78f158cc4626e345149ea9025443f8a7
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashrom.c
1 file changed, 7 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/85092/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/85092?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If1a480ae78f158cc4626e345149ea9025443f8a7
Gerrit-Change-Number: 85092
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>