December 2018 - flashrom-gerrit

Change in ...flashrom[1.0.x]: Fix verification with sparse layouts
by Nico Huber (Code Review) 22 Dec '18

22 Dec '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/30372 ) Change subject: Fix verification with sparse layouts ...................................................................... Patch Set 2: Code-Review-2 Assumes layout entries are ordered... -- To view, visit https://review.coreboot.org/c/flashrom/+/30372 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I44e0cea621f2a3d4dc70fa7e93c52ed95e54014a Gerrit-Change-Number: 30372 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 22 Dec 2018 01:07:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...flashrom[1.0.x]: Makefile: Disable `-Werror=deprecated-declarations` on release branch
by Nico Huber (Code Review) 22 Dec '18

22 Dec '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/30373 Change subject: Makefile: Disable `-Werror=deprecated-declarations` on release branch ...................................................................... Makefile: Disable `-Werror=deprecated-declarations` on release branch Change-Id: I6ed27b05b8b11f0ae1bcd331148cd61381edb8a0 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/30373/1 diff --git a/Makefile b/Makefile index 9cf52b2..7fcccd8 100644 --- a/Makefile +++ b/Makefile @@ -63,7 +63,7 @@ WARNERROR ?= yes ifeq ($(WARNERROR), yes) -CFLAGS += -Werror +CFLAGS += -Werror -Wno-error=deprecated-declarations endif ifdef LIBS_BASE -- To view, visit https://review.coreboot.org/c/flashrom/+/30373 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I6ed27b05b8b11f0ae1bcd331148cd61381edb8a0 Gerrit-Change-Number: 30373 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in ...flashrom[1.0.x]: Fix verification with sparse layouts
by Nico Huber (Code Review) 22 Dec '18

22 Dec '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/30372 Change subject: Fix verification with sparse layouts ...................................................................... Fix verification with sparse layouts The full verification step was not accounting for sparse layouts. Instead of the old contents, combine_image_by_layout() implicitly assumed the new contents for unspecified regions. Change-Id: I44e0cea621f2a3d4dc70fa7e93c52ed95e54014a Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flashrom.c 1 file changed, 11 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/30372/1 diff --git a/flashrom.c b/flashrom.c index c600efc..46b0b8b 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2307,17 +2307,24 @@ uint8_t *const newcontents, const uint8_t *const oldcontents) { const struct flashrom_layout *const layout = get_layout(flashctx); + chipoff_t next = 0; size_t i; for (i = 0; i < layout->num_entries; ++i) { - if (layout->entries[i].included) + if (!layout->entries[i].included) continue; - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + /* copy everything up to the start of this included region */ + const chipsize_t copy_len = layout->entries[i].start - next; + memcpy(newcontents + next, oldcontents + next, copy_len); - memcpy(newcontents + region_start, oldcontents + region_start, region_len); + /* skip this included region */ + next = layout->entries[i].end + 1; } + + /* copy the rest of the chip */ + const chipsize_t copy_len = flashctx->chip->total_size * 1024 - next; + memcpy(newcontents + next, oldcontents + next, copy_len); } /** -- To view, visit https://review.coreboot.org/c/flashrom/+/30372 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I44e0cea621f2a3d4dc70fa7e93c52ed95e54014a Gerrit-Change-Number: 30372 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in ...flashrom[1.0.x]: linux_spi: Hardcode default spispeed of 2MHz
by Nico Huber (Code Review) 22 Dec '18

22 Dec '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/30371 Change subject: linux_spi: Hardcode default spispeed of 2MHz ...................................................................... linux_spi: Hardcode default spispeed of 2MHz Leaving the `linux_spi` driver's unknown default is almost never what we want and resulted in many support requests since Raspbian switched to a default that is too high for most applications. Change-Id: I9361b7c1a1ab8900a619b06e1dae14cd87eb56c2 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M linux_spi.c 1 file changed, 13 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/30371/1 diff --git a/linux_spi.c b/linux_spi.c index d276402..a380477 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -22,6 +22,8 @@ #include <stdio.h> #include <string.h> #include <stdlib.h> +#include <stdint.h> +#include <inttypes.h> #include <fcntl.h> #include <errno.h> #include <ctype.h> @@ -67,7 +69,7 @@ int linux_spi_init(void) { char *p, *endp, *dev; - uint32_t speed_hz = 0; + uint32_t speed_hz = 2 * 1000 * 1000; /* FIXME: make the following configurable by CLI options. */ /* SPI mode 0 (beware this also includes: MSB first, CS active low and others */ const uint8_t mode = SPI_MODE_0; @@ -76,11 +78,15 @@ p = extract_programmer_param("spispeed"); if (p && strlen(p)) { speed_hz = (uint32_t)strtoul(p, &endp, 10) * 1000; - if (p == endp) { + if (p == endp || speed_hz == 0) { msg_perr("%s: invalid clock: %s kHz\n", __func__, p); free(p); return 1; } + } else { + msg_pinfo("Using default %"PRIu32 + "kHz clock. Use 'spispeed' parameter to override.\n", + speed_hz / 1000); } free(p); @@ -105,15 +111,12 @@ return 1; /* We rely on the shutdown function for cleanup from here on. */ - if (speed_hz > 0) { - if (ioctl(fd, SPI_IOC_WR_MAX_SPEED_HZ, &speed_hz) == -1) { - msg_perr("%s: failed to set speed to %d Hz: %s\n", - __func__, speed_hz, strerror(errno)); - return 1; - } - - msg_pdbg("Using %d kHz clock\n", speed_hz/1000); + if (ioctl(fd, SPI_IOC_WR_MAX_SPEED_HZ, &speed_hz) == -1) { + msg_perr("%s: failed to set speed to %"PRIu32"Hz: %s\n", + __func__, speed_hz, strerror(errno)); + return 1; } + msg_pdbg("Using %"PRIu32"kHz clock\n", speed_hz / 1000); if (ioctl(fd, SPI_IOC_WR_MODE, &mode) == -1) { msg_perr("%s: failed to set SPI mode to 0x%02x: %s\n", -- To view, visit https://review.coreboot.org/c/flashrom/+/30371 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I9361b7c1a1ab8900a619b06e1dae14cd87eb56c2 Gerrit-Change-Number: 30371 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in ...flashrom[master]: Fix verification with sparse layouts
by Nico Huber (Code Review) 22 Dec '18

22 Dec '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/30370 Change subject: Fix verification with sparse layouts ...................................................................... Fix verification with sparse layouts The full verification step was not accounting for sparse layouts. Instead of the old contents, combine_image_by_layout() implicitly assumed the new contents for unspecified regions. Change-Id: I44e0cea621f2a3d4dc70fa7e93c52ed95e54014a Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flashrom.c 1 file changed, 11 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/30370/1 diff --git a/flashrom.c b/flashrom.c index 59a7531..4e10629 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2355,17 +2355,24 @@ uint8_t *const newcontents, const uint8_t *const oldcontents) { const struct flashrom_layout *const layout = get_layout(flashctx); + chipoff_t next = 0; size_t i; for (i = 0; i < layout->num_entries; ++i) { - if (layout->entries[i].included) + if (!layout->entries[i].included) continue; - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + /* copy everything up to the start of this included region */ + const chipsize_t copy_len = layout->entries[i].start - next; + memcpy(newcontents + next, oldcontents + next, copy_len); - memcpy(newcontents + region_start, oldcontents + region_start, region_len); + /* skip this included region */ + next = layout->entries[i].end + 1; } + + /* copy the rest of the chip */ + const chipsize_t copy_len = flashctx->chip->total_size * 1024 - next; + memcpy(newcontents + next, oldcontents + next, copy_len); } /** -- To view, visit https://review.coreboot.org/c/flashrom/+/30370 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I44e0cea621f2a3d4dc70fa7e93c52ed95e54014a Gerrit-Change-Number: 30370 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in ...flashrom[master]: layout: Show a warning if no region is included
by Nico Huber (Code Review) 21 Dec '18

21 Dec '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/30369 Change subject: layout: Show a warning if no region is included ...................................................................... layout: Show a warning if no region is included This seems better than a plain success message for a (probably accidental) no-op run. Change-Id: I53b749ce42ecc6c267b6cbe71413d536ec3965c5 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M cli_classic.c M layout.c M layout.h 3 files changed, 16 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/30369/1 diff --git a/cli_classic.c b/cli_classic.c index df9fa67..924445a 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -645,6 +645,9 @@ goto out_shutdown; } + if (layout && layout_num_regions_included(layout) == 0) + msg_gwarn("Warning: Layout specified but no region included!\n"); + flashrom_layout_set(fill_flash, layout); flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, !!force); #if CONFIG_INTERNAL == 1 diff --git a/layout.c b/layout.c index fa66238..f0fbad8 100644 --- a/layout.c +++ b/layout.c @@ -191,6 +191,18 @@ return 0; } +unsigned int layout_num_regions_included(const struct flashrom_layout *const l) +{ + unsigned int i, count = 0; + + for (i = 0; i < l->num_entries; ++i) { + if (l->entries[i].included) + ++count; + } + + return count; +} + void layout_cleanup(void) { int i; diff --git a/layout.h b/layout.h index eb54a4f..5942038 100644 --- a/layout.h +++ b/layout.h @@ -58,5 +58,6 @@ const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx); int process_include_args(struct flashrom_layout *); +unsigned int layout_num_regions_included(const struct flashrom_layout *); #endif /* !__LAYOUT_H__ */ -- To view, visit https://review.coreboot.org/c/flashrom/+/30369 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I53b749ce42ecc6c267b6cbe71413d536ec3965c5 Gerrit-Change-Number: 30369 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in ...flashrom[master]: linux_spi: Hardcode default spispeed of 2MHz
by Nico Huber (Code Review) 21 Dec '18

21 Dec '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/30368 Change subject: linux_spi: Hardcode default spispeed of 2MHz ...................................................................... linux_spi: Hardcode default spispeed of 2MHz Leaving the `linux_spi` driver's unknown default is almost never what we want and resulted in many support requests since Raspbian switched to a default that is too high for most applications. Change-Id: I9361b7c1a1ab8900a619b06e1dae14cd87eb56c2 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M linux_spi.c 1 file changed, 13 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/30368/1 diff --git a/linux_spi.c b/linux_spi.c index 3e60492..711ab4a 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -18,6 +18,8 @@ #include <stdio.h> #include <string.h> #include <stdlib.h> +#include <stdint.h> +#include <inttypes.h> #include <fcntl.h> #include <errno.h> #include <ctype.h> @@ -66,7 +68,7 @@ int linux_spi_init(void) { char *p, *endp, *dev; - uint32_t speed_hz = 0; + uint32_t speed_hz = 2 * 1000 * 1000; /* FIXME: make the following configurable by CLI options. */ /* SPI mode 0 (beware this also includes: MSB first, CS active low and others */ const uint8_t mode = SPI_MODE_0; @@ -75,11 +77,15 @@ p = extract_programmer_param("spispeed"); if (p && strlen(p)) { speed_hz = (uint32_t)strtoul(p, &endp, 10) * 1000; - if (p == endp) { + if (p == endp || speed_hz == 0) { msg_perr("%s: invalid clock: %s kHz\n", __func__, p); free(p); return 1; } + } else { + msg_pinfo("Using default %"PRIu32 + "kHz clock. Use 'spispeed' parameter to override.\n", + speed_hz / 1000); } free(p); @@ -104,15 +110,12 @@ return 1; /* We rely on the shutdown function for cleanup from here on. */ - if (speed_hz > 0) { - if (ioctl(fd, SPI_IOC_WR_MAX_SPEED_HZ, &speed_hz) == -1) { - msg_perr("%s: failed to set speed to %d Hz: %s\n", - __func__, speed_hz, strerror(errno)); - return 1; - } - - msg_pdbg("Using %d kHz clock\n", speed_hz/1000); + if (ioctl(fd, SPI_IOC_WR_MAX_SPEED_HZ, &speed_hz) == -1) { + msg_perr("%s: failed to set speed to %"PRIu32"Hz: %s\n", + __func__, speed_hz, strerror(errno)); + return 1; } + msg_pdbg("Using %"PRIu32"kHz clock\n", speed_hz / 1000); if (ioctl(fd, SPI_IOC_WR_MODE, &mode) == -1) { msg_perr("%s: failed to set SPI mode to 0x%02x: %s\n", -- To view, visit https://review.coreboot.org/c/flashrom/+/30368 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9361b7c1a1ab8900a619b06e1dae14cd87eb56c2 Gerrit-Change-Number: 30368 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in ...flashrom[master]: libflashrom.h: Add missing includes
by Nico Huber (Code Review) 21 Dec '18

21 Dec '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/30153 ) Change subject: libflashrom.h: Add missing includes ...................................................................... libflashrom.h: Add missing includes <stddef.h> for `size_t` and <sys/types.h> for `off_t`. Change-Id: Ifc84dfe2a06633321d0abd364bdea1216925a779 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/30153 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M libflashrom.h 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved diff --git a/libflashrom.h b/libflashrom.h index 4fbcd35..38c95d2 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -18,6 +18,8 @@ #ifndef __LIBFLASHROM_H__ #define __LIBFLASHROM_H__ 1 +#include <sys/types.h> +#include <stddef.h> #include <stdarg.h> int flashrom_init(int perform_selfcheck); -- To view, visit https://review.coreboot.org/c/flashrom/+/30153 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifc84dfe2a06633321d0abd364bdea1216925a779 Gerrit-Change-Number: 30153 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in ...flashrom[master]: fmap: Fix length calculation in error message
by Nico Huber (Code Review) 21 Dec '18

21 Dec '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/30152 ) Change subject: fmap: Fix length calculation in error message ...................................................................... fmap: Fix length calculation in error message Change-Id: Ie0f448970de6a7829f304448e0835eaeb7d103a3 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/30152 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M fmap.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved diff --git a/fmap.c b/fmap.c index d44b7fa..84b3b54 100644 --- a/fmap.c +++ b/fmap.c @@ -243,7 +243,7 @@ if (flashctx->chip->read(flashctx, (uint8_t *)fmap + sig_len, offset + sig_len, sizeof(*fmap) - sig_len)) { msg_cerr("Cannot read %zu bytes at offset %06zx\n", - sizeof(*fmap) + sig_len, offset + sig_len); + sizeof(*fmap) - sig_len, offset + sig_len); continue; } -- To view, visit https://review.coreboot.org/c/flashrom/+/30152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie0f448970de6a7829f304448e0835eaeb7d103a3 Gerrit-Change-Number: 30152 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in ...flashrom[master]: chipset_enable.c: Mark Intel C224 as DEP
by Angel Pons (Code Review) 21 Dec '18

21 Dec '18

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/30361 ) Change subject: chipset_enable.c: Mark Intel C224 as DEP ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/30361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I25126b94e691289a7b29dd81d5c864854a4e0245 Gerrit-Change-Number: 30361 Gerrit-PatchSet: 1 Gerrit-Owner: Tristan Corrick <tristan(a)corrick.kiwi> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tristan Corrick <tristan(a)corrick.kiwi> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 21 Dec 2018 11:46:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2018