May 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: Add Gemini Lake support
by Edward O'Callaghan (Code Review) 14 May '21

14 May '21

Attention is currently required from: Sam McNally, Nico Huber, Angel Pons, Arthur Heymans. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54276 ) Change subject: Add Gemini Lake support ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: Thanks Angel! -- To view, visit https://review.coreboot.org/c/flashrom/+/54276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib5dcdf204166f44a8531c19b5f363b851d2ccd77 Gerrit-Change-Number: 54276 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 14 May 2021 12:41:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips.c: add support for W25Q32JW...M
by Edward O'Callaghan (Code Review) 14 May '21

14 May '21

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53946 ) Change subject: flashchips.c: add support for W25Q32JW...M ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/53946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7425e12658dd69c4ec8d3309dd591d09a935bb4d Gerrit-Change-Number: 53946 Gerrit-PatchSet: 6 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 14 May 2021 12:37:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: change chip name from 'W25Q64JW' to 'W25Q64JW...M'
by Edward O'Callaghan (Code Review) 14 May '21

14 May '21

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54291 ) Change subject: flashchips: change chip name from 'W25Q64JW' to 'W25Q64JW...M' ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib0dc914da286a191d22e666332b1063b88db4251 Gerrit-Change-Number: 54291 Gerrit-PatchSet: 1 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 14 May 2021 12:36:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: stlinkv3_spi.c: Clean up properly on all init error paths
by Miklós Márton (Code Review) 14 May '21

14 May '21

Attention is currently required from: Nico Huber, Miklós Márton, Angel Pons, Anastasia Klimchuk. Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54190 ) Change subject: stlinkv3_spi.c: Clean up properly on all init error paths ...................................................................... Patch Set 2: Code-Review+2 (2 comments) Patchset: PS2: LGTM and thanks for the cleanup! File stlinkv3_spi.c: https://review.coreboot.org/c/flashrom/+/54190/comment/a207fe85_b9f64707 PS1, Line 443: stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "STLINK_BRIDGE_CLOSE"); > Oh yes, there is a STLINK_BRIDGE_INIT_SPI in stlinkv3_spi_open(), maybe it is? […] Hello Anastasia, First of all: many thanks for cleaning out my crappy code! Yes the STLINK_BRIDGE_CLOSE is complementary to the STLINK_BRIDGE_INIT_SPI. Tldr: there are multiple bridge subsystems (I2C, CAN, GPIO) on the device and they could be closed with the STLINK_BRIDGE_CLOSE command with bridge subsystem specified by the command[2] "argument". However opening is done with separate commands (STLINK_BRIDGE_INIT_SPI/CAN/GPIO). -- To view, visit https://review.coreboot.org/c/flashrom/+/54190 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fabf48068635593bc86006c9642d8569eee8447 Gerrit-Change-Number: 54190 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 14 May 2021 07:10:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips.c: merge GD25B128B/GD25Q128B and GD25Q127C/GD25Q128C
by Nikolai Artemiev (Code Review) 13 May '21

13 May '21

Attention is currently required from: Nico Huber, Angel Pons. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52883 to look at the new patch set (#2). Change subject: flashchips.c: merge GD25B128B/GD25Q128B and GD25Q127C/GD25Q128C ...................................................................... flashchips.c: merge GD25B128B/GD25Q128B and GD25Q127C/GD25Q128C The 'B' and 'C' revisions have the same vendor ID / device ID and are mostly identical. Other than the names, the only difference between the definitions was that the 'C' definition had FEATURE_QPI enabled and the 'B' definition did not. This difference appears to be a result of the 'B' definition predating the the FEATURE_QPI feature bit and the 'C' definition being added later. The 'B' chips also support QPI, so the merged definition has FEATURE_QPI enabled. Merging the entries allows flashrom to automatically identify the chip without needing to be given the chip with the -C flag. Chip revision history: - The 'GD25B128B/GD25Q128B' was originally added as 'GD25Q128' in `commit 1525b2ad16e07f035b1de70fadd05a7018ea5756` and later revised to 'GD25Q128B' and then 'GD25B128B/GD25Q128B'. - The 'GD25Q127C/GD25Q128C' definition was added in `commit e0c7abf219b81ad049d09a4671ebc9196153d308` as 'GD25Q128C' and later renamed to 'GD25Q127C/GD25Q128C' BUG=b:153598437 BRANCH=none TEST=builds Change-Id: I688f2e15aef61afbec728a9a81094bee56d6fbfa Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashchips.c 1 file changed, 6 insertions(+), 45 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/52883/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52883 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I688f2e15aef61afbec728a9a81094bee56d6fbfa Gerrit-Change-Number: 52883 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashchips.c: add support for W25Q32JW...M
by Nikolai Artemiev (Code Review) 13 May '21

13 May '21

Attention is currently required from: Edward O'Callaghan, Angel Pons. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53946 ) Change subject: flashchips.c: add support for W25Q32JW...M ...................................................................... Patch Set 6: (2 comments) File flashchips.h: https://review.coreboot.org/c/flashrom/+/53946/comment/b139f071_2307b16a PS6, Line 980: _M _M added here to match the part name. Subsequent patch makes the 64Mb chip consistent. File flashchips.c: https://review.coreboot.org/c/flashrom/+/53946/comment/a57f63e3_7800ffbd PS6, Line 17583: W25Q32JW...M I've changed the chip's name since I realized that 'IM' is not the only matching suffix, and there are also 'JW' parts with the same device ID. -- To view, visit https://review.coreboot.org/c/flashrom/+/53946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7425e12658dd69c4ec8d3309dd591d09a935bb4d Gerrit-Change-Number: 53946 Gerrit-PatchSet: 6 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 14 May 2021 05:40:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: change chip name from 'W25Q64JW' to 'W25Q64JW...M'
by Nikolai Artemiev (Code Review) 13 May '21

13 May '21

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/54291 ) Change subject: flashchips: change chip name from 'W25Q64JW' to 'W25Q64JW...M' ...................................................................... flashchips: change chip name from 'W25Q64JW' to 'W25Q64JW...M' According to the W25Q64JW datasheet rev. E, only devices ending with the letter 'M' have a device ID of 8017h. There are other variants with different device IDs. This patch makes the 'W25Q64JW...M' definition consistent with the 'W25Q32JW...M' definition. The device ID macro defined in flashchips.h has also been renamed from WINBOND_NEX_W25Q64JW to WINBOND_NEX_W25Q64JW_M. BUG=b:166294558 BRANCH=none TEST=builds Change-Id: Ib0dc914da286a191d22e666332b1063b88db4251 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashchips.c M flashchips.h 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/54291/1 diff --git a/flashchips.c b/flashchips.c index 1b281aa..ef5ea5e 100644 --- a/flashchips.c +++ b/flashchips.c @@ -17860,10 +17860,10 @@ { .vendor = "Winbond", - .name = "W25Q64JW", + .name = "W25Q64JW...M", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, - .model_id = WINBOND_NEX_W25Q64JW, + .model_id = WINBOND_NEX_W25Q64JW_M, .total_size = 8192, .page_size = 256, /* OTP: 256B total; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ diff --git a/flashchips.h b/flashchips.h index 4bbaef1..37eadb2 100644 --- a/flashchips.h +++ b/flashchips.h @@ -978,7 +978,7 @@ #define WINBOND_NEX_W25Q128_V_M 0x7018 /* W25Q128JVSM */ #define WINBOND_NEX_W25Q256JV_M 0x7019 /* W25Q256JV_M (QE=0) */ #define WINBOND_NEX_W25Q32JW_M 0x8016 /* W25Q32JW...M */ -#define WINBOND_NEX_W25Q64JW 0x8017 +#define WINBOND_NEX_W25Q64JW_M 0x8017 /* W25Q64JW...M */ #define WINBOND_NEX_W25Q128_DTR 0x8018 /* W25Q128JW_DTR */ #define WINBOND_NEX_W25Q256_DTR 0x8019 /* W25Q256JW_DTR aka W25Q256256JW-IM */ -- To view, visit https://review.coreboot.org/c/flashrom/+/54291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib0dc914da286a191d22e666332b1063b88db4251 Gerrit-Change-Number: 54291 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashchips.c: add support for W25Q32JW...M
by Nikolai Artemiev (Code Review) 13 May '21

13 May '21

Attention is currently required from: Nikolai Artemiev. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/53946 to look at the new patch set (#6). Change subject: flashchips.c: add support for W25Q32JW...M ...................................................................... flashchips.c: add support for W25Q32JW...M The chip was added to cros flashrom in `commit 1fc77dd1ee27a5d6e58a82c6ed6ed390a15372d7`. Quoting from the commit message: > We have varied the correct chip name is reported as well as > write and read 16MBytes of random data and verified the checksum's match. > Further, --wp-list appears to report the correct ranges. > > BUG=b:130199963 > BRANCH=none > TEST=Ran flashrom with a Dediprog SF100, RW random data and checksum matched. Change-Id: I7425e12658dd69c4ec8d3309dd591d09a935bb4d Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashchips.c M flashchips.h 2 files changed, 39 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/53946/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/53946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7425e12658dd69c4ec8d3309dd591d09a935bb4d Gerrit-Change-Number: 53946 Gerrit-PatchSet: 6 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: stlinkv3_spi.c: Clean up properly on all init error paths
by Anastasia Klimchuk (Code Review) 13 May '21

13 May '21

Attention is currently required from: Nico Huber, Miklós Márton, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54190 ) Change subject: stlinkv3_spi.c: Clean up properly on all init error paths ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/54190/comment/4c432dc4_766d7459 PS1, Line 7: Cleanup > nit: The verb form has a space: `clean up` Done File stlinkv3_spi.c: https://review.coreboot.org/c/flashrom/+/54190/comment/ff432a60_4e4d8e6e PS1, Line 443: stlinkv3_command(command, sizeof(command), answer, sizeof(answer), "STLINK_BRIDGE_CLOSE"); > Looks like this might be complementary to stlinkv3_spi_open(), but I'm not sure. Oh yes, there is a STLINK_BRIDGE_INIT_SPI in stlinkv3_spi_open(), maybe it is? I think Miklós knows the answer! :) -- To view, visit https://review.coreboot.org/c/flashrom/+/54190 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fabf48068635593bc86006c9642d8569eee8447 Gerrit-Change-Number: 54190 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 14 May 2021 04:21:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: stlinkv3_spi.c: Clean up properly on all init error paths
by Anastasia Klimchuk (Code Review) 13 May '21

13 May '21

Attention is currently required from: Nico Huber, Miklós Márton, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Miklós Márton, Miklós Márton, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/54190 to look at the new patch set (#2). Change subject: stlinkv3_spi.c: Clean up properly on all init error paths ...................................................................... stlinkv3_spi.c: Clean up properly on all init error paths Do a cleanup: close handle and exit usb context in cases when init fails. If register_spi_master() fails, going to err_exit cleanup is not needed because at that point shutdown function has already been registered and it does the job. BUG=b:185191942 TEST=builds Change-Id: I9fabf48068635593bc86006c9642d8569eee8447 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M stlinkv3_spi.c 1 file changed, 7 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/90/54190/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54190 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fabf48068635593bc86006c9642d8569eee8447 Gerrit-Change-Number: 54190 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021