June 2019 - flashrom-gerrit - mail.coreboot.org

Change in ...flashrom[master]: Rework internal bus handling and laptop bail-out

by Nico Huber (Code Review)

Hello Angel Pons, Arthur Heymans, Matt DeVillier, Thomas Heijligen, David Hendricks, Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/28716 to look at the new patch set (#7). Change subject: Rework internal bus handling and laptop bail-out ...................................................................... Rework internal bus handling and laptop bail-out We used to bail out on any unknown laptop. However, modern systems with SPI flashes don't suffer from the original problem. Even if a flash chip is shared with the EC, the latter has to expect the host to send regular JEDEC SPI commands any time. So instead of bailing out, we limit the set of buses to probe. If we suspect to be running on a laptop, we only allow probing of SPI and opaque programmers. The user can still use the existing force options to probe all buses. This will obsolete some board-enables that could be moved to `print.c` in follow-up commits. Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M board_enable.c M chipset_enable.c M flashrom.8.tmpl M ichspi.c M internal.c M programmer.h 6 files changed, 500 insertions(+), 424 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/28716/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/28716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Gerrit-Change-Number: 28716 Gerrit-PatchSet: 7 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 year, 5 months

1
0
0 0

Change in ...flashrom[master]: Rework internal bus handling and laptop bail-out

by Thomas Heijligen (Code Review)

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/28716 ) Change subject: Rework internal bus handling and laptop bail-out ...................................................................... Patch Set 6: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/28716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Gerrit-Change-Number: 28716 Gerrit-PatchSet: 6 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 05 Jun 2019 11:21:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

Change in ...flashrom[master]: Rework internal bus handling and laptop bail-out

by Nico Huber (Code Review)

Hello Angel Pons, Arthur Heymans, Matt DeVillier, David Hendricks, Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/28716 to look at the new patch set (#6). Change subject: Rework internal bus handling and laptop bail-out ...................................................................... Rework internal bus handling and laptop bail-out We used to bail out on any unknown laptop. However, modern systems with SPI flashes don't suffer from the original problem. Even if a flash chip is shared with the EC, the latter has to expect the host to send regular JEDEC SPI commands any time. So instead of bailing out, we limit the set of buses to probe. If we suspect to be running on a laptop, we only allow probing of SPI and opaque programmers. The user can still use the existing force options to probe all buses. This will obsolete some board-enables that could be moved to `print.c` in follow-up commits. Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M board_enable.c M chipset_enable.c M flashrom.8.tmpl M ichspi.c M internal.c M programmer.h 6 files changed, 500 insertions(+), 424 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/28716/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/28716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Gerrit-Change-Number: 28716 Gerrit-PatchSet: 6 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 year, 5 months

1
0
0 0

Change in ...flashrom[master]: Rework internal bus handling and laptop bail-out

by Thomas Heijligen (Code Review)

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/28716 ) Change subject: Rework internal bus handling and laptop bail-out ...................................................................... Patch Set 5: please adapt documentation (man page) - remove forece_I_want_to_brick - add hint to iomem=relaxed -- To view, visit https://review.coreboot.org/c/flashrom/+/28716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Gerrit-Change-Number: 28716 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 04 Jun 2019 20:55:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

Change in ...flashrom[master]: Rework internal bus handling and laptop bail-out

by Arthur Heymans (Code Review)

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/28716 ) Change subject: Rework internal bus handling and laptop bail-out ...................................................................... Patch Set 5: Code-Review+1 manpage -- To view, visit https://review.coreboot.org/c/flashrom/+/28716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Gerrit-Change-Number: 28716 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 04 Jun 2019 18:07:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

Change in ...flashrom[master]: dediprog: Implement 4BA EAR mode for protocol v1

by Nico Huber (Code Review)

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/32016 Change subject: dediprog: Implement 4BA EAR mode for protocol v1 ...................................................................... dediprog: Implement 4BA EAR mode for protocol v1 With an SF100 and protocol version 1, using the extended address register of the flash chip seems safe. Make use of that and remove the broken 4BA modes flag. Tested with SF100 V:5.1.9 and W25Q256FV. Change-Id: If926cf3cbbebf88231116c4d65bafc19d23646f6 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M chipdrivers.h M dediprog.c M spi25.c 3 files changed, 26 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/32016/1 diff --git a/chipdrivers.h b/chipdrivers.h index 134ed26..e380878 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -58,6 +58,7 @@ int spi_write_chunked(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len, unsigned int chunksize); int spi_enter_4ba(struct flashctx *flash); int spi_exit_4ba(struct flashctx *flash); +int spi_set_extended_address(struct flashctx *, uint8_t addr_high); /* spi25_statusreg.c */ diff --git a/dediprog.c b/dediprog.c index 03675e5..d48c541 100644 --- a/dediprog.c +++ b/dediprog.c @@ -360,7 +360,7 @@ return 0; } -static void prepare_rw_cmd( +static int prepare_rw_cmd( struct flashctx *const flash, uint8_t *data_packet, unsigned int count, uint8_t dedi_spi_cmd, unsigned int *value, unsigned int *idx, unsigned int start, int is_read) { /* First 5 bytes are common in both generations. */ @@ -400,9 +400,23 @@ } } } else { - *value = start % 0x10000; - *idx = start / 0x10000; + if (flash->chip->feature_bits & FEATURE_4BA_EXT_ADDR) { + if (spi_set_extended_address(flash, start >> 24)) + return 1; + } else if (start >> 24) { + msg_cerr("Can't handle 4-byte address with dediprog.\n"); + return 1; + } + /* + * We don't know how the dediprog firmware handles 4-byte + * addresses. So let's not tell it what we are doing and + * only send the lower 3 bytes. + */ + *value = start & 0xffff; + *idx = (start >> 16) & 0xff; } + + return 0; } /* Bulk read interface, will read multiple 512 byte chunks aligned to 512 bytes. @@ -448,7 +462,8 @@ uint8_t data_packet[command_packet_size]; unsigned int value, idx; - prepare_rw_cmd(flash, data_packet, count, READ_MODE_STD, &value, &idx, start, 1); + if (prepare_rw_cmd(flash, data_packet, count, READ_MODE_STD, &value, &idx, start, 1)) + return 1; int ret = dediprog_write(CMD_READ, value, idx, data_packet, sizeof(data_packet)); if (ret != sizeof(data_packet)) { @@ -604,7 +619,8 @@ uint8_t data_packet[command_packet_size]; unsigned int value, idx; - prepare_rw_cmd(flash, data_packet, count, dedi_spi_cmd, &value, &idx, start, 0); + if (prepare_rw_cmd(flash, data_packet, count, dedi_spi_cmd, &value, &idx, start, 0)) + return 1; int ret = dediprog_write(CMD_WRITE, value, idx, data_packet, sizeof(data_packet)); if (ret != sizeof(data_packet)) { msg_perr("Command Write SPI Bulk failed, %s!\n", libusb_error_name(ret)); @@ -1134,6 +1150,9 @@ if (dediprog_standalone_mode()) return 1; + if (dediprog_devicetype == DEV_SF100 && protocol() == PROTOCOL_V1) + spi_master_dediprog.features &= ~SPI_MASTER_NO_4BA_MODES; + if (dediprog_devicetype == DEV_SF600 && protocol() == PROTOCOL_V2) spi_master_dediprog.features |= SPI_MASTER_4BA; diff --git a/spi25.c b/spi25.c index 8b6c462..fd87dc9 100644 --- a/spi25.c +++ b/spi25.c @@ -363,7 +363,7 @@ return result; } -static int spi_set_extended_address(struct flashctx *const flash, const uint8_t addr_high) +int spi_set_extended_address(struct flashctx *const flash, const uint8_t addr_high) { if (flash->address_high_byte != addr_high && spi_write_extended_address_register(flash, addr_high)) -- To view, visit https://review.coreboot.org/c/flashrom/+/32016 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If926cf3cbbebf88231116c4d65bafc19d23646f6 Gerrit-Change-Number: 32016 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 year, 5 months

2
2
0 0

Change in ...flashrom[master]: Rework internal bus handling and laptop bail-out

by Arthur Heymans (Code Review)

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/28716 ) Change subject: Rework internal bus handling and laptop bail-out ...................................................................... Patch Set 5: Code-Review+2 (1 comment) Looks fine https://review.coreboot.org/#/c/28716/5/internal.c File internal.c: https://review.coreboot.org/#/c/28716/5/internal.c@285 PS5, Line 285: is_laptop && !(laptop_ok Could we have enums for is_laptop and laptop_ok? -- To view, visit https://review.coreboot.org/c/flashrom/+/28716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Gerrit-Change-Number: 28716 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 03 Jun 2019 22:41:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

Change in ...flashrom[master]: Rework internal bus handling and laptop bail-out

by Nico Huber (Code Review)

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/28716 ) Change subject: Rework internal bus handling and laptop bail-out ...................................................................... Patch Set 5: (2 comments) Rebased again (properly, I hope) and added a commit message. https://review.coreboot.org/#/c/28716/1/chipset_enable.c File chipset_enable.c: https://review.coreboot.org/#/c/28716/1/chipset_enable.c@773 PS1, Line 773: > Yes? Done by rebase? https://review.coreboot.org/#/c/28716/1/internal.c File internal.c: https://review.coreboot.org/#/c/28716/1/internal.c@283 PS1, Line 283: proped > proped? Maybe probed? Done -- To view, visit https://review.coreboot.org/c/flashrom/+/28716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbda8cf0c10d7786106f14f0d18c3dcce35f0a3 Gerrit-Change-Number: 28716 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 03 Jun 2019 22:26:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

Change in ...flashrom[master]: chipset_enable: Mark Intel QS77 as DEP

by Evgeny Zinoviev (Code Review)

Evgeny Zinoviev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/33164 Change subject: chipset_enable: Mark Intel QS77 as DEP ...................................................................... chipset_enable: Mark Intel QS77 as DEP Tested reading and writing with `-p internal` on MacBook Air 5,1 with Intel QS77. Change-Id: I508b6379507c2881c976d6baf7348b1161449cfe Signed-off-by: Evgeny Zinoviev <me(a)ch1p.io> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/33164/1 diff --git a/chipset_enable.c b/chipset_enable.c index 6280876..19fd658 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1746,7 +1746,7 @@ {0x8086, 0x1e4a, DEP, "Intel", "H77", enable_flash_pch7}, {0x8086, 0x1e53, NT, "Intel", "C216", enable_flash_pch7}, {0x8086, 0x1e55, DEP, "Intel", "QM77", enable_flash_pch7}, - {0x8086, 0x1e56, NT, "Intel", "QS77", enable_flash_pch7}, + {0x8086, 0x1e56, DEP, "Intel", "QS77", enable_flash_pch7}, {0x8086, 0x1e57, DEP, "Intel", "HM77", enable_flash_pch7}, {0x8086, 0x1e58, NT, "Intel", "UM77", enable_flash_pch7}, {0x8086, 0x1e59, NT, "Intel", "HM76", enable_flash_pch7}, -- To view, visit https://review.coreboot.org/c/flashrom/+/33164 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I508b6379507c2881c976d6baf7348b1161449cfe Gerrit-Change-Number: 33164 Gerrit-PatchSet: 1 Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.com> Gerrit-MessageType: newchange

1 year, 5 months

2
4
0 0

Change in ...flashrom[1.1.x]: usbdev: Only match requested USB devices

by Nico Huber (Code Review)

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/32892 ) Change subject: usbdev: Only match requested USB devices ...................................................................... usbdev: Only match requested USB devices Don't use a device that has the same vendor ID, but a different than requested product ID. Fixes broken dediprog detection with TOMU in use. Change-Id: I08c1c363ce2d6603e46efecc61d3910e02314fca Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/32892 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M usbdev.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/usbdev.c b/usbdev.c index d793b65..764ac04 100644 --- a/usbdev.c +++ b/usbdev.c @@ -54,7 +54,7 @@ continue; } - if ((desc.idVendor != vid) && (desc.idProduct != pid)) + if ((desc.idVendor != vid) || (desc.idProduct != pid)) continue; msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n", -- To view, visit https://review.coreboot.org/c/flashrom/+/32892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: I08c1c363ce2d6603e46efecc61d3910e02314fca Gerrit-Change-Number: 32892 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: merged

1 year, 5 months

1
0
0 0

2020

2019

2018

2017

flashrom-gerrit June 2019