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

[S] Change in flashrom[main]: serprog: Add SPI Mode and CS Mode commands
by Anastasia Klimchuk (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Funkeleinhorn, Urja Rannikko. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81428?usp=email ) Change subject: serprog: Add SPI Mode and CS Mode commands ...................................................................... Patch Set 1: Code-Review+2 (2 comments) Patchset: PS1: Urja, good day, I thought maybe you are still around and have some interest to review the update to serprog protocol? Thank you! PS1: Thank you for detailed explanation, I think I understand now. > What do you mean with add implementation to upstream serprog? I was wondering, but you answered just above these words: > so far I only add the commands to the documentation/the spec but do not implement them in flashrom. I wasn't looking into details, but I just quickly looked into the code of avrgirl serprog-programmer and noticed that S_SPI_MODE and S_CS_MODE are handled there, this is where my question came from. I agree supporting dual or quad SPI is a good idea. But I think this patch can go ahead, doesn't have to wait. > Yes sure I am up for reviewing changes to the serprog documentation. Great. I really appreciate it! -- To view, visit https://review.coreboot.org/c/flashrom/+/81428?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: Idb5a9a3710fede322def5191d68b7fba0e135292 Gerrit-Change-Number: 81428 Gerrit-PatchSet: 1 Gerrit-Owner: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Attention: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-Comment-Date: Sun, 24 Mar 2024 12:40:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: serprog: Add SPI Mode and CS Mode commands
by Funkeleinhorn (Code Review) 23 Mar '24

23 Mar '24

Attention is currently required from: Anastasia Klimchuk. Funkeleinhorn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81428?usp=email ) Change subject: serprog: Add SPI Mode and CS Mode commands ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Nice to meet you, and thank you for the patch! […] Heyy nice to meet you too. Sorry that you missed the discussion on IRC. I didn't know about the Discord and that the main discussions are there now. 1 Yess so far I only add the commands to the documentation/the spec but do not implement them in flashrom. This is because as I understand flashrom is targeted at reading/writing flash chips and the current commands suffice for that. But we could add additional SPI modes to the 0x17 SPI mode command which could be useful for the current flashrom implementation. For example we could allow programmers to support dual or quad SPI which could speed up read/writes on supported chips and programmers. I hope that makes some things more clear. What do you mean with add implementation to upstream serprog? Are you talking about adding support for flashing AVR microcontrollers to flashrom? I so far considered this out of scope for the project as it would require some bigger changes like also implementing the AVR ISP protocol. 2 It would be cool to have some group to coordinate the serprog protocol spec across projects using it but as I know there is no central authority or working group for the serprog protocol so far. This is why I would consider flashrom and its fork flashprog the authority over the serprog spec (serprog-protocol.txt) as the protocol evolved there. The numbers 0x17 and 0x18 where chosen because they are the lowest unassigned ones and I tought it would be nice to have no gaps in the specified commands. But we could choose them arbitarily from the unassigned range. 3 Yes sure I am up for reviewing changes to the serprog documentation. -- To view, visit https://review.coreboot.org/c/flashrom/+/81428?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: Idb5a9a3710fede322def5191d68b7fba0e135292 Gerrit-Change-Number: 81428 Gerrit-PatchSet: 1 Gerrit-Owner: Funkeleinhorn <git(a)funkeleinhorn.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: Sat, 23 Mar 2024 22:00:26 +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]: serprog: Add SPI Mode and CS Mode commands
by Anastasia Klimchuk (Code Review) 23 Mar '24

23 Mar '24

Attention is currently required from: Funkeleinhorn. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81428?usp=email ) Change subject: serprog: Add SPI Mode and CS Mode commands ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Nice to meet you, and thank you for the patch! Sorry I missed your discussion in IRC (I have relocated to flashrom Discord channel some time ago) but in any case you did the right thing to send the patch! I have a few questions, sorry again if that's something you have discussed on IRC earlier, but I think it's good to have relevant info here on the patch. 1 You are adding the new commands to the documentation, but not to implementation? I understand that it is implemented in the forks you linked. Do you have any plans to add implementation to upstream serprog? That would be so cool! 2 Who decides on the serprog spec, is there a spec somewhere? Basically I want to understand why it's 0x17 and 0x18 and not something else. Can any other number be fine if it's implemented, or is it documented somewhere? 3 Lastly, the question which goes further than this patch! flashrom is currently in the process of reviewing/updating documentation and converting it to a new format. There are a few serprog docs, and I am asking people who have knowledge of it, to help review the docs. I will do the boring part of converting to a new format and creating the patch, would you agree to be one of the reviewers for the content? -- To view, visit https://review.coreboot.org/c/flashrom/+/81428?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: Idb5a9a3710fede322def5191d68b7fba0e135292 Gerrit-Change-Number: 81428 Gerrit-PatchSet: 1 Gerrit-Owner: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-Comment-Date: Sat, 23 Mar 2024 12:47:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: ich: Add names for region 5, 9, 10, 11, 12, 13, 15
by Anastasia Klimchuk (Code Review) 23 Mar '24

23 Mar '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81356?usp=email ) Change subject: ich: Add names for region 5, 9, 10, 11, 12, 13, 15 ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81356/comment/c037f319_9bad52f5 : PS1, Line 12: * Incorporate missing region names from https://github.com/coreboot/coreboot/blob/main/util/ifdtool/ifdtool.c for completeness. Thank you for syncing region names in two places! I am curious, is there any official doc from Intel on the region names? I see that the names are from coreboot code, which is good. But I assume coreboot code was written based on some doc? Do you happen to know? File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/81356/comment/fde7f702_a542e10d : PS1, Line 521: msg_pdbg2(" FD BIOS ME GbE Pltf DE BIOS2 Reg7 EC DE2 "); For this line (and diffs below in this file) you have changed the number of spaces between names? So now the whole line is shown as diff, even though the first 5 names haven't changed. If there a reason to change the number of spaces? -- To view, visit https://review.coreboot.org/c/flashrom/+/81356?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: I3b164ce4ae84bfd523fcd8be416c5d13183ed632 Gerrit-Change-Number: 81356 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sat, 23 Mar 2024 07:45:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: serprog: Add SPI Mode and CS Mode commands
by Funkeleinhorn (Code Review) 23 Mar '24

23 Mar '24

Funkeleinhorn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81428?usp=email ) Change subject: serprog: Add SPI Mode and CS Mode commands ...................................................................... serprog: Add SPI Mode and CS Mode commands This commit adds two new commands to the serprog protocol which allow more fine grained control over the SPI bus. This enables more applications over serprog like e.g. flashing AVR microcontrollers. This can be tried with my forks of pico-serprog: https://github.com/funkeleinhorn/pico-serprog/tree/spimode and avrdude: https://github.com/funkeleinhorn/avrgirl/tree/serprog-programmer I announced this change in flashrom and flashprog IRC channels and got overall positive feedback in the flashprog channel. The same changes will be sent to flashprog to prevent diverging specs. Change-Id: Idb5a9a3710fede322def5191d68b7fba0e135292 Signed-off-by: Funkeleinhorn <git(a)funkeleinhorn.com> --- M Documentation/serprog-protocol.txt 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/28/81428/1 diff --git a/Documentation/serprog-protocol.txt b/Documentation/serprog-protocol.txt index 3d4aa1a..d001ceb 100644 --- a/Documentation/serprog-protocol.txt +++ b/Documentation/serprog-protocol.txt @@ -36,6 +36,8 @@ 0x14 Set SPI clock frequency in Hz 32-bit requested frequency ACK + 32-bit set frequency / NAK 0x15 Toggle flash chip pin drivers 8-bit (0 disable, else enable) ACK / NAK 0x16 Set SPI Chip Select 8-bit ACK / NAK +0x17 Set SPI Mode 8-bit ACK / NAK +0x18 Set CS Mode 8-bit ACK / NAK 0x?? unimplemented command - invalid. @@ -93,6 +95,21 @@ 0x16 (S_SPI_CS): Set which SPI Chip Select pin to use. This operation is immediate, meaning it doesn't use the operation buffer. + 0x17 (S_SPI_MODE): + Set which SPI Mode to use for 0x13 O_SPIOP commands. + This operation is immediate, meaning it doesn't use the operation buffer. + The current defined modes are: + 0x00: SPI Half Duplex (default) + 0x01: SPI Full Duplex + 0x18 (S_CS_MODE): + Set which CS Mode to use. The CS Mode determines the CS behaviour. + This allows manual control over the CS. + This operation is immediate, meaning it doesn't use the operation buffer. + The current defined modes are: + 0x00: CS Auto Mode. The CS gets selected before 0x13 O_SPIOP commands and + deselected afterwards. (default) + 0x01: CS Selected. The CS will be selected until another mode is set. + 0x02: CS Deselected. The CS will be deselected until another mode is set. About mandatory commands: The only truly mandatory commands for any device are 0x00, 0x01, 0x02 and 0x10, but one can't really do anything with these commands. -- To view, visit https://review.coreboot.org/c/flashrom/+/81428?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: Idb5a9a3710fede322def5191d68b7fba0e135292 Gerrit-Change-Number: 81428 Gerrit-PatchSet: 1 Gerrit-Owner: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: flashchips: Add Zetta Device ZD25LQ128
by Anastasia Klimchuk (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Anton Samsonov, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81392?usp=email ) Change subject: flashchips: Add Zetta Device ZD25LQ128 ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/81392?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: I5cb20158e81ab109f16958285b8787858efb4831 Gerrit-Change-Number: 81392 Gerrit-PatchSet: 1 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 22 Mar 2024 09:51:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L12850F
by Anastasia Klimchuk (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Alexander Goncharov, DZ, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81350?usp=email ) Change subject: flashchips: Add support for MXIC MX25L12850F ...................................................................... Patch Set 1: Code-Review+1 (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81350/comment/f4711261_b90961e8 : PS1, Line 10: wp Could you please mention the commands you ran? Since there are multiple commands for WP, if you list the ones you ran that would be great. Patchset: PS1: > MX25L12845E and MX25L12865E have no configuration register. Yes right, the difference is in configuration register which changes the way `.reg_bits` need to be defined for the chips. Existing entry for `MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F` has no reg_bits currently, but if there were to be added at some point that would be different than what MX25L12850F has. Meanwhile the newer model MX25L12850F can have all set of operations defined, so users can use all this and not be constrained by older models. For complete context, existing monolithic entry `MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F` can be split further because out of the five models, they have would slightly different reg_bits config. But they can be split later, if at some point write-protect support would be added to any of them (which means you would have to define exact reg_bits). Also Joursoir, I am so happy to see you here again !! :) PS1: Daniel thank you! I think it's much better to update one chip at a time, one per commit. So you did the right thing. File flashchips.c: https://review.coreboot.org/c/flashrom/+/81350/comment/581c3430_73fc5317 : PS1, Line 9051: MACRONIX_MX25L12805D Could you please append the model name (MX25L12850F) to a comment in `include/flashchips.h` for the macro `MACRONIX_MX25L12805D` ? I know it's a long comment already, but let's add one more id at the end of it. -- To view, visit https://review.coreboot.org/c/flashrom/+/81350?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: I71ac70d273904f94d015401f9d8df587084efad0 Gerrit-Change-Number: 81350 Gerrit-PatchSet: 1 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> 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: DZ <danielzhang(a)mxic.com.cn> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 22 Mar 2024 07:50:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: DZ <danielzhang(a)mxic.com.cn> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc/dev_guide: Add section about Jenkins build, and scan-build
by Anastasia Klimchuk (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Peter Marheine, Stefan Reinauer, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81261?usp=email ) Change subject: doc/dev_guide: Add section about Jenkins build, and scan-build ...................................................................... Patch Set 2: (1 comment) File doc/dev_guide/development_guide.rst: https://review.coreboot.org/c/flashrom/+/81261/comment/5c5f90fc_2fc7a900 : PS1, Line 226: need to > "should" seems better, since you're saying authors should review the report but aren't required to. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/81261?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: I416b632c55d1ceb925456ac8c8947dfbcef2e888 Gerrit-Change-Number: 81261 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(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-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 22 Mar 2024 06:48:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc/dev_guide: Add section about Jenkins build, and scan-build
by Anastasia Klimchuk (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Anastasia Klimchuk, Stefan Reinauer, Thomas Heijligen. Hello Peter Marheine, Stefan Reinauer, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81261?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+2 by Stefan Reinauer, Verified+1 by build bot (Jenkins) Change subject: doc/dev_guide: Add section about Jenkins build, and scan-build ...................................................................... doc/dev_guide: Add section about Jenkins build, and scan-build Change-Id: I416b632c55d1ceb925456ac8c8947dfbcef2e888 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/dev_guide/development_guide.rst 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/81261/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/81261?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: I416b632c55d1ceb925456ac8c8947dfbcef2e888 Gerrit-Change-Number: 81261 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(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-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

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

22 Mar '24

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Chen, Nikolai Artemiev, Stefan Reinauer. Peter Marheine 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) Commit Message: https://review.coreboot.org/c/flashrom/+/80806/comment/66216798_cba81099 : PS2, Line 10: Linux makes this clearer > I think `#ifdef __linux__` is a good plan B , but before we come to it let me try invite few more pe […] I looked at what FreeBSD does: 1. `sys_clock_getres` calls `kern_clock_getres` 2. `kern_clock_geres(CLOCK_MONOTONIC)` computes the clock period using `tc_getfrequency` 3. `tc_getfrequency` uses the declared frequency of the first `struct timehands` in the system 4. The primary time handler is chosen in `tc_init` based on each counter's declared quality, where larger values are better The RISC-V timecounter driver (`sys/riscv/riscv/timer.c`) declares a constant high quality (1000) and its frequency is defined by the `timebase-frequency` shared by all CPUs in the system. Among device trees in `sys/contrib/device-tree/src/riscv/`, it looks like this varies between 1 MHz and 24 MHz. This traces back to `clock_gettime` through `nanouptime` and eventually `tc_delta` on the active timecounter, which calls its `tc_get_timecount`. In the RISC-V driver, that eventually executes a `rdtime` instruction which reads the hardware counter. --- So at least FreeBSD on RISC-V reports accurate resolution for `CLOCK_MONOTONIC`, and it looks like other platforms are similar. It looks like the x86 PIIX timer driver has a known fixed frequency of ~3.57 MHz, for instance. I think Linux has an unusual interpretation here. Another side of this is that Linux does this because it can't depend on a stable high-resolution timer existing, so it's conservative. On some systems it might end up needing to use an i8253 timer which has exactly the same period as one tick (== the reported resolution in low-resolution mode) so it's useful to use the calibrated delay loop. I wonder if it might be more helpful to make this a user-visible option maybe with a compile-time default? There's no harm in using OS timers if they're slower than desired, just chip delays may tend to be longer than they need to be so programming might be slower than it needs to be. If it's exposed as an option, users can judge whether it saves time to skip calibration and distributors can choose a default according to expected typical use. -- 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: Peter Marheine <pmarheine(a)chromium.org> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 22 Mar 2024 06:17:35 +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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2024