November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: tree/: Convert flashchip erase_block func ptr to enumerate [ALT]
by Edward O'Callaghan (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69133 to look at the new patch set (#3). Change subject: tree/: Convert flashchip erase_block func ptr to enumerate [ALT] ...................................................................... tree/: Convert flashchip erase_block func ptr to enumerate [ALT] 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. Change-Id: I02ae7e4c67c5bf34ec2fd7ffe4af8a2aba6fd5e5 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M at45db.c M flashrom.c M include/chipdrivers.h M include/flash.h M sfdp.c M spi25.c M tests/chip.c 7 files changed, 134 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/69133/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/69133 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I02ae7e4c67c5bf34ec2fd7ffe4af8a2aba6fd5e5 Gerrit-Change-Number: 69133 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: Makefile: Don't install git hooks automatically
by Felix Singer (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68647 ) Change subject: Makefile: Don't install git hooks automatically ...................................................................... Patch Set 4: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68647/comment/2ed5b613_f8685807 PS4, Line 16: > Could you please add one more line in commit message explaining how to install git hooks? I understa […] Done :) Patchset: PS4: > I agree on the condition that we update Development guidelines straight after this patch is merged. […] Yes sure. Development guidelines need to be adjusted anyway, because the command from there doesn't work. *sigh* -- To view, visit https://review.coreboot.org/c/flashrom/+/68647 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib83568c7ff149a8ec34ad7e92720c36a89def7bd Gerrit-Change-Number: 68647 Gerrit-PatchSet: 4 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: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Nov 2022 04:42:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: Makefile: Don't install git hooks automatically
by Felix Singer (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen. Hello build bot (Jenkins), Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68647 to look at the new patch set (#5). Change subject: Makefile: Don't install git hooks automatically ...................................................................... Makefile: Don't install git hooks automatically These specific git hooks are only needed when someone wants to push a patch to upstream and so it's not needed to run it in every make call. Beside that, we also don't know the environment in which this is executed and it might not be wanted. Thus, add a new make target `gitconfig` and move the install command to it. It can be used by running `make gitconfig`. Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: Ib83568c7ff149a8ec34ad7e92720c36a89def7bd --- M Makefile 1 file changed, 21 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/68647/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/68647 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib83568c7ff149a8ec34ad7e92720c36a89def7bd Gerrit-Change-Number: 68647 Gerrit-PatchSet: 5 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: 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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: meson: Move programmer test sources into programmer definition
by Felix Singer (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68162 ) Change subject: meson: Move programmer test sources into programmer definition ...................................................................... Patch Set 16: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/68162/comment/54cd1f7c_d47be71f PS16, Line 191: 'test_srcs' : > Is it possible to align all `:` ? Maybe it's just me... […] I also don't like it : / I'm not sure if I should fix the indentation in this patch or in a separate one. I didn't want to touch so many lines in this patch. Maybe a separate one because then we can fix the indentation of the lines 454 - 461 too? -- To view, visit https://review.coreboot.org/c/flashrom/+/68162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I307faaf8a9f7ae3c54bd96e7d871a3abb8aadea3 Gerrit-Change-Number: 68162 Gerrit-PatchSet: 16 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Nov 2022 04:29:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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 Felix Singer (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Felix Singer 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 5: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68433/comment/2d813b15_5fee52ea PS5, Line 10: 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 I don't understand this yet. I have some questions: * Why is it a problem that standard IO functions are macros? * How does the prefix prevent that the flashrom mocks are expanded? I don't understand the relation between that function pointer and the standard I/O functions. * Shouldn't undefining _FORTIFY_SOURCE solve that problem? In my understanding standard I/O functions are not expanded when _FORTIFY_SOURCE is undefined. https://review.coreboot.org/c/flashrom/+/68433/comment/2731a9ce_9ca9658f PS5, Line 13: stays typo: stay (without s) File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/68433/comment/4584ff5e_6e5b08c6 PS5, Line 73: /* Port I/O */ : void (*outb)(void *state, unsigned char value, unsigned short port); : unsigned char (*inb)(void *state, unsigned short port); : : void (*outw)(void *state, unsigned short value, unsigned short port); : unsigned short (*inw)(void *state, unsigned short port); : : void (*outl)(void *state, unsigned int value, unsigned short port); : unsigned int (*inl)(void *state, unsigned short port); : : /* USB I/O */ : int (*libusb_init)(void *state, libusb_context **ctx); : int (*libusb_control_transfer)(void *state, : libusb_device_handle *devh, : uint8_t bmRequestType, : uint8_t bRequest, : uint16_t wValue, : uint16_t wIndex, : unsigned char *data, : uint16_t wLength, : unsigned int timeout); : ssize_t (*libusb_get_device_list)(void *state, libusb_context *, libusb_device ***list); : void (*libusb_free_device_list)(void *state, libusb_device **list, int unref_devices); : int (*libusb_get_device_descriptor)(void *state, libusb_device *, struct libusb_device_descriptor *); : int (*libusb_get_config_descriptor)(void *state, : libusb_device *, : uint8_t config_index, : struct libusb_config_descriptor **); : void (*libusb_free_config_descriptor)(void *state, struct libusb_config_descriptor *); I am wondering, should these functions be renamed as well? -- 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: 5 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Nov 2022 04:22:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom_tester: Use tempdir crate for test data
by Evan Benn (Code Review) 02 Nov '22

02 Nov '22

Evan Benn has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/68739 ) Change subject: flashrom_tester: Use tempdir crate for test data ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/flashrom/+/68739 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I14089ef69c3f509cb6f01cc44c7924c244ab7d73 Gerrit-Change-Number: 68739 Gerrit-PatchSet: 5 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[S] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Felix Singer (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Felix Singer 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 5: Code-Review+2 -- 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: 5 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Nov 2022 03:58:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom_tester: Use tempdir crate for test data
by Edward O'Callaghan (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68739 ) Change subject: flashrom_tester: Use tempdir crate for test data ...................................................................... Patch Set 5: Code-Review-1 (1 comment) File util/flashrom_tester/src/tester.rs: https://review.coreboot.org/c/flashrom/+/68739/comment/c3fc7ef8_71e17e76 PS5, Line 87: : original_flash_contents: tmp_dir.path().join("flashrom_tester_golden.bin"), : The critical issue with this patch is that it can leave a DUT irrecoverable should the test suite fail in some way or get stopped with no ability to inspect the pre-run state and use that to recover with. The principle idea behind the patch is good but it needs more care to not regress the requirement of postmortem test analysis of artifacts. -- To view, visit https://review.coreboot.org/c/flashrom/+/68739 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I14089ef69c3f509cb6f01cc44c7924c244ab7d73 Gerrit-Change-Number: 68739 Gerrit-PatchSet: 5 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 02 Nov 2022 03:30:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Use path types for things that are paths
by Angel Pons (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Evan Benn. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69064 ) Change subject: flashrom_tester: Use path types for things that are paths ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69064/comment/33712aac_5bfa8074 PS2, Line 9: string > See above, 'String' vs 'string' have very specific meanings. […] Updated commit message no longer says anything about strings or Strings. -- 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: 3 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: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 02 Nov 2022 03:19:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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

[M] Change in flashrom[master]: tree/: Convert flashchip erase_block func ptr to enumerate [ALT]
by Edward O'Callaghan (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69133 to look at the new patch set (#2). Change subject: tree/: Convert flashchip erase_block func ptr to enumerate [ALT] ...................................................................... tree/: Convert flashchip erase_block func ptr to enumerate [ALT] 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. Change-Id: I02ae7e4c67c5bf34ec2fd7ffe4af8a2aba6fd5e5 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M at45db.c M flashrom.c M include/chipdrivers.h M include/flash.h M sfdp.c M spi25.c M tests/chip.c 7 files changed, 135 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/69133/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69133 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I02ae7e4c67c5bf34ec2fd7ffe4af8a2aba6fd5e5 Gerrit-Change-Number: 69133 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022