October 2022 - flashrom-gerrit

[S] Change in flashrom[master]: flashrom: Make platform-specific errors only print on that platform
by Thomas Heijligen (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Nico Huber, Keno Fischer. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68468 ) Change subject: flashrom: Make platform-specific errors only print on that platform ...................................................................... Patch Set 2: Code-Review-1 (1 comment) Patchset: PS2: I think it would be better to integrate the message into `platform_get_io_perms` instead of using additional `#ifdef` -- To view, visit https://review.coreboot.org/c/flashrom/+/68468 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4d1e369cb60e160f555f1f9f0c832fe27b75e9de Gerrit-Change-Number: 68468 Gerrit-PatchSet: 2 Gerrit-Owner: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Comment-Date: Mon, 17 Oct 2022 17:50:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree: Change signature of extract_programmer_param_str()
by Angel Pons (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66654 ) Change subject: tree: Change signature of extract_programmer_param_str() ...................................................................... Patch Set 6: (1 comment) File atavia.c: https://review.coreboot.org/c/flashrom/+/66654/comment/58dd3256_d832ca04 PS6, Line 148: NULL > Not sure if that would have caught the multiple foot shotguns while unraveling that technical debt. […] The idea of having a special macro for `NULL` is so that it can be grepped for. Once the refactoring is done, the macro should not be used anywhere. Nico also suggested another approach a long time ago, but it didn't seem to gain much traction. Probably because of animosity, which is unfortunate... The idea is simple: change the code in the opposite order. That is, start with passing a (non-NULL) pointer to a `programmer_cfg` to `programmer_init()` functions, then plumb the new parameter into each programmer until it reaches `extract_programmer_param_str()`. In any case, the end result would be the same, but the journey would be different and (hopefully) smoother. We decided to write down this comment so that everyone can learn from this. 😄 -- To view, visit https://review.coreboot.org/c/flashrom/+/66654 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I781a328fa280e0a9601050dd99a75af72c39c899 Gerrit-Change-Number: 66654 Gerrit-PatchSet: 6 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 17 Oct 2022 17:46:10 +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

[S] Change in flashrom[master]: flashrom: Add some helpful hints about missing Linux kernel configs
by Thomas Heijligen (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Felix Singer, Keno Fischer. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68442 ) Change subject: flashrom: Add some helpful hints about missing Linux kernel configs ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Patchset: PS4: Can you give us the name of the CPU you are using? -- To view, visit https://review.coreboot.org/c/flashrom/+/68442 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I05cc3d170e59d5bcaf731df5152db5a47746cc0c Gerrit-Change-Number: 68442 Gerrit-PatchSet: 4 Gerrit-Owner: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Comment-Date: Mon, 17 Oct 2022 17:45:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom: Add some helpful hints about missing Linux kernel configs
by Angel Pons (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Felix Singer, Keno Fischer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68442 ) Change subject: flashrom: Add some helpful hints about missing Linux kernel configs ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/68442 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I05cc3d170e59d5bcaf731df5152db5a47746cc0c Gerrit-Change-Number: 68442 Gerrit-PatchSet: 4 Gerrit-Owner: Keno Fischer <keno(a)alumni.harvard.edu> 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: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Comment-Date: Mon, 17 Oct 2022 17:06:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom: Make platform-specific errors only print on that platform
by Angel Pons (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Keno Fischer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68468 ) Change subject: flashrom: Make platform-specific errors only print on that platform ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/68468 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4d1e369cb60e160f555f1f9f0c832fe27b75e9de Gerrit-Change-Number: 68468 Gerrit-PatchSet: 2 Gerrit-Owner: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Comment-Date: Mon, 17 Oct 2022 16:45:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: raiden_debug_spi: Remove fixme with explanation
by Thomas Heijligen (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Edward O'Callaghan, Liam Flaherty. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68364 ) Change subject: raiden_debug_spi: Remove fixme with explanation ...................................................................... Patch Set 2: Code-Review+1 (2 comments) Patchset: PS2: Ok for now. For the future we should consider to extend the `struct dev_entry` to also deal with usb classes / subclasses. File raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/68364/comment/08b4622c_d3b5fa97 PS2, Line 354: * Table is empty as raiden_debug_spi matches against connected USB ... matches against the class and subclass of the connected ... -- To view, visit https://review.coreboot.org/c/flashrom/+/68364 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43e364c02f42dd499d3c9ca3e0a03ead673da3e6 Gerrit-Change-Number: 68364 Gerrit-PatchSet: 2 Gerrit-Owner: Liam Flaherty <liamflaherty(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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Comment-Date: Mon, 17 Oct 2022 12:56:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree: Change signature of extract_programmer_param_str()
by Edward O'Callaghan (Code Review) 17 Oct '22

17 Oct '22

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66654 ) Change subject: tree: Change signature of extract_programmer_param_str() ...................................................................... Patch Set 6: (1 comment) File atavia.c: https://review.coreboot.org/c/flashrom/+/66654/comment/b98f134b_2ff3d675 PS6, Line 148: NULL > For any future patches... […] Not sure if that would have caught the multiple foot shotguns while unraveling that technical debt. It took multiple developers to spot all the ways the state machine can explode when held the wrong way. However good suggestion and food for thought we definitely need more of these sorts of tactics in our pockets to mitigate fireworks. I did however try to include some past lessons from this in a similar kind of patch CB:68434 to tries catch different interpretations of NULL means different things depending on which control path you wind up on [see comment in flashrom.c]. I am optimistic that a tree where functions that consume and work on arguments over everything being a singleton closure will be a lot less like playing a game of mine sweeper. Thank you for thinking about this kind of problem! -- To view, visit https://review.coreboot.org/c/flashrom/+/66654 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I781a328fa280e0a9601050dd99a75af72c39c899 Gerrit-Change-Number: 66654 Gerrit-PatchSet: 6 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 17 Oct 2022 11:05:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree: Change signature of extract_programmer_param_str()
by Angel Pons (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66654 ) Change subject: tree: Change signature of extract_programmer_param_str() ...................................................................... Patch Set 6: (1 comment) File atavia.c: https://review.coreboot.org/c/flashrom/+/66654/comment/6530899e_7c92b514 PS6, Line 148: NULL For any future patches... If this should eventually not be a NULL pointer, at the very least please define a temporary macro to keep track of all instances of this. After all the necessary changes are done, the temporary macro should no longer be used anywhere and can be dropped. ``` /* Temporary macro to keep track of things, will eventually be dropped. */ #define NULL_PROGRAMMER_CFG_FIXME NULL ``` Then, this line would look like this until it's fixed: ``` char *arg = extract_programmer_param_str(NULL_PROGRAMMER_CFG_FIXME, "offset"); ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/66654 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I781a328fa280e0a9601050dd99a75af72c39c899 Gerrit-Change-Number: 66654 Gerrit-PatchSet: 6 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 17 Oct 2022 10:31:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom: Make platform-specific errors only print on that platform
by Felix Singer (Code Review) 17 Oct '22

17 Oct '22

Attention is currently required from: Keno Fischer. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68468 ) Change subject: flashrom: Make platform-specific errors only print on that platform ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68468 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4d1e369cb60e160f555f1f9f0c832fe27b75e9de Gerrit-Change-Number: 68468 Gerrit-PatchSet: 2 Gerrit-Owner: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Keno Fischer <keno(a)alumni.harvard.edu> Gerrit-Comment-Date: Mon, 17 Oct 2022 10:27:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: include/programmer.h: Relocal decode_sizes def and complete prog cfg ...
by Edward O'Callaghan (Code Review) 17 Oct '22

17 Oct '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68480 ) Change subject: include/programmer.h: Relocal decode_sizes def and complete prog cfg type ...................................................................... include/programmer.h: Relocal decode_sizes def and complete prog cfg type Change-Id: Ief5ec76a4050f8d5756fe6417732315a1c68f511 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c M include/programmer.h 2 files changed, 25 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/68480/1 diff --git a/flashrom.c b/flashrom.c index 0f982c9..8afbd21 100644 --- a/flashrom.c +++ b/flashrom.c @@ -66,10 +66,6 @@ /* Did we change something or was every erase/write skipped (if any)? */ static bool all_skipped = true; -struct programmer_cfg { - char *params; -}; - /* Register a function to be executed on programmer shutdown. * The advantage over atexit() is that you can supply a void pointer which will * be used as parameter to the registered function upon programmer shutdown. diff --git a/include/programmer.h b/include/programmer.h index 386e6ec..b68c73f 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -25,12 +25,25 @@ #include "flash.h" /* for chipaddr and flashctx */ +struct decode_sizes { + uint32_t parallel; + uint32_t lpc; + uint32_t fwh; + uint32_t spi; +}; enum programmer_type { PCI = 1, /* to detect uninitialized values */ USB, OTHER, }; -struct programmer_cfg; +struct programmer_cfg { + char *params; + /* + * Programmers supporting multiple buses can have differing size limits on + * each bus. Store the limits for each bus in a common struct. + */ + struct decode_sizes max_rom_decode; +}; struct dev_entry { uint16_t vendor_id; @@ -47,6 +60,7 @@ const struct dev_entry *const dev; const char *const note; } devs; + struct programmer_cfg cfg; int (*init) (const struct programmer_cfg *cfg); @@ -276,12 +290,6 @@ /* flashrom.c */ -struct decode_sizes { - uint32_t parallel; - uint32_t lpc; - uint32_t fwh; - uint32_t spi; -}; // FIXME: These need to be local, not global extern struct decode_sizes max_rom_decode; extern bool programmer_may_write; -- To view, visit https://review.coreboot.org/c/flashrom/+/68480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ief5ec76a4050f8d5756fe6417732315a1c68f511 Gerrit-Change-Number: 68480 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022