January 2023 - flashrom-gerrit

[M] Change in flashrom[master]: sb600spi.c: Move promontory code into a mmap_read=yes mode
by Arthur Heymans (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Jonathan Zhang, Thomas Heijligen, Edward O'Callaghan, Arthur Heymans. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71565 ) Change subject: sb600spi.c: Move promontory code into a mmap_read=yes mode ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71565/comment/62fa5ed7_bc250dea PS2, Line 10: This is not : hardware specific > > I believe it is hardware specific to controllers after a certain generation. i.e., supported on SPI100 controllers (Zen etc) however not supported on pre-SPI100 controllers was my understanding. > > I think the parts controlling the speed and mode might not apply, but it is memory mapped for sure. It looks like the speed params are handled correctly. Memory mapping should work on all AMD platforms. -- To view, visit https://review.coreboot.org/c/flashrom/+/71565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Gerrit-Change-Number: 71565 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 12 Jan 2023 13:40:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: sb600spi.c: Move promontory code into a mmap_read=yes mode
by Arthur Heymans (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Jonathan Zhang, Thomas Heijligen, Edward O'Callaghan, Arthur Heymans. Hello Varshit Pandya, build bot (Jenkins), Jonathan Zhang, Thomas Heijligen, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71565 to look at the new patch set (#3). Change subject: sb600spi.c: Move promontory code into a mmap_read=yes mode ...................................................................... sb600spi.c: Move promontory code into a mmap_read=yes mode The only reason promontory code is handled differently is because memory mapped reads can be faster than manual SPI sequencing. This is not hardware specific so move it to an option 'mmap_read=yes'. Tested on an amd picasso and genoa board. Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M flashrom.8.tmpl M sb600spi.c 2 files changed, 71 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/71565/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/71565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Gerrit-Change-Number: 71565 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: sb600spi.c: Move promontory code into a mmap_read=yes mode
by Varshit Pandya (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Jonathan Zhang, Thomas Heijligen, Edward O'Callaghan, Arthur Heymans. Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71565 ) Change subject: sb600spi.c: Move promontory code into a mmap_read=yes mode ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Gerrit-Change-Number: 71565 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 12 Jan 2023 13:27:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: programmer: Add bitbanging programmer driver for Linux libgpiod
by Angel Pons (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Steve Markgraf. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71801 ) Change subject: programmer: Add bitbanging programmer driver for Linux libgpiod ...................................................................... Patch Set 1: Code-Review+1 (4 comments) File include/programmer.h: https://review.coreboot.org/c/flashrom/+/71801/comment/219830f7_ff4d44ee PS1, Line 75: extern const struct programmer_entry programmer_linux_gpiod; Looks like the list is sorted alphabetically File linux_gpiod.c: https://review.coreboot.org/c/flashrom/+/71801/comment/664ba19c_b6d93497 PS1, Line 97: atoi Other places use `strtol()` because it's safer https://review.coreboot.org/c/flashrom/+/71801/comment/6d807e3e_908cdb4f PS1, Line 142: if (register_spi_bitbang_master(&bitbang_spi_master_gpiod, data)) : return 1; /* shutdown function does the cleanup */ : : return 0; How about: ``` /* shutdown function does the cleanup */ return register_spi_bitbang_master(&bitbang_spi_master_gpiod, data); ``` https://review.coreboot.org/c/flashrom/+/71801/comment/34428947_b64a0637 PS1, Line 148: if (data) { : if (gpiod_line_bulk_num_lines(&data->bulk) > 0) : gpiod_line_release_bulk(&data->bulk); : : free(data); : } : : if (chip) : gpiod_chip_close(chip); > I have few questions about the error path exit. […] At some point we decided to register the shutdown function early so that it does the cleanup in case init fails. If we still do this, looks like we could benefit here too. -- To view, visit https://review.coreboot.org/c/flashrom/+/71801 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icad3eb7764f28feaea51bda3a7893da724c86d06 Gerrit-Change-Number: 71801 Gerrit-PatchSet: 1 Gerrit-Owner: Steve Markgraf <steve(a)steve-m.de> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Steve Markgraf <steve(a)steve-m.de> Gerrit-Comment-Date: Thu, 12 Jan 2023 12:48:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build
by Name of user not set (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: eshankelkar(a)galorithm.com. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71791 to look at the new patch set (#5). Change subject: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build ...................................................................... sfdp.c : Initializing hbuf to avoid warnings generated by scan-build scan-build uses the clang analyzer which is giving warnings about garbage/undefined values in hbuf assigned to member of hdrs[0] if hbuf is not initialized. Though the path of the control flow shown by it in its html output cannot occur in reality(since it assumes that (nph+1)*8 is <= 0 i.e nph<=-1 but later assumes 0 <= nph ) hence its a false positive. Still initializing all bytes of hbuf to 0 leads to the two warnings for sfdp.c (one for hbuf, one for tbuf) to go away. Change-Id: I6815e246b4fd225d1837cae6e7d2aa0236b48b1b Signed-off-by: Eshan Kelkar <eshankelkar(a)galorithm.com> --- M sfdp.c 1 file changed, 27 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/71791/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/71791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6815e246b4fd225d1837cae6e7d2aa0236b48b1b Gerrit-Change-Number: 71791 Gerrit-PatchSet: 5 Gerrit-Owner: eshankelkar(a)galorithm.com Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: eshankelkar(a)galorithm.com Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests/chip: Add non-aligned write within a region unit-test
by Evan Benn (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71659 ) Change subject: tests/chip: Add non-aligned write within a region unit-test ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.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: Thu, 12 Jan 2023 04:51:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build
by Name of user not set (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: eshankelkar(a)galorithm.com. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71791 to look at the new patch set (#4). Change subject: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build ...................................................................... sfdp.c : Initializing hbuf to avoid warnings generated by scan-build scan-build uses the clang analyzer which is giving warnings about garbage/undefined values in hbuf assigned to member of hdrs[0] if hbuf is not initialized. Though the path of the control flow shown by it in its html output cannot occur in reality(since it assumes that (nph+1)*8 is <= 0 i.e nph<=-1 but later assumes 0 <= nph ) hence its a false positive. Still initializing all bytes of hbuf to 0 leads to the two warnings for sfdp.c (one for hbuf, one for tbuf) to go away. Change-Id: I6815e246b4fd225d1837cae6e7d2aa0236b48b1b Signed-off-by : Eshan Kelkar <eshankelkar(a)galorithm.com> --- M sfdp.c 1 file changed, 27 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/71791/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/71791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6815e246b4fd225d1837cae6e7d2aa0236b48b1b Gerrit-Change-Number: 71791 Gerrit-PatchSet: 4 Gerrit-Owner: eshankelkar(a)galorithm.com Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: eshankelkar(a)galorithm.com Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests/chip: Add non-aligned write within a region unit-test
by Sam McNally (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Evan Benn. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71659 ) Change subject: tests/chip: Add non-aligned write within a region unit-test ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.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-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 03:26:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dmi.c: Pass is_laptop by ref into dmi
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71622 ) Change subject: dmi.c: Pass is_laptop by ref into dmi ...................................................................... Patch Set 2: (2 comments) File dmi.c: https://review.coreboot.org/c/flashrom/+/71622/comment/a9ea2717_0d92df76 PS1, Line 158: *is_laptop = 2; > For another patch: This should be an enum. I improved upon the patch to at least make it a return value. Thanks for the review. https://review.coreboot.org/c/flashrom/+/71622/comment/c6b4986d_89ed23d7 PS1, Line 402: is_laptop > nit: update comment too? Done -- To view, visit https://review.coreboot.org/c/flashrom/+/71622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Gerrit-Change-Number: 71622 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 03:19:24 +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]: dmi.c: Pass is_laptop by ref into dmi
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71622 to look at the new patch set (#2). Change subject: dmi.c: Pass is_laptop by ref into dmi ...................................................................... dmi.c: Pass is_laptop by ref into dmi Prefix the remaining global cases with `g_` to avoid shadowing issues and for easy greping. Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M dmi.c M include/programmer.h M internal.c 4 files changed, 45 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/71622/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Gerrit-Change-Number: 71622 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023