April 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: pony_spi.c: Extract out get_params to simplify init
by Anastasia Klimchuk (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Light. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63130 ) Change subject: pony_spi.c: Extract out get_params to simplify init ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/63130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac Gerrit-Change-Number: 63130 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Apr 2022 00:45:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests/: Factor out all link-time wrapped func
by Anastasia Klimchuk (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62777 ) Change subject: tests/: Factor out all link-time wrapped func ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: That's a good idea, and I was thinking about something similar too, so thank you! :) When I was thinking about it, I was going to have port io wraps it their own file. Second thing is that this patch needs to, if possible, wait for CB:63489 to get merged. I suggest, don't do anything right now (just to save time), let's get meson compiler options done and then come back to this one? is that okay? -- To view, visit https://review.coreboot.org/c/flashrom/+/62777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5e18c9390c064a3112caa7f5778569ae478552d Gerrit-Change-Number: 62777 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 26 Apr 2022 00:15:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Introduce HSFC_FCYCLE_MASK(n) macro
by Anastasia Klimchuk (Code Review) 25 Apr '22

25 Apr '22

Attention is currently required from: Subrata Banik, Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62891 ) Change subject: ichspi: Introduce HSFC_FCYCLE_MASK(n) macro ...................................................................... Patch Set 11: (1 comment) Patchset: PS6: > Also without this CL, the Flash Cycle bit field width is wrong for latest chipset. It's 4 bit width on PCH as per EDS but without this CL, we are still considering it 2 bit. Sorry I just noticed this. It is still true, that this patch fixes a bug? The comment is about Patchset6, and code has changed since then, so I am not sure is this still true? The latest patchset does not make changes to the code itself, only to macro definitions. If the patch does fix a bug, the info needs to be added to commit message. (if not then not). -- To view, visit https://review.coreboot.org/c/flashrom/+/62891 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id222304165610c7ae48e365d72ec8fdeea51c51d Gerrit-Change-Number: 62891 Gerrit-PatchSet: 11 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 25 Apr 2022 23:42:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Introduce HSFC_FDBC_VAL(n) macro
by Anastasia Klimchuk (Code Review) 25 Apr '22

25 Apr '22

Attention is currently required from: Nico Huber, Subrata Banik. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62892 ) Change subject: ichspi: Introduce HSFC_FDBC_VAL(n) macro ...................................................................... Patch Set 10: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie2512c85de9fc21286234b97f5842ecef1729787 Gerrit-Change-Number: 62892 Gerrit-PatchSet: 10 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Mon, 25 Apr 2022 23:24:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Introduce HSFC CYCLE READ/WRITE/ERASE macros
by Anastasia Klimchuk (Code Review) 25 Apr '22

25 Apr '22

Attention is currently required from: Subrata Banik, Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62868 ) Change subject: ichspi: Introduce HSFC CYCLE READ/WRITE/ERASE macros ...................................................................... Patch Set 17: Code-Review+1 (1 comment) Patchset: PS14: > I realised my comment looked like thinking aloud :) Comparing final binary is even a bit easier to d […] This one [comment] left -- To view, visit https://review.coreboot.org/c/flashrom/+/62868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ea74b668e5f8d8e4b3da2a8ad8b81f1813e1e80 Gerrit-Change-Number: 62868 Gerrit-PatchSet: 17 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 25 Apr 2022 23:16:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher: move struct declaration & probe_variable_size to spi.(h|c)
by Nico Huber (Code Review) 25 Apr '22

25 Apr '22

Attention is currently required from: Namyoon Woo, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63831 ) Change subject: dummyflasher: move struct declaration & probe_variable_size to spi.(h|c) ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Might be best to figure out first what the code tries to express. If this is only about emulating something with a command-line configurable size, it should implement an `opaque` master instead. That would leave no doubt where to place the code and magically use the proper pointers (no link- time trouble). -- To view, visit https://review.coreboot.org/c/flashrom/+/63831 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic93c8b9ba7b9f7ce5fe49326c8de34070ca83a2e Gerrit-Change-Number: 63831 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Namyoon Woo <namyoon(a)google.com> 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-Comment-Date: Mon, 25 Apr 2022 22:34:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Reduce read size for FT232H
by Peter Farley (Code Review) 25 Apr '22

25 Apr '22

Attention is currently required from: Nico Huber, Angel Pons. Peter Farley has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63798 ) Change subject: ft2232_spi: Reduce read size for FT232H ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/63798/comment/826d7b87_7e23e539 PS1, Line 9: FT232H-based devices are unable to read over 256 bytes of flash in one > One more general thought: Is there any other software running that might > try to access the FT232H device? any kernel driver or anything using libusb > should not cause any trouble, but who knows... The only thing I notice is that when the device is attached, it gets associated with the `ftdi_sio` driver. But after `flashrom` takes control, it appears to release it. Unloading and blacklisting that module doesn't make a difference either. > What would be interesting is if it > always stops at that point or if always on a 4096B boundary. I guess it > would suffice to re-run the capture and check if/how the number of frames > changes. The 4096B these URBs request are probably the default read chunk > size of libftdi. So lowering this (maybe to an odd multiple of 512, e.g. > 1536) to see if it still fails after a full chunk might be interesting as > well. If it does it would further increase my suspicion that it's a soft- > ware issue. Testing it some more, even without changing the chunk size, the number of received packets is not constant. It seems I might have been lucky getting a full 64K yesterday, today I am seeing anywhere between 0 and 5 4080B blocks. Due to its apparent randomness, I can't say definitively, but decreasing the chunk size from 4096 to 1536 does increase the average RX packet count (over only 5 iterations), from 2 to 8. -- To view, visit https://review.coreboot.org/c/flashrom/+/63798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a45fb8ac8ff37bbf01b0cb6b88851cf74af495d Gerrit-Change-Number: 63798 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Farley <far.peter1(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 25 Apr 2022 22:03:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Peter Farley <far.peter1(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Close GitHub PRs and issues automatically
by Anastasia Klimchuk (Code Review) 25 Apr '22

25 Apr '22

Attention is currently required from: Felix Singer, Martin L Roth, Thomas Heijligen, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63671 ) Change subject: Close GitHub PRs and issues automatically ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: > My reply became much bigger as I intended it to be, but I think it's a good analysis at least :) […] I agree with Felix's analysis. -- To view, visit https://review.coreboot.org/c/flashrom/+/63671 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8076f0fb964970ffd05f355b9d1e33a65aa7a3c8 Gerrit-Change-Number: 63671 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Martin L Roth <martinroth(a)google.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 25 Apr 2022 21:40:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Martin L Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [WIP] meson: rework the programmer selection
by Thomas Heijligen (Code Review) 25 Apr '22

25 Apr '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63724 ) Change subject: [WIP] meson: rework the programmer selection ...................................................................... Patch Set 7: (2 comments) Patchset: PS2: > Will not build if not one or more usb programmer are selected. […] CB:63832 PS2: > Will not build when the `dummy` programmer is not selected. […] CB:63831 -- To view, visit https://review.coreboot.org/c/flashrom/+/63724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib44b26e3748fc71f116184082b4aed0bb208b4c1 Gerrit-Change-Number: 63724 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 25 Apr 2022 17:42:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: add option to enable or disable tests
by Thomas Heijligen (Code Review) 25 Apr '22

25 Apr '22

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63832 ) Change subject: meson: add option to enable or disable tests ...................................................................... meson: add option to enable or disable tests During development it can be usefull to disable unit testing. Run `meson -Dtests=true` to enable tests. (default) Run `meson -Dtests=false` to disable tests. Change-Id: I384c904c577b265dfe36bf46bf07c641bc29de9b Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M meson.build M meson_options.txt 2 files changed, 22 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/63832/1 diff --git a/meson.build b/meson.build index 7e129da..5b74f8c 100644 --- a/meson.build +++ b/meson.build @@ -495,30 +495,29 @@ subdir('util') +if get_option('tests') # unit-test framework cmocka_dep = dependency( - 'cmocka', - fallback: ['cmocka', 'cmocka_dep'], - required: false -) -flashrom_test_dep = declare_dependency( - include_directories : include_directories('.'), - sources : [ - srcs, - 'cli_common.c', - 'cli_output.c', - 'flashrom.c', - ], - compile_args : [ - '-includestdlib.h', - '-includeunittest_env.h', - '-includehwaccess_x86_io_unittest.h' - ], - dependencies : [ - deps, - ], -) + 'cmocka', + fallback: ['cmocka', 'cmocka_dep'], + ) + flashrom_test_dep = declare_dependency( + include_directories : include_directories('.'), + sources : [ + srcs, + 'cli_common.c', + 'cli_output.c', + 'flashrom.c', + ], + compile_args : [ + '-includestdlib.h', + '-includeunittest_env.h', + '-includehwaccess_x86_io_unittest.h' + ], + dependencies : [ + deps, + ], + ) -if cmocka_dep.found() subdir('tests') endif diff --git a/meson_options.txt b/meson_options.txt index 8022c74..2347391 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -42,3 +42,4 @@ option('config_lspcon_i2c_spi', type : 'boolean', value : false, description : 'Parade lspcon USB-C to HDMI protocol translator') option('config_mediatek_i2c_spi', type : 'boolean', value : false, description : 'MediaTek LCD controller') option('config_realtek_mst_i2c_spi', type : 'boolean', value : true, description : 'Realtek MultiStream Transport MST') +option('tests', type : 'boolean', value : 'true', description : 'Build and run unit tests') -- To view, visit https://review.coreboot.org/c/flashrom/+/63832 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I384c904c577b265dfe36bf46bf07c641bc29de9b Gerrit-Change-Number: 63832 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2022