- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: tests: Add comprehensive comment for chip.c
by Anastasia Klimchuk (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59238 ) Change subject: tests: Add comprehensive comment for chip.c ...................................................................... Patch Set 7: (1 comment) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/59238/comment/3d62246a_4b67680e PS6, Line 24: these > Indeed, `this type` or `these types` (of tests) "this type", right -- To view, visit https://review.coreboot.org/c/flashrom/+/59238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac Gerrit-Change-Number: 59238 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> 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: Tue, 25 Jan 2022 00:15:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Set up mock chip memory in consistent and predictable way
by Anastasia Klimchuk (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59237 ) Change subject: tests: Set up mock chip memory in consistent and predictable way ...................................................................... Patch Set 7: (2 comments) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/59237/comment/e3b8045c_dad901b5 PS6, Line 33: .buf = { MOCK_CHIP_CONTENT }, > This only initialises the first element of `buf`, the rest of the array is initialised to 0. […] Yes it is misleading: I misled myself :( I will keep {0} here as it was before, that's a good idea. And yes, setup_chip is expected to be called at the beginning of every test anyway. https://review.coreboot.org/c/flashrom/+/59237/comment/f0b9efa9_a3860d99 PS6, Line 97: memset(&g_chip_state.buf[0], MOCK_CHIP_CONTENT, MOCK_CHIP_SIZE); > How about: […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/59237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871 Gerrit-Change-Number: 59237 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 25 Jan 2022 00:14:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add comprehensive comment for chip.c
by Anastasia Klimchuk (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Felix Singer, Nico Huber, 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/+/59238 to look at the new patch set (#7). Change subject: tests: Add comprehensive comment for chip.c ...................................................................... tests: Add comprehensive comment for chip.c The following describes the two mechanisms of testing done for flash chip operations. BUG=b:181803212 TEST=ninja test Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c 1 file changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/59238/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/59238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac Gerrit-Change-Number: 59238 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Set up mock chip memory in consistent and predictable way
by Anastasia Klimchuk (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Nico Huber, Paul Menzel, 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/+/59237 to look at the new patch set (#7). Change subject: tests: Set up mock chip memory in consistent and predictable way ...................................................................... tests: Set up mock chip memory in consistent and predictable way This patch adds a macro MOCK_CHIP_CONTENT which represents a memory state of a mock chip. The macro is used to initialise mock chip memory at the beginning of a test (in setup_chip function). Previously mock chip memory was not reset between tests. For existing tests that did not matter, however new test for verify operation (added later in this chain) needs mock chip memory to be setup in a predictable way. BUG=b:181803212 TEST=ninja test Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/59237/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/59237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871 Gerrit-Change-Number: 59237 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Upgrade linux_spi test to run probe lifecycle
by Nico Huber (Code Review) 24 Jan '22

24 Jan '22

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/+/59742 ) Change subject: tests: Upgrade linux_spi test to run probe lifecycle ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59742 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I32d8e972d99b52c2b18f688aa6aeae75dd170f72 Gerrit-Change-Number: 59742 Gerrit-PatchSet: 5 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-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, 24 Jan 2022 14:08:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Set up mock chip memory in consistent and predictable way
by Angel Pons (Code Review) 24 Jan '22

24 Jan '22

Attention is currently required from: Nico Huber, Paul Menzel, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59237 ) Change subject: tests: Set up mock chip memory in consistent and predictable way ...................................................................... Patch Set 6: Code-Review+1 (2 comments) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/59237/comment/29291648_c2e54296 PS6, Line 33: .buf = { MOCK_CHIP_CONTENT }, This only initialises the first element of `buf`, the rest of the array is initialised to 0. I see it gets filled later on, but I find this potentially misleading. I'd remove the static initialiser (or leave it to explicitly initialise to 0) and put the actual initialisation code in a helper function just below. https://review.coreboot.org/c/flashrom/+/59237/comment/0541c335_3cc7c8fe PS6, Line 97: memset(&g_chip_state.buf[0], MOCK_CHIP_CONTENT, MOCK_CHIP_SIZE); How about: memset(g_chip_state.buf, MOCK_CHIP_CONTENT, sizeof(g_chip_state.buf)); -- To view, visit https://review.coreboot.org/c/flashrom/+/59237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871 Gerrit-Change-Number: 59237 Gerrit-PatchSet: 6 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 24 Jan 2022 13:40:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add comprehensive comment for chip.c
by Angel Pons (Code Review) 24 Jan '22

24 Jan '22

Attention is currently required from: Felix Singer, Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59238 ) Change subject: tests: Add comprehensive comment for chip.c ...................................................................... Patch Set 6: (1 comment) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/59238/comment/aee848e3_9a32fd57 PS6, Line 24: these > this? Indeed, `this type` or `these types` (of tests) -- To view, visit https://review.coreboot.org/c/flashrom/+/59238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac Gerrit-Change-Number: 59238 Gerrit-PatchSet: 6 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 24 Jan 2022 13:30:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add comprehensive comment for chip.c
by Felix Singer (Code Review) 24 Jan '22

24 Jan '22

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59238 ) Change subject: tests: Add comprehensive comment for chip.c ...................................................................... Patch Set 6: (1 comment) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/59238/comment/938f2f83_6c7d45b2 PS6, Line 24: these this? -- To view, visit https://review.coreboot.org/c/flashrom/+/59238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac Gerrit-Change-Number: 59238 Gerrit-PatchSet: 6 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 24 Jan 2022 13:18:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Move {read,write}_buf_from_include_args() to layout
by Nico Huber (Code Review) 24 Jan '22

24 Jan '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61286 ) Change subject: flashrom.c: Move {read,write}_buf_from_include_args() to layout ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > These two functions are best situated within cli_common once read_flash_to_file() has been eliminate […] Actually, they are dead code ATM, so I find it hard to reason about intermediate steps. If I'd write this (per-region file arguments) from scratch, I'd probably go either one of these two ways: 1. Keep everything file related in CLI code. CLI would have to maintain its own mapping from region names to file names (to implement the two functions with the libflashrom API, we'd need a public API to walk the layout entries). 2. Add at set of optional, file-aware libflashrom APIs, e.g. libflashrom_file.[hc]. Something that let's the API user add file names to layout entries and then the two functions that read/write everything from/to files. -- To view, visit https://review.coreboot.org/c/flashrom/+/61286 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I61d4460ea26e07b67c99baeae5cb3840c25cb913 Gerrit-Change-Number: 61286 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 24 Jan 2022 13:04:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: fix build on non-x86 targets
by Angel Pons (Code Review) 24 Jan '22

24 Jan '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Paul Menzel, Edward O'Callaghan, Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61194 ) Change subject: hwaccess: fix build on non-x86 targets ...................................................................... Patch Set 7: Code-Review+1 (1 comment) File hwaccess_x86_io.c: https://review.coreboot.org/c/flashrom/+/61194/comment/a62e0fb1_4db42191 PS7, Line 82: /* PCI port I/O support is unimplemented on PPC/MIPS and unavailable on ARM. */ > Where did this go? Ah: The comment on the stub implementation of rget_io_perms() is also modified to remove references to non-x86 platforms, since that file is only built on x86 now. -- To view, visit https://review.coreboot.org/c/flashrom/+/61194 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I20f122679c30340b2c73afd7419e79644ddc3c4e Gerrit-Change-Number: 61194 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(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: Anastasia Klimchuk <aklm(a)chromium.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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 24 Jan 2022 12:43:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit