March 2024 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: udelay: clock_getres() does not provide clock_gettime() resolution
by Chen (Code Review) 11 Mar '24

11 Mar '24

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Nikolai Artemiev, Stefan Reinauer. Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80806?usp=email ) Change subject: udelay: clock_getres() does not provide clock_gettime() resolution ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > Chen, I remember you are developing on Mac, maybe with your knowledge you can have a look whether th […] Although I use macOS, I can't give an answer to this question. This may require someone who is very familiar with the macOS kernel and libc. -- To view, visit https://review.coreboot.org/c/flashrom/+/80806?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: Icdc6e184eacb14d3bd27029e3fc75be4431d82b4 Gerrit-Change-Number: 80806 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Chen <c(a)jia.je> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.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-Comment-Date: Mon, 11 Mar 2024 11:30:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: udelay: clock_getres() does not provide clock_gettime() resolution
by Anastasia Klimchuk (Code Review) 11 Mar '24

11 Mar '24

Attention is currently required from: Brian Norris, Chen, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80806?usp=email ) Change subject: udelay: clock_getres() does not provide clock_gettime() resolution ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/80806/comment/43d5efbe_d5a4ecb8 : PS2, Line 10: Linux makes this clearer > It's a good question, and my answer is that I'm not sure. […] I think `#ifdef __linux__` is a good plan B , but before we come to it let me try invite few more people with deeper knowledge of non-Linux, Patchset: PS3: Chen, I remember you are developing on Mac, maybe with your knowledge you can have a look whether this patch will work as is for MacOS? Thank you so much!! -- To view, visit https://review.coreboot.org/c/flashrom/+/80806?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: Icdc6e184eacb14d3bd27029e3fc75be4431d82b4 Gerrit-Change-Number: 80806 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Chen <c(a)jia.je> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Chen <c(a)jia.je> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 11 Mar 2024 10:31:34 +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> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: udelay: Lower the sleep vs delay threshold
by Anastasia Klimchuk (Code Review) 11 Mar '24

11 Mar '24

Attention is currently required from: Brian Norris, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80808?usp=email ) Change subject: udelay: Lower the sleep vs delay threshold ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/80808?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: Ifd4821c66c5564f7c975c08769a6742f645e9be0 Gerrit-Change-Number: 80808 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 11 Mar 2024 10:12:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: doc: Update arbitration team to be flashrom specific
by Anastasia Klimchuk (Code Review) 10 Mar '24

10 Mar '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81106?usp=email ) Change subject: doc: Update arbitration team to be flashrom specific ...................................................................... doc: Update arbitration team to be flashrom specific Same approach as it was before: founder and current project lead. Change-Id: I1043b9499ab22da5ec981592d7b4311f027c4b5f Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81106 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M doc/about_flashrom/code_of_conduct.rst 1 file changed, 1 insertion(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/doc/about_flashrom/code_of_conduct.rst b/doc/about_flashrom/code_of_conduct.rst index da94e29..2967095 100644 --- a/doc/about_flashrom/code_of_conduct.rst +++ b/doc/about_flashrom/code_of_conduct.rst @@ -126,10 +126,8 @@ Our arbitration team consists of the following people +* Anastasia Klimchuk <aklm(a)flashrom.org> (Australia) * Stefan Reinauer <stefan.reinauer(a)coreboot.org> (USA) -* Patrick Georgi <patrick(a)coreboot.org> (Germany) -* Ronald Minnich <rminnich(a)coreboot.org> (USA) -* Martin Roth <martin(a)coreboot.org> (USA) License and attribution ======================= -- To view, visit https://review.coreboot.org/c/flashrom/+/81106?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: I1043b9499ab22da5ec981592d7b4311f027c4b5f Gerrit-Change-Number: 81106 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[main]: cli_classic: Defer flashrom_init calibration until after options parsing
by Anastasia Klimchuk (Code Review) 09 Mar '24

09 Mar '24

Attention is currently required from: Brian Norris. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80772?usp=email ) Change subject: cli_classic: Defer flashrom_init calibration until after options parsing ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/80772?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: I57949ab511df04c6922008fa8cb12467154e2c5e Gerrit-Change-Number: 80772 Gerrit-PatchSet: 2 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Comment-Date: Sat, 09 Mar 2024 09:07:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: linux_mtd: Provide no-op delay implementation
by Anastasia Klimchuk (Code Review) 09 Mar '24

09 Mar '24

Attention is currently required from: Brian Norris, Hsuan-ting Chen, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80771?usp=email ) Change subject: linux_mtd: Provide no-op delay implementation ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS1: > Sure! The BUG= and TEST= thing seems like more of a ChromiumOS thing than a coreboot. […] Very useful, thank you. The tag `TEST=` itself is not that important, but the info about testing is. However, it's often good to have structure and tags, I like structure. -- To view, visit https://review.coreboot.org/c/flashrom/+/80771?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: I3ac69d3fd7cfc689c5b32c130d044516ed846c29 Gerrit-Change-Number: 80771 Gerrit-PatchSet: 2 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 09 Mar 2024 09:06:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: linux_mtd: Provide no-op delay implementation
by Brian Norris (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Anastasia Klimchuk, Hsuan-ting Chen, Nikolai Artemiev. Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80771?usp=email ) Change subject: linux_mtd: Provide no-op delay implementation ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > Do you have any testing info that you can add to commit message? I strongly suspect that there is in […] Sure! The BUG= and TEST= thing seems like more of a ChromiumOS thing than a coreboot.org thing, but I guess coreboot.org doesn't seem to mind them. The details I had in the non-public bug were essentially a more verbose version of my last paragraph (1.1 seconds time waste). But I've added a few more details of the exact operation and stats. -- To view, visit https://review.coreboot.org/c/flashrom/+/80771?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: I3ac69d3fd7cfc689c5b32c130d044516ed846c29 Gerrit-Change-Number: 80771 Gerrit-PatchSet: 2 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 08 Mar 2024 23:25:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: udelay: Lower the sleep vs delay threshold
by Brian Norris (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80808?usp=email ) Change subject: udelay: Lower the sleep vs delay threshold ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > I can update the commit message. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/80808?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: Ifd4821c66c5564f7c975c08769a6742f645e9be0 Gerrit-Change-Number: 80808 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 08 Mar 2024 23:25:09 +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> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: udelay: Lower the sleep vs delay threshold
by Brian Norris (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. Hello Nikolai Artemiev, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/80808?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+2 by Nikolai Artemiev, Verified+1 by build bot (Jenkins) Change subject: udelay: Lower the sleep vs delay threshold ...................................................................... udelay: Lower the sleep vs delay threshold By default, we busy-loop (a.k.a., "delay") for most delay values, and only allow sleeping for large delays. But busy-looping is expensive, as it wastes CPU cycles. In a simple program that runs a bunch of samples of [1] over 1000 samples, I find that for 0.1 s (100000 us): 64x2 AMD CPU (CONFIG_HZ=250 / CONFIG_NO_HZ_FULL=y): min diff: 60 us max diff: 831 us mean diff: 135 us 4+2 Mediatek MT8183 CPU (CONFIG_HZ=1000 / CONFIG_NO_HZ_IDLE=y / sysctl kernel.timer_highres=1): min diff: 70 us max diff: 1556 us mean diff: 146 us 4+2 Mediatek MT8183 CPU (CONFIG_HZ=1000 / CONFIG_NO_HZ_IDLE=y / sysctl kernel.timer_highres=0): min diff: 94 us max diff: 7222 us mean diff: 1201 us i.e., maximum 1.5% error, typically ~0.1% error with high resolution timers. Max 7% error, typical 1% error with low resolution timers. The error is always in the positive direction (i.e., sleep longer than the requested delay, not shorter than the request). This seems reasonable. [1] Stripped / pseudocode: clock_gettime(CLOCK_MONOTONIC, before); nanosleep({ .tv_nsec = usecs * 1000 }, NULL); clock_gettime(CLOCK_MONOTONIC, after); diff = abs((after - before) / 1000 - usecs)); Change-Id: Ifd4821c66c5564f7c975c08769a6742f645e9be0 Signed-off-by: Brian Norris <briannorris(a)chromium.org> --- M udelay.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/80808/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/80808?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: Ifd4821c66c5564f7c975c08769a6742f645e9be0 Gerrit-Change-Number: 80808 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[XS] Change in flashrom[main]: linux_mtd: Provide no-op delay implementation
by Brian Norris (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Brian Norris, Hsuan-ting Chen, Nikolai Artemiev. Hello Anastasia Klimchuk, Hsuan-ting Chen, Nikolai Artemiev, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/80771?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+2 by Hsuan-ting Chen, Code-Review+2 by Nikolai Artemiev, Verified+1 by build bot (Jenkins) Change subject: linux_mtd: Provide no-op delay implementation ...................................................................... linux_mtd: Provide no-op delay implementation Flashrom has several magic programmer_delay() calls scattered throughout its codebase (see cli_classic.s/main() and flashrom.c/flashrom_image_write(), at least). These delays are superfluous for the linux_mtd programmer, because it's an opaque programmer in which all protocol details are handled by the kernel driver. Stub out the delay function, so we don't waste up to 1.1 seconds of needless delay time, depending on the operation and CLI vs libflashrom usage. BUG=b:325539928 TEST=`elogtool add 0xa7` on a linux_mtd system (such as a Mediatek Chromebook): Time improvement - reduces from ~1.5s wall clock to about 0.4s CPU usage: -90%, as most of the CPU cycles (before this change) are spent in needless magic-delay busy loops Change-Id: I3ac69d3fd7cfc689c5b32c130d044516ed846c29 Signed-off-by: Brian Norris <briannorris(a)chromium.org> --- M linux_mtd.c 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/80771/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/80771?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: I3ac69d3fd7cfc689c5b32c130d044516ed846c29 Gerrit-Change-Number: 80771 Gerrit-PatchSet: 2 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2024