Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Peter Marheine.
Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email )
Change subject: flashrom: Don't throw around "delay 1 second" so lightly
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> > If nobody knows why this is necessary or how to reproduce, I think it's okay to remove. […]
Unfortunately, this hack survived so long it migrated from the CLI to the library. So a CLI flag won't really provide an escape hatch for all users -- unless you also want to also add a libflashrom flag for it??
I'm still somewhat of a lazy opinion here:
1. I doubt that anything will come of either this CL, nor of a CLI flag that provides a bug-report-escape-hatch for it and
2. the whole `--verify` sequence is trivial enough to emulate on one's own [1]. That gets slightly more complex to implement if people are using environments without basic stuff like a decent shell or tools like `cmp`, or if they're using surgical flags (like `--include`). But it's not that hard.
And the good news is we don't have to try #2 if I'm right about #1 😊
But if you still think we need a flag, I can give it a shot.
And either way, do you want me to start such a mailing list thread, or were you suggesting I do this?
[1] e.g., `flashrom --read /tmp/foo.bin`; `flashrom --write /my/new/image.bin --noverify`; `sleep 1`; `flashrom --read /tmp/bar.bin`; `cmp /tmp/foo.bin /tmp/bar.bin`
--
To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc
Gerrit-Change-Number: 80807
Gerrit-PatchSet: 3
Gerrit-Owner: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Tue, 23 Apr 2024 00:23:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Brian Norris, Nikolai Artemiev, Peter Marheine.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email )
Change subject: flashrom: Don't throw around "delay 1 second" so lightly
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> If nobody knows why this is necessary or how to reproduce, I think it's okay to remove.
Yes, true.
I was trying to find out which exactly chips needed that extra delay, but no success so far. If we knew which chips we could add a feature flag as Nikolai suggested, and mark them in flashchips. But it seems, we don't know.
So your plan looks very reasonable: an option will cover for the edge cases (if they exist).
I was thinking to start from a post on a mailing list, very specifically about removing 1s delay (there has been none dedicated thread), so that if someone suddenly recalls information about those odd chips, they can share. Even if no one responds, it stays in archives as a record of decision being made with explanation why.
And then update this patch with an option, post link to patch to the thread. That should be enough for the "paperwork" :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc
Gerrit-Change-Number: 80807
Gerrit-PatchSet: 3
Gerrit-Owner: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Brian Norris <briannorris(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 22 Apr 2024 11:03:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81970?usp=email )
Change subject: <Flashrom SPI NOR>: adding a few new SPI NOR part numbers
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Victor, this is amazing, thank you so much for adding and updating so many chips!
What you need to do now, is to split this large patch into several smaller ones. I can see some of the models are related and can be grouped together. So for example, GD25LB128E, GD25LQ128E, GD25LR128E can be in one patch.
And then GD25LB256F, GD25LQ256H, GD25LR256F can be in the next patch, and so on.
When you split your git commit locally, you will have a chain of commits in your local branch. And then when you push the whole chain to Gerrit, you will see all the patches in a chain as well.
For commit titles, you can use the pattern:
`flashchips: Add (or update) chip models <list models>`
Another thing is, you need to provide links to public datasheets for the models added or updated in each patch.
Thanks!
--
To view, visit https://review.coreboot.org/c/flashrom/+/81970?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I1526eed830845d5391ca14c7fd3ac58a5aee71f2
Gerrit-Change-Number: 81970
Gerrit-PatchSet: 1
Gerrit-Owner: Victor Lim <victorswlim(a)yahoo.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: Victor Lim <victorswlim(a)yahoo.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Mon, 22 Apr 2024 10:42:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Brian Norris, Nikolai Artemiev.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email )
Change subject: flashrom: Don't throw around "delay 1 second" so lightly
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> My main concern is that there may be some chips in the flashchips database which need this delay, bu […]
If nobody knows why this is necessary or how to reproduce, I think it's okay to remove.
If we're feeling particularly conservative, we could add an option (perhaps `--extra-readback-delay`, but feel free to bikeshed the name) that enables the extra delay so there's an escape hatch available if it turns out somebody has such a chip- then we can suggest using that option as a fallback in `emergency_help_message()` and ask that people report cases where they need to use it (so hopefully over time the escape hatch would become unnecessary).
--
To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc
Gerrit-Change-Number: 80807
Gerrit-PatchSet: 3
Gerrit-Owner: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Brian Norris <briannorris(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Mon, 22 Apr 2024 00:50:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Riku Viitanen, Swift Geek (Sebastian Grzywna), Sydney, Thomas Heijligen, Urja Rannikko.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82018?usp=email )
Change subject: doc: Convert serprog docs to rst and add to doc directory
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I tried to gather people who know about serprog, or did development recently.
One person I was not able to contact is GNUtoo, if someone knows how to contact them and invite to review, that would be great!
Many thanks for review!
I have converted existing docs to rst format, fixed few typos and removed broken links. Otherwise, it's the same content.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82018?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ie52f1e051ed215d61d5fb535e3eddeac71f64d13
Gerrit-Change-Number: 82018
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Reviewer: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Attention: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Sydney <git(a)funkeleinhorn.com>
Gerrit-Attention: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Comment-Date: Sat, 20 Apr 2024 13:54:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Riku Viitanen, Swift Geek (Sebastian Grzywna), Sydney, Thomas Heijligen, Urja Rannikko.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82018?usp=email )
Change subject: doc: Convert serprog docs to rst and add to doc directory
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
For reference, wiki page is here https://wiki.flashrom.org/Serprog
--
To view, visit https://review.coreboot.org/c/flashrom/+/82018?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ie52f1e051ed215d61d5fb535e3eddeac71f64d13
Gerrit-Change-Number: 82018
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Reviewer: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Attention: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Sydney <git(a)funkeleinhorn.com>
Gerrit-Attention: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Comment-Date: Sat, 20 Apr 2024 13:50:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment