October 2023 - flashrom-gerrit

[XS] Change in flashrom[main]: doc: Add meson test command to TLDR for meson instructions
by Alexander Goncharov (Code Review) 29 Oct '23

29 Oct '23

Attention is currently required from: Anastasia Klimchuk, Jes Klinke, Peter Marheine, Thomas Heijligen. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78689?usp=email ) Change subject: doc: Add meson test command to TLDR for meson instructions ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: There's also TLDR section in `README.rst` file that is used as the main page on our site, so I suggest adding meson test command to that file as well -- To view, visit https://review.coreboot.org/c/flashrom/+/78689?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: I67d5f4decdac15e6a72f4372135dab7d44396594 Gerrit-Change-Number: 78689 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sun, 29 Oct 2023 19:21:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Add Puya P25Q21H/11H/06H
by Alexander Goncharov (Code Review) 29 Oct '23

29 Oct '23

Attention is currently required from: Anastasia Klimchuk, Angel Pons, WereCatf. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58134?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: flashchips: Add Puya P25Q21H/11H/06H ...................................................................... Patch Set 6: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58134/comment/c025e00d_bf1b98c8 : PS6, Line 10: https://datasheet.lcsc.com/szlcsc/1908281813_PUYA-P25Q21H-SSH-IT_C414061.pdf I cannot access this _part_ of the site from Switzerland / Russia. Error Code: Access Denied I used datasheet from this site (https://semic-boutique.com/wp-content/uploads/2016/05/P25Q21H-SSH-IT.pdf) to review the patch File flashchips.c: https://review.coreboot.org/c/flashrom/+/58134/comment/4ee58ba1_f6c7f6fe : PS6, Line 14141: 256 The document that I used says: "3*512-Byte Security Registers With OTP Lock". Is it the same in the one from lcsc.com? -- To view, visit https://review.coreboot.org/c/flashrom/+/58134?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: Idd43145c72607837cb7afa1b007e68eb8e63ebd9 Gerrit-Change-Number: 58134 Gerrit-PatchSet: 6 Gerrit-Owner: WereCatf <werecatf(a)outlook.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: WereCatf <werecatf(a)outlook.com> Gerrit-Comment-Date: Sun, 29 Oct 2023 19:13:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE
by Anastasia Klimchuk (Code Review) 28 Oct '23

28 Oct '23

Attention is currently required from: Edward O'Callaghan, Jes Klinke, Nikolai Artemiev, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email ) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... Patch Set 5: Code-Review+1 (4 comments) Patchset: PS2: > I have added a few more cases to the test. […] Yes, existing test is running `run_basic_lifecycle` which expects the lifecycle to run successfully. There is another method in our test framework, which tests initialisation error (invalid value for programmer param for example). This is `run_init_error_path` here https://review.coreboot.org/plugins/gitiles/flashrom/+/refs/heads/main/test… If you would like to add a test to verify that init fails with invalid value of param, you can use run_init_error_path for that. I suspect `raiden_debug_io` would not need that many mocks, maybe even no mocks at all: if init fails early those mocks are never called. File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/77999/comment/7c68030b_c99e4595 : PS4, Line 873: > I have attempted to merge your suggested paragraph into the existing documentation. Looks really good File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/77999/comment/a1844fc0_7b8a005a : PS5, Line 871: trailing whitespace Otherwise the text is very good! File tests/raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/77999/comment/3154708d_7bf2511b : PS5, Line 112: : snprintf(raiden_debug_param, sizeof(raiden_debug_param), : "address=%d,target=AP", USB_DEVICE_ADDRESS); : run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); : : snprintf(raiden_debug_param, sizeof(raiden_debug_param), : "address=%d,target=ec", USB_DEVICE_ADDRESS); : run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); : : snprintf(raiden_debug_param, sizeof(raiden_debug_param), : "address=%d,target=0", USB_DEVICE_ADDRESS); : run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); : : snprintf(raiden_debug_param, sizeof(raiden_debug_param), : "address=%d,target=1", USB_DEVICE_ADDRESS); : run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); This is great, thank you so much for adding test coverage! The only thing: let's make each test case a separate test. Which means, initial test case stays in `raiden_debug_basic_lifecycle_test_success` function, and the other 4 you add into their own test functions. I don't mind long names for test functions, so you can name them for example: raiden_debug_targetAP_basic_lifecycle_test_success raiden_debug_targetEC_basic_lifecycle_test_success raiden_debug_target0_basic_lifecycle_test_success raiden_debug_target1_basic_lifecycle_test_success The definitions of `raiden_debug_fallback_open_state` and `raiden_debug_io` at the beginning of each function you can just copy-paste. Then you need to add 4 new functions into: tests/tests.c tests/tests.h and at the bottom of this file add SKIP_TEST macro. And that should be it! -- To view, visit https://review.coreboot.org/c/flashrom/+/77999?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: I03bf4f3210186fb5937b42e298761907b03e08b7 Gerrit-Change-Number: 77999 Gerrit-PatchSet: 5 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 28 Oct 2023 09:20:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Jes Klinke <jbk(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE
by Jes Klinke (Code Review) 27 Oct '23

27 Oct '23

Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, Nikolai Artemiev, Thomas Heijligen. Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email ) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... Patch Set 5: (2 comments) Patchset: PS2: > Yes, I looked at the test now (tests/raiden_debug_spi.c), this parameter is not set. […] I have added a few more cases to the test. In particular, I have noticed that if I put something like "target=abc" in the `raiden_debug_param` instead of the valid values such as "ap", "ec" or numbers, then the test will fail. File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/77999/comment/c587c7d9_16bd314a : PS4, Line 873: > There is a lot of information, I looked at your commit message and I really like the paragraph that […] I have attempted to merge your suggested paragraph into the existing documentation. -- To view, visit https://review.coreboot.org/c/flashrom/+/77999?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: I03bf4f3210186fb5937b42e298761907b03e08b7 Gerrit-Change-Number: 77999 Gerrit-PatchSet: 5 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 27 Oct 2023 21:04:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jes Klinke <jbk(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE
by Jes Klinke (Code Review) 27 Oct '23

27 Oct '23

Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, Jes Klinke, Nikolai Artemiev, Thomas Heijligen. Hello Anastasia Klimchuk, Edward O'Callaghan, Nikolai Artemiev, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/77999?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+1 by Anastasia Klimchuk, Verified+1 by build bot (Jenkins) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... raiden: Support target index with generic REQ_ENABLE Some devices such as the GSC knows how it is wired to AP and EC flash chips, and can be told which specific device to talk to. Other devices such as Servo Micro and HyperDebug are generic, and do not know how they are wired, the caller is responsible for first configure the appropriate MUXes or buffers, and then tell the debugger which port to use (Servo Micro has just one SPI port, HyperDebug is the first that has multiple). The Raiden protocol allows both the cases of USB devices knowing their wiring and not. If I were to declare the protocol in Rust, this is how the information of the Raiden protocol "enable request" would be encoded: ``` enum { EnableGeneric(u8), EnableAp, EnableEc, ... } ``` The first label `EnableGeneric(u8)` is to be used with HyperDebug that does not know how its ports are wired, and allow access by index. The other labels `EnableAp` and `EnableEc` are to be used with the GSC. The actual transmission of the enum above uses the bRequest and low byte of wValue of a USB control request, but that is a detail and not conceptually important. Until now, `-p raiden_debug_spi:target=AP` or `...:target=EC` could be used to make flashrom use `EnableAp` or `EnableEc`, and if neither was given, it would default to `EnableGeneric`, which now that wValue is used means `EnableGeneric(0)`. I find it rather straight-forward, that `-p raiden_debug_spi:target=1`, `...:target=2`, etc. should translate to `EnableGeneric(1)`, etc. This patchset achieves this, by adding a second 16-bit parameter value, next to request_enable. I have tested that flashrom can detect the same Winbond flash chip "W25Q128.V..M" with two different Raiden USB devices as below. TEST=flashrom -p raiden_debug_spi:serial=0701B044-91AC3132,target=AP TEST=flashrom -p raiden_debug_spi:serial=205635783236,target=1 Signed-off-by: Jes B. Klinke <jbk(a)chromium.org> Change-Id: I03bf4f3210186fb5937b42e298761907b03e08b7 --- M doc/classic_cli_manpage.rst M raiden_debug_spi.c M tests/raiden_debug_spi.c 3 files changed, 52 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/77999/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/77999?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: I03bf4f3210186fb5937b42e298761907b03e08b7 Gerrit-Change-Number: 77999 Gerrit-PatchSet: 5 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in flashrom[main]: doc: Add meson test command to TLDR for meson instructions
by Anastasia Klimchuk (Code Review) 27 Oct '23

27 Oct '23

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/78689?usp=email ) Change subject: doc: Add meson test command to TLDR for meson instructions ...................................................................... doc: Add meson test command to TLDR for meson instructions Running tests is one of the regular things to do in dev process, same as compile, so it should be highlighted in the TLDR section of the document. Change-Id: I67d5f4decdac15e6a72f4372135dab7d44396594 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/dev_guide/building_from_source.rst 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/89/78689/1 diff --git a/doc/dev_guide/building_from_source.rst b/doc/dev_guide/building_from_source.rst index 64250dd..4e50753 100644 --- a/doc/dev_guide/building_from_source.rst +++ b/doc/dev_guide/building_from_source.rst @@ -33,6 +33,7 @@ meson setup builddir meson compile -C builddir + meson test -C builddir meson install -C builddir -- To view, visit https://review.coreboot.org/c/flashrom/+/78689?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: I67d5f4decdac15e6a72f4372135dab7d44396594 Gerrit-Change-Number: 78689 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE
by Anastasia Klimchuk (Code Review) 27 Oct '23

27 Oct '23

Attention is currently required from: Edward O'Callaghan, Jes Klinke, Nikolai Artemiev, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email ) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... Patch Set 4: Code-Review+1 (2 comments) Patchset: PS2: > I do not see any mention of existing values `target=AP` or `target=EC` in `raiden_debug_spi. […] Yes, I looked at the test now (tests/raiden_debug_spi.c), this parameter is not set. Which means it goes by default flow which is, from the code... request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE. I am not sure whether this is realistic (or even valid) scenario, but because unit tests do not talk to actual hardware, only emulate it, this can explain why unit test passes. For some more content, unit tests are relatively recent in comparison with the most of code base (started in 2021 vs ~2000), so there is of course lots of room for improvement and adding more coverage. How to run tests: instructions here https://flashrom.org/dev_guide/building_from_source.html#unit-tests If you don't have meson build dir yet, you need to scroll up in the doc. tldr mkdir builddir meson setup builddir meson compile -C builddir meson test -C builddir Also you can run `ninja test` inside builddir once you setup. For adding new test: You can copy `raiden_debug_basic_lifecycle_test_success` and add target param into `raiden_debug_param` string. Perhaps all the mocks will apply without any change (I would try)! You need to add your new test function into tests/tests.c and tests/tests.h If new test run, that's amazing! If it fails, or does not run, you can still push new patchset and tell what the error is. It is possible that maybe some of the mock functions (you can see them in `raiden_debug_io` need to be copied->adjusted) but first lets see whether test runs. I am plannig to write a doc "How to add unit test", I will use this my comment as a starting point :) File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/77999/comment/2bd2c13a_7aa14914 : PS4, Line 873: There is a lot of information, I looked at your commit message and I really like the paragraph that you wrote. Can you copy it here? (and keep it in commit message of course). The paragraph is easy to read, and it explains stuff, also importantly it gives examples of devices. Let's inject this on the line 873 ? > Some devices such as the GSC knows how it is wired to AP and EC flash chips, and can be told which specific device to talk to. Other devices such as Servo Micro and HyperDebug are generic, and do not know how they are wired, the caller is responsible for first configure the appropriate MUXes or buffers, and then tell the debugger which port to use (Servo Micro has just one SPI port, HyperDebug is the first that has multiple). The Raiden protocol allows both the cases of USB devices knowing their wiring and not. -- To view, visit https://review.coreboot.org/c/flashrom/+/77999?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: I03bf4f3210186fb5937b42e298761907b03e08b7 Gerrit-Change-Number: 77999 Gerrit-PatchSet: 4 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 27 Oct 2023 06:21:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Jes Klinke <jbk(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashchips: Add GD25Q128E name to the GD25Q127C/GD25Q128C entry
by Anastasia Klimchuk (Code Review) 27 Oct '23

27 Oct '23

Attention is currently required from: Hsuan-ting Chen, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78348?usp=email ) Change subject: flashchips: Add GD25Q128E name to the GD25Q127C/GD25Q128C entry ...................................................................... Patch Set 2: (2 comments) File flashchips.c: https://review.coreboot.org/c/flashrom/+/78348/comment/93f859e3_7b14fbbd : PS1, Line 6802: /* FIXME: 128E's OTP: 3072B total and doesn't support QPI */ > I have two questions about these: […] 1. Yes, I think you need to separate the two chip definitions. This is what I meant when I said you need to split. You can copypaste existing definition into new one, make necessary changes and adjustments, and put new the one in alphabetical order. At the end there will be two definitions instead of one that it now. Also you will need to update the commit title. 2. If there are 3 different chips with the same IDs, you will need to provide chip name with `-c` or `--chip` options (you can check this option on man page which is also on the web page https://flashrom.org/classic_cli_manpage.html#options) https://review.coreboot.org/c/flashrom/+/78348/comment/78a3141c_58c89996 : PS1, Line 6833: .reg_bits = : { : .srp = {STATUS1, 7, RW}, : .srl = {STATUS2, 0, RW}, : .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, : .tb = {STATUS1, 5, RW}, /* Called BP3 in datasheet, acts like TB */ : .sec = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like SEC */ : .cmp = {STATUS2, 6, RW}, : } > > Are all these registers the same for new chip you are adding […] Great thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/78348?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: I3300671b1cf74b3ea0469b9c5a833489ab4914f5 Gerrit-Change-Number: 78348 Gerrit-PatchSet: 2 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.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: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 27 Oct 2023 05:19:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Add Puya P25Q21H/11H/06H
by Nikolai Artemiev (Code Review) 26 Oct '23

26 Oct '23

Attention is currently required from: Anastasia Klimchuk, Angel Pons, WereCatf. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58134?usp=email ) Change subject: flashchips: Add Puya P25Q21H/11H/06H ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58134?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: Idd43145c72607837cb7afa1b007e68eb8e63ebd9 Gerrit-Change-Number: 58134 Gerrit-PatchSet: 6 Gerrit-Owner: WereCatf <werecatf(a)outlook.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: WereCatf <werecatf(a)outlook.com> Gerrit-Comment-Date: Thu, 26 Oct 2023 22:27:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE
by Jes Klinke (Code Review) 25 Oct '23

25 Oct '23

Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, Nikolai Artemiev, Thomas Heijligen. Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email ) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... Patch Set 4: (7 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/77999/comment/1536cdca_86be7fba : PS2, Line 42: target=1 > Since these are new values for `target` param, please document it on the manpage, here: […] Done https://review.coreboot.org/c/flashrom/+/77999/comment/1cc81745_67b2379c : PS2, Line 45: CL > Just noticed: you need to say "this patch" […] Done https://review.coreboot.org/c/flashrom/+/77999/comment/97759e44_e57e1e0d : PS2, Line 45: archieves > achieves (typo) Done https://review.coreboot.org/c/flashrom/+/77999/comment/dca9dc69_122661b9 : PS2, Line 48: run flashrom with HyperDebug > Could you please list the exact commands that you ran? […] Done Patchset: PS2: > Also, maybe you can add a test for target=1 and 2? […] I do not see any mention of existing values `target=AP` or `target=EC` in `raiden_debug_spi.c`, I think I will need more help on how to create a test for this. How do I even run the existing test of the Raiden SPI protocol? File raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/77999/comment/ff2cc291_f4cdb339 : PS2, Line 1596: request_enable & 0xFF > If this is a no-op, and given your idea to change the signature of `get_target` , maybe this is not […] Acknowledged https://review.coreboot.org/c/flashrom/+/77999/comment/58b39cce_12478b9d : PS2, Line 1596: request_enable & 0xFF, : (request_enable >> 8) & 0xFF, > Jes, thank you for detailed explanation, I understand much better now! […] The documentation at the start of the file actually mentions only the bulk data on the USB endpoint, not the initial control transfer extended by this patchset. So all if it still applies. -- To view, visit https://review.coreboot.org/c/flashrom/+/77999?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: I03bf4f3210186fb5937b42e298761907b03e08b7 Gerrit-Change-Number: 77999 Gerrit-PatchSet: 4 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 25 Oct 2023 20:25:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jes Klinke <jbk(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 October 2023