June 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: tests: Do not run a test if its driver is not built
by Anastasia Klimchuk (Code Review) 22 Jun '21

22 Jun '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55295 to look at the new patch set (#4). Change subject: tests: Do not run a test if its driver is not built ...................................................................... tests: Do not run a test if its driver is not built For all tests that exist as of today, drivers are built by default, however config options can be disabled and in that case test should not be run. Technically, this is done by skipping the test scenario, so the test itself is running empty. BUG=b:181803212 TEST=1) Tested by adding into tests/meson.build -DCONFIG_xxx=0 4 times (for every driver with test), and then running ninja test Result: corresponding test is running empty, all other tests are running fine 2) Running ninja test with default config settings (everything is enabled, no overriding in test meson). Result: all tests are running. 3) Replacing one of config options in the patch with CONFIG_JLINK_SPI which is disabled by default. Result: corresponding test is running empty. Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/55295/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/55295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Gerrit-Change-Number: 55295 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Guard sys/io.h the same as in hwaccess_x86_io.h
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55741 ) Change subject: tests: Guard sys/io.h the same as in hwaccess_x86_io.h ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: not fully tested (see other message) -- To view, visit https://review.coreboot.org/c/flashrom/+/55741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Gerrit-Change-Number: 55741 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Jun 2021 23:47:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Guard sys/io.h the same as in hwaccess_x86_io.h
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55741 to look at the new patch set (#2). Change subject: tests: Guard sys/io.h the same as in hwaccess_x86_io.h ...................................................................... tests: Guard sys/io.h the same as in hwaccess_x86_io.h Inclusion of this header in hwaccess_x86_io.h is guarded by `defined(__linux__) || defined(__GLIBC__)` , in addition to that inclusion of hwaccess_x86_io.h into hwaccess.h is guarded by IS_X86. Combining these two together, this patch adds the same guards for unittest header file, so that test build don't break on non-x86. This is a follow up on commit 21e22ba8a7750f1cfe5cd3323e3137695ffef0a4 which introduced hwaccess_x86_io_unittest.h BUG=b:181803212 TEST=builds and ninja test on x86 Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M hwaccess_x86_io_unittest.h 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/55741/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Gerrit-Change-Number: 55741 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 ) Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... Patch Set 19: (2 comments) File hwaccess_x86_io_unittest.h: https://review.coreboot.org/c/flashrom/+/51487/comment/5a637deb_5e7d9584 PS19, Line 32: > This header needs guards for x86 as it's inclusion will result in attempts by the toolchain to resol […] Thank you so much for spotting this! I missed it, sorry for that! I created CB:55741 because I am not sure how urgent is this (I understand tests are broken on non-x86). It is not completely tested yet, I wanted to see how it breaks and how it is fixed on arm, sorry it is taking long time for me :\ https://review.coreboot.org/c/flashrom/+/51487/comment/39ccff80_b0acbd34 PS19, Line 41: #include <sys/io.h> > Inclusion of this header in hwaccess_x86_io. […] Thank you so much! I used this in CB:55741 (see also my other reply). -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 19 Gerrit-Owner: 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 21 Jun 2021 23:40:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Guard sys/io.h the same as in hwaccess_x86_io.h
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55741 ) Change subject: tests: Guard sys/io.h the same as in hwaccess_x86_io.h ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I am still in progress of trying to test this on arm. I want to see how it breaks, and then how it is fixed with this patch. But I am not sure how urgent this fix is, maybe it is urgent - so I created this patch anyway. -- To view, visit https://review.coreboot.org/c/flashrom/+/55741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Gerrit-Change-Number: 55741 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 21 Jun 2021 23:29:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Guard sys/io.h the same as in hwaccess_x86_io.h
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55741 ) Change subject: tests: Guard sys/io.h the same as in hwaccess_x86_io.h ...................................................................... tests: Guard sys/io.h the same as in hwaccess_x86_io.h Inclusion of this header in hwaccess_x86_io.h is guarded by `defined(__linux__) || defined(__GLIBC__)` , in addition to that inclusion of hwaccess_x86_io.h into hwaccess.h is guarded by IS_X86. Combining these two together, this patch adds the same guards for unittest header file, so that test build don't break on non-x86. BUG=b:181803212 TEST=builds and ninja test on x86 Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M hwaccess_x86_io_unittest.h 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/55741/1 diff --git a/hwaccess_x86_io_unittest.h b/hwaccess_x86_io_unittest.h index 7bffc96..67a92e6 100644 --- a/hwaccess_x86_io_unittest.h +++ b/hwaccess_x86_io_unittest.h @@ -38,7 +38,9 @@ #define INL(p) test_inl(p) #include <stdint.h> +#if IS_X86 && defined(__linux__) || defined(__GLIBC__) #include <sys/io.h> +#endif /* All functions below are mocked in unit tests. */ -- To view, visit https://review.coreboot.org/c/flashrom/+/55741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Gerrit-Change-Number: 55741 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Michael Niewöhner (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: Well, Tuxedo sells Clevo notebooks. Clevo uses ITE EC with ITE EC firmware framework. This code is *NOT* tuxedo-specific nor Clevo-specific, but ITE-specific and can be used with other ITE ECs as well (I know, because I know the original ITE tool - yes, a single one ;-) ). So, please do not artificially restrict this to Tuxedo. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Jun 2021 21:40:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Do not run a test if its driver is not built
by Nico Huber (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55295 ) Change subject: tests: Do not run a test if its driver is not built ...................................................................... Patch Set 3: (1 comment) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/55295/comment/2a328f60_73f1cb60 PS1, Line 35: #if CONFIG_DUMMY == 1 : void dummy_init_and_shutdown_test_success(void **state) : { : run_lifecycle(state, PROGRAMMER_DUMMY, "bus=parallel+lpc+fwh+spi"); : } : #endif /* CONFIG_DUMMY */ > Actually, this needs more work, I tried as an experiment to use a config option that is not enabled […] "yes"/"no" is only used on the user end. Both Meson and the Makefile always use `=1` for the `-D` arguments. However, what is rather unfortunate, they only ever provide `1` and never `0`. So the only way to use it is useful is indeed with #if. :-/ I think this should change... but that's not a reason to postpone your patch. If you want, you can go on with #if. -- To view, visit https://review.coreboot.org/c/flashrom/+/55295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Gerrit-Change-Number: 55295 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Jun 2021 21:10:38 +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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Michał Żygowski (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (5 comments) File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/55715/comment/deae2c33_b4c6c0ad PS1, Line 648: .SS > Was this removed on purpose? No. Cleaned up this change. File tuxedo_ec.c: https://review.coreboot.org/c/flashrom/+/55715/comment/1ed61f58_9e677399 PS1, Line 235: len % BYTES_PER_BLOCK != 0 && len < BYTES_PER_BLOCK > It should be multiple of BYTES_PER_BLOCK Done https://review.coreboot.org/c/flashrom/+/55715/comment/af2ee5e5_b9dd583e PS1, Line 433: len % BYTES_PER_BLOCK != 0 && len < BYTES_PER_BLOCK > Are you sure this check is supposed to use an AND? Shouldn't it be checking that the length is a mul […] Done https://review.coreboot.org/c/flashrom/+/55715/comment/51d2d727_792443b9 PS1, Line 482: if (!ec_wait_for_obuf(ctx_data->control_port, : EC_MAX_STATUS_CHECKS * 3)) : return 1; : : if (INB(ctx_data->data_port) == 0xf8) : return 0; > Looks like this is open-coding `tuxedo_ec_read_byte` because the `max_checks` parameter isn't expose […] Added max_checks to all ACPI EC API calls and removed this open-coding https://review.coreboot.org/c/flashrom/+/55715/comment/888acbfc_f802615a PS1, Line 504: ++to_chunk > The check below should prevent it. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Jun 2021 13:32:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Michał Żygowski (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Hello Felix Singer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55715 to look at the new patch set (#4). Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops Add a new programmer supporting TUXEDO Embedded Controllers. Tested on TUXEDO InfinityBook S 14 Gen6 and 15 Gen6 with EC firmware updates from IBV. Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 --- M Makefile M flashrom.8.tmpl M flashrom.c M programmer.h M programmer_table.c A tuxedo_ec.c A tuxedo_ec.h 7 files changed, 972 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/55715/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021