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

Change in flashrom[master]: flashchips: Add Spansion/Cypress S25FL256L
by Thomas Walker (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Nico Huber, Thomas Walker. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52310 to look at the new patch set (#6). Change subject: flashchips: Add Spansion/Cypress S25FL256L ...................................................................... flashchips: Add Spansion/Cypress S25FL256L Conducted random write test using Adafruit FT232H programmer. Confirmed read/write/erase/probe and VERIFIED on completion. Signed-off-by: Thomas Walker <thh.walker(a)gmail.com> Change-Id: I4e8cba554d0590c94dac92aa91e9ab400efca281 --- M chipdrivers.h M flashchips.c M flashchips.h M spi25.c 4 files changed, 57 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/52310/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/52310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4e8cba554d0590c94dac92aa91e9ab400efca281 Gerrit-Change-Number: 52310 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Walker <thh.walker(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Thomas Walker <thh.walker(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: jlink_spi.c: Drop jaylink_ prefix for spi data struct members
by Miklós Márton (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52661 ) Change subject: jlink_spi.c: Drop jaylink_ prefix for spi data struct members ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Ship it! -- To view, visit https://review.coreboot.org/c/flashrom/+/52661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6e68e0dabb6bfad1088ff975445961294bbc03d Gerrit-Change-Number: 52661 Gerrit-PatchSet: 1 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Apr 2021 19:59:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: jlink_spi.c: Drop jaylink_ prefix for spi data struct members
by Miklós Márton (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52661 ) Change subject: jlink_spi.c: Drop jaylink_ prefix for spi data struct members ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: Ship it! -- To view, visit https://review.coreboot.org/c/flashrom/+/52661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6e68e0dabb6bfad1088ff975445961294bbc03d Gerrit-Change-Number: 52661 Gerrit-PatchSet: 1 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Apr 2021 19:09:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: chipset_enable.c: Add CMP-H IDs
by Gaggery Tsai (Code Review) 26 Apr '21

26 Apr '21

Gaggery Tsai has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37677 ) Change subject: chipset_enable.c: Add CMP-H IDs ...................................................................... chipset_enable.c: Add CMP-H IDs This patch adds CMP-H support. They are HM470, WM490, QM480, W480, H470, Z490 and Q470. TEST=build flashrom and run on CML-S with CMP-H flashrom -p internal -w ./coreboot.rom reboot and check the code is flashed correctly Signed-off-by: Gaggery Tsai <gaggery.tsai(a)intel.com> Change-Id: Ic7f04fc5cbe3422cbd219c46586c32fc847c921f --- M chipset_enable.c 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/37677/1 diff --git a/chipset_enable.c b/chipset_enable.c index b55852c..62a19af 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -2046,6 +2046,13 @@ {0x8086, 0xa30c, B_S, NT, "Intel", "QM370", enable_flash_pch300}, {0x8086, 0xa30d, B_S, NT, "Intel", "HM370", enable_flash_pch300}, {0x8086, 0xa30e, B_S, DEP, "Intel", "CM246", enable_flash_pch300}, + {0x8086, 0x068d, B_S, NT, "Intel", "HM470", enable_flash_pch300}, + {0x8086, 0x068e, B_S, NT, "Intel", "WM490", enable_flash_pch300}, + {0x8086, 0x068c, B_S, NT, "Intel", "QM480", enable_flash_pch300}, + {0x8086, 0x0697, B_S, NT, "Intel", "W480", enable_flash_pch300}, + {0x8086, 0x0684, B_S, NT, "Intel", "H470", enable_flash_pch300}, + {0x8086, 0x0685, B_S, NT, "Intel", "Z490", enable_flash_pch300}, + {0x8086, 0x0687, B_S, NT, "Intel", "Q470", enable_flash_pch300}, #endif {0}, }; -- To view, visit https://review.coreboot.org/c/flashrom/+/37677 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic7f04fc5cbe3422cbd219c46586c32fc847c921f Gerrit-Change-Number: 37677 Gerrit-PatchSet: 1 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-MessageType: newchange

7 25

Change in flashrom[refs/meta/config]: Allow merging content when submitting
by Angel Pons (Code Review) 26 Apr '21

26 Apr '21

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/52427 ) Change subject: Allow merging content when submitting ...................................................................... Abandoned Not necessary, changes can simply be rebased from within Gerrit before submitting them -- To view, visit https://review.coreboot.org/c/flashrom/+/52427 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: refs/meta/config Gerrit-Change-Id: Iace6052fb7ff20ea7b9d93635e04e56a24799bbd Gerrit-Change-Number: 52427 Gerrit-PatchSet: 5 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Change in flashrom[master]: s25f.c: Fix mismatched function definitions
by Angel Pons (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52473 ) Change subject: s25f.c: Fix mismatched function definitions ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52473/comment/be62f7b2_31eb5a6e PS1, Line 8: : This was missed because `uint32_t` is `unsigned int` in most cases. : However, it is not the case for DJGPP 6.1.0 for some reason. > You got no response for a couple of days because I have been recovering from surgery WTF.. That's why -2'ed to give a pause on this to give me a chance to discuss. I've not actually been at work but been trying to keep up to date so Anastasia and the community can keep moving. I wasn't aware of that, my apologies. > I cannot see how 'unsigned int' would be preferred over uint32_t, the correct type is clearly uint32_t for the intended case. Why do you say `uint32_t` is the correct type? https://www.gnu.org/software/libc/manual/html_node/Integers.html Fixed-width or exact-width types like `uint32_t` are meant to be used when one needs an integer of exactly N bits. In this case, nothing specifically requires that the parameters be 32 bits wide, so I wouldn't use an exact-width type for them. https://www.gnu.org/software/libc/manual/html_node/Important-Data-Types.html At least for `blocklen`, I'd say `size_t` would be a reasonable choice. As per the link above, it may not be compatible with implementations that predate ISO C, but flashrom already uses size_t in other places. In any case, manibuilder is the way to go to assess compatibility. -- To view, visit https://review.coreboot.org/c/flashrom/+/52473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I656a72b85d4c70b57f6ff9268186a4a60933f8a9 Gerrit-Change-Number: 52473 Gerrit-PatchSet: 3 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: 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: 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-Comment-Date: Mon, 26 Apr 2021 10:01:31 +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> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Alan Green, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 30: (6 comments) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/fbc132c6_cd7eafcc PS25, Line 317: if (!cmds->readcnt && ((cmds + 1)->writecnt || (cmds + 1)->readcnt)){ > > Checking if next command fits is a very good idea! I did as you proposed. […] Done File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/024d0c77_50e16669 PS29, Line 171: return 12 + cmd->writecnt + cmd->readcnt <= buffer_size; > I just realized we could make the check more accurate, taking into account […] I think it is a cleaner solution and, theoretically, it could save some send_buf() calls. https://review.coreboot.org/c/flashrom/+/40477/comment/dc06cb71_b4454fd0 PS29, Line 306: /* Overflow check for buf : * 12 : up to 12 CMD-Bytes are added below : */ > I think we can drop the comment here, the function name should be self-explanatory. Done https://review.coreboot.org/c/flashrom/+/40477/comment/6b61986f_4e76f3ee PS29, Line 344: ((cmds + 1)->writecnt || (cmds + 1)->readcnt) && : ft2232_spi_command_fits((cmds + 1), FTDI_HW_BUFFER_SIZE - i)) { > Please indent either with 4 spaces or two tabs (relative to the `if`). […] Done https://review.coreboot.org/c/flashrom/+/40477/comment/a2837648_91c616e5 PS29, Line 352: msg_perr("send_buf failed: %i\n", ret); : break; > Needs braces. Doing too much python ;-) https://review.coreboot.org/c/flashrom/+/40477/comment/a914ff65_d5842eab PS29, Line 358: msg_perr("get_buf failed: %i\n", ret); : break; > Same here. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 30 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Alan Green <avg(a)google.com> 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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Mon, 26 Apr 2021 07:17:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Buhrow Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Simon Buhrow, Alan Green, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Hello build bot (Jenkins), Alan Green, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Samir Ibradžić, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/40477 to look at the new patch set (#30). Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... ft2232_spi.c: Implement spi_send_multicommand() Every ftdi_write_data() call is quite time consuming as the ftdi-chips seems to take always 2-3ms to respond. This leads to what the comment already says: Minimize USB transfers by packing as many commands as possible together. So I packed the WREN command together with the following operation which can be program or erase operation. This saves about 1 minute when programming a 128MBit Flash with my config! Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a --- M ft2232_spi.c 1 file changed, 91 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/30 -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 30 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: mec1308.c: Separate shutdown from failed init cleanup
by Anastasia Klimchuk (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52597 ) Change subject: mec1308.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 3: (2 comments) File mec1308.c: https://review.coreboot.org/c/flashrom/+/52597/comment/4c53f134_01b40c5c PS2, Line 496: goto mec1308_init_exit; > It's becoming more and more obvious that the original cleanup procedure […] I noticed this in other places, that shutdown != cleanup and all of them I want to sort eventually. I took the approach not to change current behavior as a first step. I can do "partial cleanup" for the case when SMI failed to be disabled, but that would be changing current behavior here, so maybe in the separate patch? What do you think? https://review.coreboot.org/c/flashrom/+/52597/comment/4075bf85_95d49bcb PS2, Line 519: failed_init_cleanup: : mec1308_shutdown(ctx_data); : return 1; : : mec1308_init_exit: > Would be nice to align the label names somehow. I first thought "aren't […] My second attempt is init_err_cleanup_exit init_err_exit -- To view, visit https://review.coreboot.org/c/flashrom/+/52597 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Gerrit-Change-Number: 52597 Gerrit-PatchSet: 3 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Apr 2021 05:31:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Untangle successful vs failed init paths
by Anastasia Klimchuk (Code Review) 26 Apr '21

26 Apr '21

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52596 ) Change subject: mec1308.c: Untangle successful vs failed init paths ...................................................................... Patch Set 3: (1 comment) File mec1308.c: https://review.coreboot.org/c/flashrom/+/52596/comment/fab96e82_c35fa8c8 PS1, Line 519: mec1308_init_exit > I would probably rename it, e.g. `mec1308_init_err`. It's not "the" exit anymore. […] Yeah I was thinking about those label names a lot, I agree just saying this one is "exit" is not ideal. Especially since this patch aims to "Untangle successful vs failed init paths", I feel it's relevant here to get the right name. So my second attempt is "init_err_exit" here (and "init_err_cleanup_exit" in 52597). -- To view, visit https://review.coreboot.org/c/flashrom/+/52596 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibf35335501e59636c544af124ad7a04a186790b4 Gerrit-Change-Number: 52596 Gerrit-PatchSet: 3 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 26 Apr 2021 05:26:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021