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

Change in flashrom[master]: bitbang_spi.c: Do not set MOSI during initialization
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49270 ) Change subject: bitbang_spi.c: Do not set MOSI during initialization ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: If we don't care about MOSI's idle state, how about removing the MOSI setting in the read function along with this? -- To view, visit https://review.coreboot.org/c/flashrom/+/49270 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I925d0ae55ae006dfcc6e7fb90364235526c576ef Gerrit-Change-Number: 49270 Gerrit-PatchSet: 1 Gerrit-Owner: 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-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 26 Jun 2021 13:51:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: bitbang_spi: Refactor `set_sck_set_mosi`
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49269 ) Change subject: bitbang_spi: Refactor `set_sck_set_mosi` ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49269/comment/f3c013ad_cf433f81 PS1, Line 11: for those that do not, because MOSI now has more time to stabilize. In case of `.half_period = 0`, this would also mean that we program the next MOSI value as the very next step after the rising SCK edge. Tests would have to show if MOSI is then still stable long enough. -- To view, visit https://review.coreboot.org/c/flashrom/+/49269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia84d32965e56ca563062893ebbefbbea5c35814c Gerrit-Change-Number: 49269 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 26 Jun 2021 12:19:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: nicintel_spi.c: Cache FLA register value
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55720 ) Change subject: nicintel_spi.c: Cache FLA register value ...................................................................... Patch Set 2: Code-Review+1 (2 comments) File nicintel_spi.c: https://review.coreboot.org/c/flashrom/+/55720/comment/73d23472_c0fe7d16 PS2, Line 122: pci_mmio_readl If somebody else is still in charge, could the register value still change? or is the register contents only ever set by the host? If it can still change, we should keep the last value read. https://review.coreboot.org/c/flashrom/+/55720/comment/8c579d58_211cc327 PS2, Line 129: data->fla = pci_mmio_readl(data->spibar + FLA); NB. If we were indeed in charge of the interface, this read shouldn't be necessary. -- To view, visit https://review.coreboot.org/c/flashrom/+/55720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4013a71ec498be977f62fe2531ac8770bfffadcd Gerrit-Change-Number: 55720 Gerrit-PatchSet: 2 Gerrit-Owner: 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-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 26 Jun 2021 12:04:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: nicintel_spi.c: Implement `set_sck_set_mosi` and `set_sck_get_miso`
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Idwer Vollering, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49267 ) Change subject: nicintel_spi.c: Implement `set_sck_set_mosi` and `set_sck_get_miso` ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: (uargh, BIT() macro and parents) -- To view, visit https://review.coreboot.org/c/flashrom/+/49267 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I52a0ae5ff331ee2ed41652eb5c2ed7ebe7253d74 Gerrit-Change-Number: 49267 Gerrit-PatchSet: 3 Gerrit-Owner: 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-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 26 Jun 2021 11:57:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess_x86_io_unittest: Add dummy iopl to avoid including sys/io.h
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55741 ) Change subject: hwaccess_x86_io_unittest: Add dummy iopl to avoid including sys/io.h ...................................................................... hwaccess_x86_io_unittest: Add dummy iopl to avoid including sys/io.h sys/io.h is platform specific, and also in tests environment we don't need real functions anyway. Adding dummy implementation of iopl is sufficient for tests. The rest of io is not needed because hwaccess_x86_io_unittest.h re-defines macros OUTB/INB/etc and those macros evaluate to test-only functions. 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 (same as before) Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/55741 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M tests/hwaccess_x86_io_unittest.h 1 file changed, 11 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/tests/hwaccess_x86_io_unittest.h b/tests/hwaccess_x86_io_unittest.h index 7bffc96..2a4cd5f 100644 --- a/tests/hwaccess_x86_io_unittest.h +++ b/tests/hwaccess_x86_io_unittest.h @@ -38,7 +38,17 @@ #define INL(p) test_inl(p) #include <stdint.h> -#include <sys/io.h> + +/* + * Dummy implementation of iopl from sys/io.h. + * sys/io.h by itself is platform-specific, so instead of including + * the header we just have this dummy function, which is sufficient + * for test purposes. + */ +static inline int iopl(int level) +{ + return 0; +} /* 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: 7 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: sio.c: Put generic Super I/O access scattered throughout files together
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Michał Żygowski. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55849 ) Change subject: sio.c: Put generic Super I/O access scattered throughout files together ...................................................................... Patch Set 1: (1 comment) File sio.c: https://review.coreboot.org/c/flashrom/+/55849/comment/20eb49fe_026a0d80 PS1, Line 4: * Copyright (C) 2021, TUXEDO Computers GmbH > If you got this code from somewhere else, how can it be copyrighted […] Also the date is wrong, copyright starts with the original creative action. -- To view, visit https://review.coreboot.org/c/flashrom/+/55849 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie36370c22edb05ee59b036859b44a968bcc37980 Gerrit-Change-Number: 55849 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Sat, 26 Jun 2021 11:41:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: sio.c: Put generic Super I/O access scattered throughout files together
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Michał Żygowski. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55849 ) Change subject: sio.c: Put generic Super I/O access scattered throughout files together ...................................................................... Patch Set 1: (5 comments) File programmer.h: https://review.coreboot.org/c/flashrom/+/55849/comment/e9cfcbb0_ded41c42 PS1, Line 215: /* board_enable.c */ You just moved them out of there. https://review.coreboot.org/c/flashrom/+/55849/comment/d054d2cd_14e0af99 PS1, Line 224: void sio_switch_ldn(uint16_t port, uint8_t ldn); : uint16_t sio_get_iobase(uint16_t port, uint8_t io_bar_number); : uint16_t sio_read_id(uint16_t port, uint8_t io_bar_number); : bool sio_is_ldn_enabled(uint16_t port); Why create APIs before you have a user? File sio.c: https://review.coreboot.org/c/flashrom/+/55849/comment/8248f2f2_ad08e511 PS1, Line 4: * Copyright (C) 2021, TUXEDO Computers GmbH If you got this code from somewhere else, how can it be copyrighted by TUXEDO? https://review.coreboot.org/c/flashrom/+/55849/comment/c8f96218_17043aac PS1, Line 8: * the Free Software Foundation; version 2 of the License. It's an unfortunate situation, but if you pull code from files with different licenses, you should maintain them individually (or keep the code in separate files). https://review.coreboot.org/c/flashrom/+/55849/comment/628f7090_48f74b38 PS1, Line 31: } I don't think we should encourage anyone to use such a function by making it a public API. I expect you only need a tiny bit of code out of probe_superio_ite() to confirm if the EC you expect is really there? Given that we are supposed to know the board flashrom runs on, we should already know the EC. Probing is for the case that we don't know the board. -- To view, visit https://review.coreboot.org/c/flashrom/+/55849 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie36370c22edb05ee59b036859b44a968bcc37980 Gerrit-Change-Number: 55849 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Sat, 26 Jun 2021 11:40:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: util: Give the udev-rules file a meaningful name
by Nico Huber (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Felix Singer, Paul Menzel. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55866 ) Change subject: util: Give the udev-rules file a meaningful name ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/55866/comment/ba203396_b9f49c66 PS1, Line 9: Rename `z60_flashrom.rules` to `flashrom_udev.rules`. > Doesn’t `rules` imply udev for most folks? […] Is there a standard for that ordering? -- To view, visit https://review.coreboot.org/c/flashrom/+/55866 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1e7918d3121d89d3c388745e433a3a413eac0e21 Gerrit-Change-Number: 55866 Gerrit-PatchSet: 1 Gerrit-Owner: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Sat, 26 Jun 2021 10:16:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: util: Give the udev-rules file a meaningful name
by Paul Menzel (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Felix Singer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55866 ) Change subject: util: Give the udev-rules file a meaningful name ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/55866/comment/85e2c42a_37ba7889 PS1, Line 7: util: Give the udev-rules file a meaningful name Name udev-rules meaningfully https://review.coreboot.org/c/flashrom/+/55866/comment/13872603_79e0b4c6 PS1, Line 9: Rename `z60_flashrom.rules` to `flashrom_udev.rules`. Doesn’t `rules` imply udev for most folks? Note, udev rules are often prefixed with a number to ensure ordering (`/usr/lib/udev/rules.d/`). -- To view, visit https://review.coreboot.org/c/flashrom/+/55866 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1e7918d3121d89d3c388745e433a3a413eac0e21 Gerrit-Change-Number: 55866 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Sat, 26 Jun 2021 06:52:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Avoid using the global layout
by Edward O'Callaghan (Code Review) 26 Jun '21

26 Jun '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Daniel Campello. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54284 ) Change subject: libflashrom: Avoid using the global layout ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic48c9e73a3d00782f638f6ff41b620910b24ab6f Gerrit-Change-Number: 54284 Gerrit-PatchSet: 7 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Sat, 26 Jun 2021 03:45:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021