- flashrom-gerrit - mail.coreboot.org

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

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: (2 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: if ((entry->start > size) || (entry->end > size)) { > Checking `entry->end` should suffice. is entry->start always smaller than entry->end? afaik coreboot/util/ifdtool generates regions where this is not the case for unused regions... 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. */ > why test it here? why add it now? I suppose this could go in the read_romlayout() function for when a layout file is provided. when fetched with --ifd, " } else if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) || process_include_args(layout))) { " one can expect the layout to be sane right? -- 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 11:13:52 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: board_enable: Wyse Cx0 thin client is not a laptop
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/23157 ) Change subject: board_enable: Wyse Cx0 thin client is not a laptop ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1015/ : SUCCESS -- 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 10:35:22 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

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

08 Jan '18

Hello Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/23157 to look at the new patch set (#2). Change subject: board_enable: Wyse Cx0 thin client is not a laptop ...................................................................... board_enable: Wyse Cx0 thin client is not a laptop Add it to the whitelist. The subsystem IDs on the machine look completely random. Hopefully they're stable across the boards. The PCI gigabit ethernet is external to the chipset. Change-Id: Icc9d48c61680f2dc6818b4dc7e1e721a97f0c0bb Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk> --- M board_enable.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/23157/2 -- 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: newpatchset Gerrit-Change-Id: Icc9d48c61680f2dc6818b4dc7e1e721a97f0c0bb Gerrit-Change-Number: 23157 Gerrit-PatchSet: 2 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> 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]: board_enable: Wyse Cx0 thin client is not a laptop
by Paul Menzel (Code Review) 08 Jan '18

08 Jan '18

Paul Menzel 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 1: (1 comment) https://review.coreboot.org/#/c/23157/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23157/1//COMMIT_MSG@11 PS1, Line 11: no on -- 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: 1 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> 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 10:03:28 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

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

08 Jan '18

Paul Menzel 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 1: Code-Review+1 -- 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: 1 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> 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 10:01:34 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

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

08 Jan '18

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/23158 ) Change subject: chipset_enable: Mark VX855 as tested ...................................................................... Patch Set 1: Code-Review+1 -- 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 10:01:27 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit