Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68675 )
Change subject: it87spi: Plumb par bus size into it8705f_write_enable() helper
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/68675
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iec08cb4d56a8ca62f00b0f7323b36f7d2f850e81
Gerrit-Change-Number: 68675
Gerrit-PatchSet: 2
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Wed, 02 Nov 2022 13:33:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68479 )
Change subject: cli_classic.c: Make count_max_decode_exceedings() pure
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/68479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ia01f77993deab68e251850008e885828e55b9462
Gerrit-Change-Number: 68479
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Wed, 02 Nov 2022 13:29:37 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67393 )
Change subject: tree/: Move programmer_delay() out of programmer state machine
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/67393/comment/38dabfb9_2ddee046
PS4, Line 13:
> Did the above answer the outstanding questions, is this resolved? This comment appears to have gone […]
Sorry for the late reply, couldn't sleep properly for more than a week... 😞 What you said makes sense, we'll need to read it again to make sure we understand it correctly (lack of sleep makes it hard).
Let's try to revisualize this. Re-checking the delay functions, looks like they rely on global variables that are also used by other programmer functions. If the long-term goal is to remove the global state, in which struct should these currently-global variables go?
--
To view, visit https://review.coreboot.org/c/flashrom/+/67393
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c
Gerrit-Change-Number: 67393
Gerrit-PatchSet: 6
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: 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-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Wed, 02 Nov 2022 13:28:46 +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: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons, Anastasia Klimchuk, Alexander Goncharov.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67393 )
Change subject: tree/: Move programmer_delay() out of programmer state machine
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/67393/comment/d9949f73_b088fb03
PS4, Line 13:
> These are great questions Angel. […]
Did the above answer the outstanding questions, is this resolved? This comment appears to have gone into a floating state.
--
To view, visit https://review.coreboot.org/c/flashrom/+/67393
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c
Gerrit-Change-Number: 67393
Gerrit-PatchSet: 6
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Wed, 02 Nov 2022 12:08:24 +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: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69133 )
Change subject: tree/: Convert flashchip erase_block func ptr to enumerate [ALT]
......................................................................
Patch Set 3:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/69133/comment/831cc5b4_42e9de84
PS3, Line 401: erase_func == NO_BLOCK_ERASE_FUNC
> `erase_func` is now a function pointer, it shouldn't be compared to enum value `NO_BLOCK_ERASE_FUNC` […]
Same applies to the other checks below.
--
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 02 Nov 2022 12:03:21 +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
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69133 )
Change subject: tree/: Convert flashchip erase_block func ptr to enumerate [ALT]
......................................................................
Patch Set 3:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/69133/comment/38e40518_a3cb70d5
PS3, Line 401: erase_func == NO_BLOCK_ERASE_FUNC
> Sorry I couldn't follow, could you elaborate what you are seeing here please?
`erase_func` is now a function pointer, it shouldn't be compared to enum value `NO_BLOCK_ERASE_FUNC`. This should instead be `!erase_func`.
--
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 02 Nov 2022 12:03:08 +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
Attention is currently required from: Evan Benn.
Edward O'Callaghan 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/aee02f0d_f4c9c465
PS2, Line 9: string
> Updated commit message no longer says anything about strings or Strings.
LGTM, nice side step lol.
--
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 12:02:34 +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
Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69133 )
Change subject: tree/: Convert flashchip erase_block func ptr to enumerate [ALT]
......................................................................
Patch Set 3:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/69133/comment/2bac8560_a8e9e701
PS3, Line 401: erase_func == NO_BLOCK_ERASE_FUNC
> These checks should be updated accordingly.
Sorry I couldn't follow, could you elaborate what you are seeing here please?
--
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-Comment-Date: Wed, 02 Nov 2022 11:59:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69131 )
Change subject: tree/: Convert flashchips db to use indirection for erase_block
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> O-oh? Y-you did it, Edward... Thank you so much for giving this idea a try! 😊 […]
Thank you for taking the time to review and suggest the idea. I can confirm the above results in identical binaries pre-post this change.
```
└──╼ sha256sum flashrom.bin flashrom
98488c7f35f3fcdef9029fd2acb8f93e0af3daf25291ff325cec81883db14927 flashrom.pre
98488c7f35f3fcdef9029fd2acb8f93e0af3daf25291ff325cec81883db14927 flashrom
```
--
To view, visit https://review.coreboot.org/c/flashrom/+/69131
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I122295ec9add0fe0efd27273c9725e5d64f6dbe2
Gerrit-Change-Number: 69131
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 02 Nov 2022 11:55:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment