January 2018 - flashrom-gerrit

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Arthur Heymans (Code Review) 09 Jan '18

09 Jan '18

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 7: (3 comments) https://review.coreboot.org/#/c/23021/5/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23021/5/flashrom.c@1387 PS5, Line 1387: } > For IFD layouts, it's taken care of. For layout files, idk. being taken care of by normalize_romentries() later. so ok to drop here. https://review.coreboot.org/#/c/23021/5/layout.c File layout.c: https://review.coreboot.org/#/c/23021/5/layout.c@189 PS5, Line 189: if (idx < 0) { > What is sane? Regarding IFD region names, those are hardcoded, and always […] read_romlayout bails out if there are spaces so there is no need for this. same holds for empty strings. https://review.coreboot.org/#/c/23021/7/layout.c File layout.c: https://review.coreboot.org/#/c/23021/7/layout.c@206 PS7, Line 206: file = strdup(file); otherwise layout_cleanup calls free(layout.entries[i].file) on an invalid pointer. -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 7 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 09 Jan 2018 22:49:19 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Arthur Heymans (Code Review) 09 Jan '18

09 Jan '18

Arthur Heymans has uploaded a new patch set (#7) to the change originally created by David Hendricks. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... layout: Add -i <region>[:<file>] support. Add an optional sub-parameter to the -i parameter to allow building the image to be written from multiple files. This will also allow regions to be read from flash and written to separate image files. This is a rebase of a patch that was ported from chromiumos. A lot of things have changed, but the idea is the same. Original patch by Louis Yung-Chieh Lo <yjlou(a)chromium.org>: Summary: Support -i partition:file feature for both read and write. Commit: 9c7525f Review URL: http://codereview.chromium.org/6611015 Ported version by Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> and Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>: Summary: [PATCH 2/6] layout: Add -i <region>[:<file>] support. Review URL: https://mail.coreboot.org/pipermail/flashrom/2013-October/011729.html Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Signed-off-by: David Hendricks <dhendrix(a)chromium.org> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M cli_classic.c M dummyflasher.c M flash.h M flashrom.8.tmpl M flashrom.c M layout.c M layout.h 7 files changed, 259 insertions(+), 41 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/23021/7 -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 7 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: buspirate_spi: Add support for variable serial speeds
by Nico Huber (Code Review) 08 Jan '18

08 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23057 ) Change subject: buspirate_spi: Add support for variable serial speeds ...................................................................... Patch Set 3: > My reasoning is that the Bus Pirate itself offers options for > different baud rates, including many of the values included in the > table. I just thought it best to expose as much of the Bus Pirate's > feature set as possible and let the user choose between them. I don't see a reason to offer values that make no sense to use with flashrom (anything below 115200). > The > extra values in my table are intermediate values that the Bus > Pirate's menu doesn't offer by default but are still slower than > the max of 2M. Also, the table omits values too slow to be usable > (<9600 baud) for this purpose. Values between 115200 and 2M might be useful if there are pre-v3 pirates that support the baud setting. Though, that's pure specu- lation so far if such a thing exists, right? I'd like to see everything below 115200 removed (the only effect I'd expect from these rates would be confusing humble users). For the rates between 115200 and 2M, I'm still not fully convinced. If you really want to keep the serialspeed option, the man page would have to be updated as well. -- To view, visit https://review.coreboot.org/23057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3706f17a94fdf056063f2ad4a5f0a219665cdcbf Gerrit-Change-Number: 23057 Gerrit-PatchSet: 3 Gerrit-Owner: Shawn Anastasio <shawnanastasio(a)yahoo.com> 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: Shawn Anastasio <shawnanastasio(a)yahoo.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 22:44:10 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: make args for -r/-w/-v non-positional and optional
by build bot (Jenkins) (Code Review) 08 Jan '18

08 Jan '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23022 ) Change subject: make args for -r/-w/-v non-positional and optional ...................................................................... Patch Set 5: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/866/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/1017/ : SUCCESS -- To view, visit https://review.coreboot.org/23022 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefbcb7dc4fefe26f5afd1292dfd5c1687fa62803 Gerrit-Change-Number: 23022 Gerrit-PatchSet: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Louis Yung-Chieh Lo <yjlou(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 21:26:31 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by build bot (Jenkins) (Code Review) 08 Jan '18

08 Jan '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 6: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/865/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/1016/ : SUCCESS -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 6 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 21:26:26 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Arthur Heymans (Code Review) 08 Jan '18

08 Jan '18

Arthur Heymans has uploaded a new patch set (#6) to the change originally created by David Hendricks. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... layout: Add -i <region>[:<file>] support. Add an optional sub-parameter to the -i parameter to allow building the image to be written from multiple files. This will also allow regions to be read from flash and written to separate image files. This is a rebase of a patch that was ported from chromiumos. A lot of things have changed, but the idea is the same. Original patch by Louis Yung-Chieh Lo <yjlou(a)chromium.org>: Summary: Support -i partition:file feature for both read and write. Commit: 9c7525f Review URL: http://codereview.chromium.org/6611015 Ported version by Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> and Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>: Summary: [PATCH 2/6] layout: Add -i <region>[:<file>] support. Review URL: https://mail.coreboot.org/pipermail/flashrom/2013-October/011729.html Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Signed-off-by: David Hendricks <dhendrix(a)chromium.org> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M cli_classic.c M dummyflasher.c M flash.h M flashrom.8.tmpl M flashrom.c M layout.c M layout.h 7 files changed, 281 insertions(+), 41 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/23021/6 -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 6 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Arthur Heymans (Code Review) 08 Jan '18

08 Jan '18

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 5: (1 comment) https://review.coreboot.org/#/c/23021/5/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23021/5/flashrom.c@1379 PS5, Line 1379: } > I'm a bit confused here. Do we allow such layouts at all? Will […] I could think of a future use that might be useful if/once fmap support is implemented: if you use a layout that reflects fmap stuff will overlap because subsections... This would prevent such use case. What should be done is to make sure that the include args regions don't overlap. -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 21:25:43 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: board_enable: Wyse Cx0 thin client is not a laptop
by Nico Huber (Code Review) 08 Jan '18

08 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23157 ) Change subject: board_enable: Wyse Cx0 thin client is not a laptop ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/#/c/23157/2/board_enable.c File board_enable.c: https://review.coreboot.org/#/c/23157/2/board_enable.c@2474 PS2, Line 2474: 0x1106, 0x0409, 0x6334, 0x0920, 0x1106, 0x3119, 0x1106, 0x0110, "^C CLASS$" How confident are you that this combination isn't found in other devices? Especially the "C CLASS" string makes me wonder, is it a place holder that somebody forgot to fill? or related to "Cx0"? -- To view, visit https://review.coreboot.org/23157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icc9d48c61680f2dc6818b4dc7e1e721a97f0c0bb Gerrit-Change-Number: 23157 Gerrit-PatchSet: 2 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> 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, 08 Jan 2018 16:27:45 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: chipset_enable: Mark VX855 as tested
by Nico Huber (Code Review) 08 Jan '18

08 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23158 ) Change subject: chipset_enable: Mark VX855 as tested ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/23158 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f48b49ccb760f69d676ec6cbb233e532b12fbe8 Gerrit-Change-Number: 23158 Gerrit-PatchSet: 1 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> 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, 08 Jan 2018 16:24:17 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

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

08 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 5: (2 comments) Generally, I think we should always assume a "sane" layout in the internal code to keep it simple (e.g. in flashrom.c). Regions with start above end should be filtered (warning) / denied (bail out) when reading the layout. Regions partially out of the flash chip's bounds should be sanitized (warning) / denied (bail out). This should be implemented in a follow-up if necessary. Regarding overlapping regions: I don't see a use case. But if we want to allow them in the layout file, I'm ok with the current solution implemented here. https://review.coreboot.org/#/c/23021/5/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23021/5/flashrom.c@1387 PS5, Line 1387: if ((entry->start > size) || (entry->end > size)) { > is entry->start always smaller than entry->end? afaik coreboot/util/ifdtool generates regions where […] For IFD layouts, it's taken care of. For layout files, idk. https://review.coreboot.org/#/c/23021/5/layout.c File layout.c: https://review.coreboot.org/#/c/23021/5/layout.c@189 PS5, Line 189: /* Quotes or whitespace are not allowed in region names. */ > I suppose this could go in the read_romlayout() function for when a layout file is provided. […] What is sane? Regarding IFD region names, those are hardcoded, and always sane. I don't see what we gain from this check here, if somebody wants to name his region "foo bar" or even "\"foo bar\"", why would it hurt? And if any- where, I'd filter the names when we read the layout file. Then, weird names in a -i argument would just not be found (but why add extra code???). What do I miss? -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 13:43:02 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2018