November 2022 - flashrom-gerrit

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

06 Nov '22

Edward O'Callaghan has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/67240 ) Change subject: tree/: Convert flashchip erase_block func ptr to enumerate ...................................................................... Abandoned split into CB:69131 && CB:69133 as requested. -- To view, visit https://review.coreboot.org/c/flashrom/+/67240 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I58415a4f2589812b26d284c96876e88f8d9acf52 Gerrit-Change-Number: 67240 Gerrit-PatchSet: 15 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

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

06 Nov '22

Attention is currently required from: Edward O'Callaghan, 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 (#5). Change subject: tree/: Convert flashchip erase_block func ptr to enumerate ...................................................................... tree/: Convert flashchip erase_block func ptr to enumerate 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, 123 insertions(+), 47 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/69133/5 -- 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: 5 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-MessageType: newpatchset

1 0

[M] Change in flashrom[1.3.x]: sb600spi.c: Drop "Promontory" support
by Angel Pons (Code Review) 06 Nov '22

06 Nov '22

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68824 ) Change subject: sb600spi.c: Drop "Promontory" support ...................................................................... sb600spi.c: Drop "Promontory" support The "Promontory" code is riddled with issues, some of them can result in soft bricks. Moreover, Promontory doesn't have a SPI controller. Drop support for "Promontory" in flashrom for now: it's holding back the entire project and it's unlikely that it'll be fixed in a timely manner. Change-Id: I1457946dce68321b496d9ffa40a0c5ab46455f72 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68824 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M sb600spi.c 1 file changed, 21 insertions(+), 42 deletions(-) Approvals: build bot (Jenkins): Verified Swift Geek (Sebastian Grzywna): Looks good to me, approved Felix Singer: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/sb600spi.c b/sb600spi.c index 6020145..2e7be81 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -48,7 +48,6 @@ CHIPSET_HUDSON234, CHIPSET_BOLTON, CHIPSET_YANGTZE, - CHIPSET_PROMONTORY, }; #define FIFO_SIZE_OLD 8 @@ -124,18 +123,6 @@ if (rev == 0x4a) { msg_pdbg("Yangtze detected.\n"); return CHIPSET_YANGTZE; - /** - * FCH chipsets called 'Promontory' are one's with the - * so-called SPI100 ip core that uses memory mapping and - * not a ring buffer for transactions. Typically this is - * found on both Stoney Ridge and Zen platforms. - * - * The revisions I have found by searching various lspci - * outputs are as follows: 0x4b, 0x59, 0x61 & 0x71. - */ - } else if (rev == 0x4b || rev == 0x51 || rev == 0x59 || rev == 0x61 || rev == 0x71) { - msg_pdbg("Promontory (rev 0x%02x) detected.\n", rev); - return CHIPSET_PROMONTORY; } else { msg_pwarn("FCH device found but SMBus revision 0x%02x does not match known values.\n" "Please report this to flashrom(a)flashrom.org and include this log and\n" @@ -572,18 +559,6 @@ return amd_imc_shutdown(dev); } -static int promontory_read_memmapped(struct flashctx *flash, uint8_t *buf, - unsigned int start, unsigned int len) -{ - struct sb600spi_data * data = (struct sb600spi_data *)flash->mst->spi.data; - if (!data->flash) { - map_flash(flash); - data->flash = flash; /* keep a copy of flashctx for unmap() on tear-down. */ - } - mmio_readn((void *)(flash->virtual_memory + start), buf, len); - return 0; -} - static int sb600spi_shutdown(void *data) { struct sb600spi_data *sb600_data = data; @@ -623,20 +598,6 @@ .probe_opcode = default_spi_probe_opcode, }; -static const struct spi_master spi_master_promontory = { - .max_data_read = MAX_DATA_READ_UNLIMITED, - .max_data_write = FIFO_SIZE_YANGTZE - 3, - .command = spi100_spi_send_command, - .multicommand = default_spi_send_multicommand, - .map_flash_region = physmap, - .unmap_flash_region = physunmap, - .read = promontory_read_memmapped, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, - .shutdown = sb600spi_shutdown, - .probe_opcode = default_spi_probe_opcode, -}; - int sb600_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev) { struct pci_dev *smbus_dev; @@ -740,7 +701,6 @@ case CHIPSET_SB89XX: case CHIPSET_HUDSON234: case CHIPSET_YANGTZE: - case CHIPSET_PROMONTORY: msg_pdbg(", SpiBusy=%i", (tmp >> 31) & 0x1); default: break; } @@ -817,8 +777,6 @@ register_spi_master(&spi_master_sb600, data); else if (amd_gen == CHIPSET_YANGTZE) register_spi_master(&spi_master_yangtze, data); - else - register_spi_master(&spi_master_promontory, data); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/68824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I1457946dce68321b496d9ffa40a0c5ab46455f72 Gerrit-Change-Number: 68824 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: board_enable.c: plumb paralle_bus_size into board_enable_flash() helper
by Angel Pons (Code Review) 05 Nov '22

05 Nov '22

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/+/68674 ) Change subject: board_enable.c: plumb paralle_bus_size into board_enable_flash() helper ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68674 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I180b047fc07be946cdc5ba93871f9ff4f4c67df4 Gerrit-Change-Number: 68674 Gerrit-PatchSet: 3 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: Sat, 05 Nov 2022 07:58:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to identify chip
by Angel Pons (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to identify chip ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/0de418ef_aeddc16b PS2, Line 9: BUG=b:253715389,b:253713774 > re Angel, to answer your question directly - the bugs I created for Nik don't contain anything usefu […] Ah, so downstream has some extra functionality, probably to control WP on platforms where only hwseq is available. Agree that it'd be great to elaborate. -- 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: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 05 Nov 2022 07:43:48 +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

[M] Change in flashrom[master]: tree/: Convert flashchip erase_block func ptr to enumerate [ALT]
by Angel Pons (Code Review) 04 Nov '22

04 Nov '22

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 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69133/comment/635a5562_fac57e1c PS4, Line 7: [ALT] > I can or push this over the change-id of https://review.coreboot. […] We can go with these if it's OK with you. -- 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: 4 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: Fri, 04 Nov 2022 23:44:24 +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

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

04 Nov '22

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 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69133/comment/6eae0166_5a9621c5 PS4, Line 7: [ALT] > drop? I can or push this over the change-id of https://review.coreboot.org/c/flashrom/+/67240 and abandon this? -- 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: 4 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: Fri, 04 Nov 2022 23:40:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: ensure chip erase operation is executed
by Edward O'Callaghan (Code Review) 04 Nov '22

04 Nov '22

Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69130 ) Change subject: tests: ensure chip erase operation is executed ...................................................................... Patch Set 3: (1 comment) File include/flash.h: https://review.coreboot.org/c/flashrom/+/69130/comment/875bd743_b07d26dc PS1, Line 159: #define UNERASED_VALUE(flash) > UNERASED_VALUE could also be 0x55. […] *shivers* what about the 0xAA chips ;) -- To view, visit https://review.coreboot.org/c/flashrom/+/69130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia00444dcd2ad96c64832a13201efbd064cd7302d Gerrit-Change-Number: 69130 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 04 Nov 2022 23:00:45 +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> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> 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 identify chip
by Edward O'Callaghan (Code Review) 04 Nov '22

04 Nov '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to identify chip ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/d247ab93_525bbadf PS2, Line 9: BUG=b:253715389,b:253713774 > Yep, please include the details here Nik so the story is more complete for upstream but also the git […] re Angel, to answer your question directly - the bugs I created for Nik don't contain anything useful. Just a title for him to track progress with: * b/253715389 - spi25_statusreg.c hunk missing from upstream * b/253713774 - writeprotect.c hunk missing from upstream the above needs a commit message body as mentioned. -- 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: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: 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: Fri, 04 Nov 2022 22:59:28 +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

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to identify chip
by Edward O'Callaghan (Code Review) 04 Nov '22

04 Nov '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to identify chip ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/4ecaa7c5_8372e272 PS2, Line 9: BUG=b:253715389,b:253713774 > Out of curiosity, what are these bug tracker entries about? Yep, please include the details here Nik so the story is more complete for upstream but also the git history. Specifically explain how CSME presents a quasi-opaque chip however you still need to understand the chip behind hwseq for things like wp. ``` /* support writeprotect. */ flash->chip->reg_bits = entry->reg_bits; flash->chip->decode_range = entry->decode_range; ``` https://review.coreboot.org/c/flashrom/+/69195/comment/4a46fa58_e39be76e PS2, Line 11: TEST=builds I think you can include this was actually tested here across the DUT pool? -- 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: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: 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: Fri, 04 Nov 2022 22:54:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022