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

Change in flashrom[staging]: hwaccess: Constify `addr` argument of MMIO read functions
by Philippe Mathieu-Daudé (Code Review) 20 Apr '17

20 Apr '17

Philippe Mathieu-Daudé has posted comments on this change. ( https://review.coreboot.org/18881 ) Change subject: hwaccess: Constify `addr` argument of MMIO read functions ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/18881 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1419241a8332b74103b3921df2c615bdf91346a7 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Move register decodes into enable_flash_ich_handle_gcs()
by build bot (Jenkins) (Code Review) 19 Apr '17

19 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/18882 ) Change subject: Move register decodes into enable_flash_ich_handle_gcs() ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/156/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/172/ : SUCCESS -- To view, visit https://review.coreboot.org/18882 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40addec98cb6840763adad30f9d0e27dadce6d1e Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: hwaccess: Constify `addr` argument of MMIO read functions
by build bot (Jenkins) (Code Review) 19 Apr '17

19 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/18881 ) Change subject: hwaccess: Constify `addr` argument of MMIO read functions ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/155/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/171/ : SUCCESS -- To view, visit https://review.coreboot.org/18881 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1419241a8332b74103b3921df2c615bdf91346a7 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: layout: Add -i <region>[:<file>] support.
by Nico Huber (Code Review) 18 Apr '17

18 Apr '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19342 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 2: (10 comments) https://review.coreboot.org/#/c/19342/2/flashrom.8.tmpl File flashrom.8.tmpl: PS2, Line 147: <region>[:<file>] This seems wrong. Bad merge? PS2, Line 292: .TP : flashrom supports reading/writing/erasing/verifying flash chips in whole (default) or in part. To access only \ : parts of a chip one has to use layout files and respective arguments described below. : .TP : .B "\-l, \-\-layout <layoutfile>" : Read layout entries from : .BR <layoutfile> . : .sp : Each layout entry describes an address region of the flash chip and gives it a name (hereinafter referred to as : a region). One entry per line is allowed with the following syntax: : .sp : .B " startaddr:endaddr regionname" : .sp : .BR "startaddr " "and " "endaddr " : are hexadecimal addresses within the ROM image representing the flash ROM contents and do not refer to any : physical address. Please note that using a 0x prefix for those hexadecimal numbers is not necessary, but you : can't specify decimal/octal numbers. : .BR "regionname " "is the name for the region from " "startaddr " "to " "endaddr " "(both addresses included)." : Spaces, tabs, newlines, colons, single or double quotes are not allowed in region names. : .sp : Example content of file rom.layout: : .sp : 0x00000000:0x00008fff gfx_rom : 0x00009000:0x0003ffff normal : 0x00040000:0x0007ffff fallback : .sp : .TP : .B "\-i, \-\-include <name>[:<regionfile>]" : Work on the flash region : .B name : instead of the full address space if a layout file is given and parsed correctly. : Multiple such include parameters can be used to work on the union of different regions. : .sp : The optional : .B regionfile : parameter specifies the name of a file that is used to map the contents of that very region only. : The optional file parameter causes the contents of this region to be replaced by the contents of the file : specified here. : .sp : If you only want to update the regions named : .BR "normal " "and " "gfx_rom " "in a ROM based on the layout mentioned above, run" : .sp : .B " flashrom \-p prog \-l rom.layout \-i normal -i "gfx_rom" \-w some.rom" : .sp : Overlapping regions are resolved in an implementation-dependent manner (or may even yield an error) - do : .BR "not " "rely on it." : .sp Should be merged with the old description above. Probably just a bad merge. https://review.coreboot.org/#/c/19342/2/flashrom.c File flashrom.c: PS2, Line 1308: msg_gerr("Error: Image size (%jd B) doesn't match the flash chip's size (%lu B)!\n", : (intmax_t)image_stat.st_size, size); remove Line 1353: IMHO, not having an empty line here was intentional. Line 1392: static const struct fl_layout *get_layout(const struct flashctx *const flashctx); If we make it the standard, the declaration should probably move to `layout.h`. Line 1404: * @param size Size of chip-sized buffer Now that we pass the flashctx, the size argument is redundant. We should either drop it, or bail out if they don't match. Line 1407: * 1 if all available erase functions failed. ahem PS2, Line 1427: * *const ? to match the other "variables" Line 1444: double blank line? https://review.coreboot.org/#/c/19342/2/layout.h File layout.h: PS2, Line 43: char `const char`? we'd never want to change it, would we? If we're going to have something dynamically allocated here, we should also reevaluate how `name` is handled. -- To view, visit https://review.coreboot.org/19342 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(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) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Add functions to read/erase/write/verify by layout
by Nico Huber (Code Review) 18 Apr '17

18 Apr '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/17945 ) Change subject: Add functions to read/erase/write/verify by layout ...................................................................... Patch Set 6: Also tested it again (I've pushed the simple script I used). I let it run through with a build of the head of this series, plus once with the 4KiB eraser removed from flashchips.c and once with a no-op spi_block_erase_20() to test the fallback. -- To view, visit https://review.coreboot.org/17945 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6194cea4c4c430e0cf9d586052508a865b09c86 Gerrit-PatchSet: 6 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Add functions to read/erase/write/verify by layout
by Nico Huber (Code Review) 18 Apr '17

18 Apr '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/17945 ) Change subject: Add functions to read/erase/write/verify by layout ...................................................................... Patch Set 6: (3 comments) I think I'm done here :) https://review.coreboot.org/#/c/17945/4/flashrom.c File flashrom.c: PS4, Line 1764: size_t > int to match num_entries? I've adapted `num_entries` instead. I'm not a fan of signed integers for index variables and got used to `size_t` (albeit I know that it's technically wrong, not a size; but that's how many people use it, even some POSIX functions). Line 1855: > nit: add a newline above to separate the declarations and the body Done PS4, Line 1886: info->erase_start; /* within th > Thanks for updating the offset calculation. However, it somehow didn't end Done -- To view, visit https://review.coreboot.org/17945 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6194cea4c4c430e0cf9d586052508a865b09c86 Gerrit-PatchSet: 6 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Simple, clumsy script to test layout handling
by Nico Huber (Code Review) 18 Apr '17

18 Apr '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19352 ) Change subject: Simple, clumsy script to test layout handling ...................................................................... Patch Set 1: Code-Review-2 Just a hacky script until we have better test infrastructure. -- To view, visit https://review.coreboot.org/19352 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I37124455071bf3395384a58ad236c998f738c418 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Simple, clumsy script to test layout handling
by build bot (Jenkins) (Code Review) 18 Apr '17

18 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19352 ) Change subject: Simple, clumsy script to test layout handling ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/154/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/170/ : SUCCESS -- To view, visit https://review.coreboot.org/19352 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I37124455071bf3395384a58ad236c998f738c418 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Simple, clumsy script to test layout handling
by Nico Huber (Code Review) 18 Apr '17

18 Apr '17

Nico Huber has uploaded a new change for review. ( https://review.coreboot.org/19352 ) Change subject: Simple, clumsy script to test layout handling ...................................................................... Simple, clumsy script to test layout handling Not for merge. First parameter are flashrom options, second an optional layout file (if not provided, `--ifd` will be used). e.g. sh util/test_layout "-p dummy:emulate=MX25L6436,image=/tmp/dummy.rom -c MX25L6436E/MX25L6445E/MX25L6465E/MX25L6473E" util/test.layout Change-Id: I37124455071bf3395384a58ad236c998f738c418 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- A util/test.layout A util/test_layout 2 files changed, 48 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/19352/1 diff --git a/util/test.layout b/util/test.layout new file mode 100644 index 0000000..3139f05 --- /dev/null +++ b/util/test.layout @@ -0,0 +1,4 @@ +00000000:00000fff Flash_Descriptor +00001000:00002fff GbE +00003000:0057ffef Intel_ME +0057fff0:007fffff BIOS diff --git a/util/test_layout b/util/test_layout new file mode 100644 index 0000000..d328bcc --- /dev/null +++ b/util/test_layout @@ -0,0 +1,44 @@ +#!/bin/sh + +set -e + +TMP=$(mktemp -d --tmpdir layout-test.XXXXXX) +echo ${TMP} + +FLASHROM_ARGS="$1" +if [ $# -ge 2 ]; then + LAYOUT_ARGS="${FLASHROM_ARGS} --layout $2" +else + LAYOUT_ARGS="${FLASHROM_ARGS} --ifd" +fi + +flashrom() { + echo flashrom $* + ./flashrom -V ${LAYOUT_ARGS} $* 2>&1 | tee -a ${TMP}/layout.log | grep VERIFIED +} + +./flashrom ${FLASHROM_ARGS} -r ${TMP}/backup.rom +dd if=/dev/urandom of=${TMP}/random.rom bs=$(stat -c%s ${TMP}/backup.rom) count=1 + +test_layout() { + all_regions="Flash_Descriptor GbE Intel_ME BIOS" + included="$*" + + for i in $included; do + flashrom -i $i -A -w ${TMP}/random.rom + done + for i in $all_regions; do + if echo $included | grep -q "\<${i}\>"; then + f=${TMP}/random.rom + else + f=${TMP}/backup.rom + fi + flashrom -i $i -v $f + done + ./flashrom ${FLASHROM_ARGS} -w ${TMP}/backup.rom +} + +test_layout GbE +test_layout Intel_ME +test_layout GbE BIOS +test_layout GbE Intel_ME -- To view, visit https://review.coreboot.org/19352 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I37124455071bf3395384a58ad236c998f738c418 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[staging]: dediprog: Fix bug where too many transfers would be queued
by Nico Huber (Code Review) 18 Apr '17

18 Apr '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19351 ) Change subject: dediprog: Fix bug where too many transfers would be queued ...................................................................... Patch Set 1: Code-Review+2 Already acked on ML. -- To view, visit https://review.coreboot.org/19351 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I91a8de47db7107455f5fc63ab2f13a0bd50c5b63 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2017