November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: spi25.c: Rename spi_get_erasefn_from_opcode to spi25_get_erasefn_from...
by Thomas Heijligen (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Simon Buhrow, Aarya. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67716 ) Change subject: spi25.c: Rename spi_get_erasefn_from_opcode to spi25_get_erasefn_from_opcode ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie054160b0fdd34bcb128285c6a047e3a3fa8be0c Gerrit-Change-Number: 67716 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 17:50:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: spi25.c: Move spi_get_opcode_from_erasefn() to spi.c
by Thomas Heijligen (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Simon Buhrow, Aarya. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67715 ) Change subject: spi25.c: Move spi_get_opcode_from_erasefn() to spi.c ...................................................................... Patch Set 4: Code-Review+2 (1 comment) File spi.c: https://review.coreboot.org/c/flashrom/+/67715/comment/ea24f1ec_148cdc56 PS4, Line 202: } Need newline at the end -- To view, visit https://review.coreboot.org/c/flashrom/+/67715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id654e998d0af2d3f5845336bb98b38d724519038 Gerrit-Change-Number: 67715 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 17:49:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: util: add bash completion script
by Thomas Heijligen (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Felix Singer, Alexander Goncharov. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 ) Change subject: util: add bash completion script ...................................................................... Patch Set 9: Code-Review+1 (3 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/68247/comment/e0dc52d7_29d87f82 PS9, Line 1041: .bash IIRC there should be no `.bash` suffix File meson.build: https://review.coreboot.org/c/flashrom/+/68247/comment/03a7a3c0_be91b777 PS9, Line 624: if get_option('classic_cli').disabled() When `get_option('bash_completion').auto()` and `get_option('classic_cli').disabled()` are true, there should be no action. `get_option('bash_completion').auto()` equals `get_option('classic_cli').auto()` in the behavior. So if I disable the cli, the bash_completion is also disabled. https://review.coreboot.org/c/flashrom/+/68247/comment/5bd340f3_7da26838 PS9, Line 646: .bash same here -- To view, visit https://review.coreboot.org/c/flashrom/+/68247 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie68bc91c3cea4de2ffdbeffd07e48edd8d5590e1 Gerrit-Change-Number: 68247 Gerrit-PatchSet: 9 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 29 Nov 2022 17:44:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: libflashrom: Define macro to detect v1.3 header
by Alexander Goncharov (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Evan Benn. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70027 ) Change subject: libflashrom: Define macro to detect v1.3 header ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Heres what SDL does, […] The current macro looks like a stub. As far as I know, nothing prevents us from adding a normal version system, so +1 for Evan's ideas. -- To view, visit https://review.coreboot.org/c/flashrom/+/70027 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac35ff167272e1bc3d5434f7b47099f53bfceab7 Gerrit-Change-Number: 70027 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 14:50:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Subrata Banik (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 9: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/c586b5b9_0e0ca9a4 PS2, Line 7: ichspi.c: Read chip ID and use it to identify chip > > Maybe we should just print a warning if >1 chip is detected? > > > > Subrata, do you have any thoughts on this? > > Ideally as per descriptor, there can be 2 SPI flash and we need to drive CS PIN low to initiate the READ JEDEC ID (0x9F) HW seq command for each SPI flash chip to capture the response. looking at the all latest platform, I don't see the usage of dual chips and mostly I could see the descriptor is set to 0 aka 1 chip. -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 07:44:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Subrata Banik (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 9: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/11eb0f42_f6d40c02 PS2, Line 7: ichspi.c: Read chip ID and use it to identify chip > Maybe we should just print a warning if >1 chip is detected? > > Subrata, do you have any thoughts on this? Ideally as per descriptor, there can be 2 SPI flash and we need to drive CS PIN low to initiate the READ JEDEC ID (0x9F) HW seq command for each SPI flash chip to capture the response. -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 07:42:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer: Add get_region to spi/opaque masters
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70126 to look at the new patch set (#2). Change subject: programmer: Add get_region to spi/opaque masters ...................................................................... programmer: Add get_region to spi/opaque masters Add a get_region function to spi and opaque masters so that they can expose access permissions for multiple regions within the flash. A get_region() implementation is added for the ichspi driver in a following patch. Finally, another patch uses get_region() to make read_flash() and write_flash() skip inaccessable regions, making read, write, and erase operations work on Intel platforms with active an CSME coprocessor. This logic will be integrated with layout in the future, but for now this moves ichspi support forward without making refactoring too hard later on. BUG=b:260440773 BRANCH=none TEST=ninja test Change-Id: I8c43f6b705f36ef18842a04ba6241d3a0b36b232 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M include/programmer.h 1 file changed, 39 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/70126/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70126 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c43f6b705f36ef18842a04ba6241d3a0b36b232 Gerrit-Change-Number: 70126 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: programmer,flashrom: Add get_region to spi/opaque masters
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70126 ) Change subject: programmer,flashrom: Add get_region to spi/opaque masters ...................................................................... Patch Set 1: This change is ready for review. -- To view, visit https://review.coreboot.org/c/flashrom/+/70126 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c43f6b705f36ef18842a04ba6241d3a0b36b232 Gerrit-Change-Number: 70126 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Nov 2022 07:02:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69195 to look at the new patch set (#9). Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... ichspi.c: Read chip ID and use it to populate `flash->chip` Read the flash chip vendor/device ID using hardware sequencing, find the corresponding flashchip entry, and copy it over to `flash->chip`. Identifying the chip was not previously required as ICH hardware sequencing handles chip-level details related to read/write/erase ops. However writeprotect operations require the chip entry to be identified so that chip->reg_bits can be used to compute status register values. BUG=b:253715389,b:253713774 BRANCH=none TEST=flashrom on dedede (JSL) identifies "W25Q128.V..M" chip TEST=flashrom -{r,v} on dedede TEST=write/erase bios region on dedede: flashrom -{E,w} --layout <(echo '0x381000:0xffffff bios') -i bios Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c 1 file changed, 90 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/69195/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: writeprotect,ichspi,spi25: handle register access constraints
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69129 ) Change subject: writeprotect,ichspi,spi25: handle register access constraints ...................................................................... Patch Set 10: (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/69129/comment/ac875194_b83e75ee PS7, Line 133: /* Some SPI masters like ICH7/ICH9 block opcodes for writing SR2/SR3/etc. */ > Do they? I thought they are swseq and this was only a limitation of hwseq? I thought so too but when I was debugging cyan/wizpig failures in b/240229722 I found that it didn't work. Looking at again, ichspi actually skips the reprogram_opcodes code path if ichspi_lock is set, maybe it would work if we forced it to take that code path. But that might break something else. We should still call probe_opcode here but I'll drop the comment since it might be misleading and it isn't necessary to understand what's going on. -- To view, visit https://review.coreboot.org/c/flashrom/+/69129 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2145749dcc51f4556550650dab5aa1049f879c45 Gerrit-Change-Number: 69129 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 05:35:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022