November 2022 - flashrom-gerrit

[XL] Change in flashrom[master]: tree/: Convert flashchip erase_block func ptr to enumerate
by Edward O'Callaghan (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Felix Singer, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Anastasia Klimchuk, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67240 to look at the new patch set (#15). Change subject: tree/: Convert flashchip erase_block func ptr to enumerate ...................................................................... tree/: Convert flashchip erase_block func ptr to enumerate This forges the way for flashchips.c to be pure declarative data and lookup functions for dispatch to be pure. This means that the flashchips data could be extracted out to be agnostic data of the flashrom code and algorithms. TEST='R|W|E && --flash-name' on ARM, AMD & Intel DUT's. Change-Id: I58415a4f2589812b26d284c96876e88f8d9acf52 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M at45db.c M atapromise.c M flashchips.c M flashrom.c M include/chipdrivers.h M include/flash.h M sfdp.c M spi25.c M tests/chip.c M tests/chip_wp.c 10 files changed, 2,105 insertions(+), 1,982 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/67240/15 -- To view, visit https://review.coreboot.org/c/flashrom/+/67240 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I58415a4f2589812b26d284c96876e88f8d9acf52 Gerrit-Change-Number: 67240 Gerrit-PatchSet: 15 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Rename 'internal_delay()' to 'default_delay()'
by Anastasia Klimchuk (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68855 ) Change subject: tree/: Rename 'internal_delay()' to 'default_delay()' ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: I like it because I had the same thought in one of the previous patches in the chain :) -- To view, visit https://review.coreboot.org/c/flashrom/+/68855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5e04adf16812ceb1480992c92bca25ed80f8897a Gerrit-Change-Number: 68855 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 01 Nov 2022 04:25:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Replace NULL-case of programmer_delay() with internal_delay
by Anastasia Klimchuk (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68434 ) Change subject: tree/: Replace NULL-case of programmer_delay() with internal_delay ...................................................................... Patch Set 2: Code-Review+2 (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/68434/comment/1e04dc4f_bcbb9e79 PS1, Line 282: else > ok but that is outside this diff. Yes outside this diff... I think I got confused because the body of `programmer_delay` has been changed recently, and this comment should have been in some of the earlier patches. Probably it's more accurate to say: it's outside of this one patch, but belongs to the ongoing effort to un-globalize programmer delay. I am fine to have it in a separate patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/68434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1da230804d5e8f47a6e281feb66f381514dc6861 Gerrit-Change-Number: 68434 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 01 Nov 2022 04:21:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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

[M] Change in flashrom[master]: tests: Add prefix to io_mock functions not to clash with macros
by Anastasia Klimchuk (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Felix Singer, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68433 ) Change subject: tests: Add prefix to io_mock functions not to clash with macros ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Previous patch in the chain has changed, as a result this one has been rebased (no changes in this patch). -- To view, visit https://review.coreboot.org/c/flashrom/+/68433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Gerrit-Change-Number: 68433 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 01 Nov 2022 04:01:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Anastasia Klimchuk (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Felix Singer, Thomas Heijligen, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68432 ) Change subject: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > Thank you, that's an interesting idea, I decided to try. […] Thank you so much for your help Peter, `-U_FORTIFY_SOURCE` did the job. Please have a look. -- To view, visit https://review.coreboot.org/c/flashrom/+/68432 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Gerrit-Change-Number: 68432 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Nov 2022 03:57:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Anastasia Klimchuk (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Felix Singer, Thomas Heijligen, Peter Marheine. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68432 to look at the new patch set (#3). Change subject: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants ...................................................................... tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants The option _FORTIFY_SOURCE, when enabled, can result in some functions being expanded into _chk variants. For example, `fprintf` can get expanded into `__fprintf_chk`. This makes sence for building a real binary, but is not needed for unit tests. In unit test environment all those functions are wrapped. In the example above, both `fprintf` and `__fprintf_chk` needed to be mocked. Disabling _FORTIFY_SOURCE avoids expanding functions into _chk variants, without any loss of testing coverage because that would be wrapped/mocked anyway. BUG=b:237606255 TEST=ninja test on 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build 1 file changed, 28 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/68432/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/68432 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Gerrit-Change-Number: 68432 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Add prefix to io_mock functions not to clash with macros
by Anastasia Klimchuk (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68433 to look at the new patch set (#3). Change subject: tests: Add prefix to io_mock functions not to clash with macros ...................................................................... tests: Add prefix to io_mock functions not to clash with macros Flashrom I/O mock functions need to be renamed so that they do not have name clash with standard I/O, because the latter are allowed to be macros. Adding a prefix to flashrom mock functions avoids them being accidentally expanded. Standard I/O functions are expanded and flashrom mocks stays as they are. BUG=b:237606255 TEST=ninja test 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/io_mock.c M tests/io_mock.h M tests/linux_mtd.c M tests/linux_spi.c M tests/parade_lspcon.c M tests/realtek_mst_i2c_spi.c M tests/tests.c 8 files changed, 69 insertions(+), 47 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/68433/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/68433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Gerrit-Change-Number: 68433 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom_tester: Use Path type for test data
by Edward O'Callaghan (Code Review) Nov. 1, 2022

Nov. 1, 2022

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68643 ) Change subject: flashrom_tester: Use Path type for test data ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: It's a good idea to keep the same change id otherwise review comments become discontinuous. https://review.coreboot.org/c/flashrom/+/69064 -- To view, visit https://review.coreboot.org/c/flashrom/+/68643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4a0b25c8c70fa6dfd682fba3db7d3bf06b2eb7ac Gerrit-Change-Number: 68643 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Nov 2022 02:06:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Change string types to path types where appropriate
by Evan Benn (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Angel Pons, Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69064 ) Change subject: flashrom_tester: Change string types to path types where appropriate ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/69064/comment/865ac338_a68bb7f3 PS1, Line 7: String > `str` not `String`, a `String` is a heap allocated "string" while a `str` is a stack allocated "stri […] good point, i changed to 'string' as the rust book does https://review.coreboot.org/c/flashrom/+/69064/comment/3d96ac1a_92c2219a PS1, Line 9: String > s/String/Stack allocated str's with life-time 'a'/ Done -- To view, visit https://review.coreboot.org/c/flashrom/+/69064 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I69531bec5436a60430eae975eeab02c8835962bf Gerrit-Change-Number: 69064 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Nov 2022 02:06:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Change string types to path types where appropriate
by Edward O'Callaghan (Code Review) Nov. 1, 2022

Nov. 1, 2022

Attention is currently required from: Angel Pons, Peter Marheine, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69064 ) Change subject: flashrom_tester: Change string types to path types where appropriate ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69064 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I69531bec5436a60430eae975eeab02c8835962bf Gerrit-Change-Number: 69064 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Tue, 01 Nov 2022 02:05:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0