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

Change in flashrom[master]: ch341a_spi.c: Reset device before use
by Angel Pons (Code Review) April 3, 2022

April 3, 2022

Attention is currently required from: Nico Huber, Thomas Heijligen, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63258 ) Change subject: ch341a_spi.c: Reset device before use ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63258 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff824c22dc871bb2d9504abe8b5ad4e4c191c4af Gerrit-Change-Number: 63258 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sun, 03 Apr 2022 21:14:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: .gitignore: Avoid committing libflashrom.a
by Thomas Heijligen (Code Review) April 3, 2022

April 3, 2022

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63320 ) Change subject: .gitignore: Avoid committing libflashrom.a ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: duplicate of CB:62826 This patch adds all *.a to gitignore, not only libflashrom.a which is mentioned in the commit message -- To view, visit https://review.coreboot.org/c/flashrom/+/63320 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie9ca7c6ab846d0b07c16798cdd3ff5aa0a748a9b Gerrit-Change-Number: 63320 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 03 Apr 2022 12:43:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptor.c: Simplify get_density lookups by deduplicate sw
by Edward O'Callaghan (Code Review) April 3, 2022

April 3, 2022

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63321 ) Change subject: ich_descriptor.c: Simplify get_density lookups by deduplicate sw ...................................................................... ich_descriptor.c: Simplify get_density lookups by deduplicate sw These two switch statements look up the same thing with only the return type as different. Therefore deduce out the density encoding lookup from the use-case logic. TEST=`flashrom -p internal --flash-name` on Intel Kaby Lake U w/ iHDCP2.2 Prem. Change-Id: Ieb4e9f19f4f06becd344fe08829dabe20c3c8de0 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M ich_descriptors.c 1 file changed, 50 insertions(+), 79 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/63321/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 7c03499..ec8546c 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -161,26 +161,17 @@ msg_pdbg2("\n"); } -static const char *pprint_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx) +static int get_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx, uint8_t *size_enc, uint8_t *size_max) { + *size_enc = 0; + *size_max = 0; + if (idx > 1) { msg_perr("Only ICH SPI component index 0 or 1 are supported yet.\n"); - return NULL; + return -2; } - if (desc->content.NC == 0 && idx > 0) - return "unused"; - - static const char * const size_str[] = { - "512 kB", /* 0000 */ - "1 MB", /* 0001 */ - "2 MB", /* 0010 */ - "4 MB", /* 0011 */ - "8 MB", /* 0100 */ - "16 MB", /* 0101 */ /* Maximum up to Lynx Point (excl.) */ - "32 MB", /* 0110 */ - "64 MB", /* 0111 */ - }; + return 0; switch (cs) { case CHIPSET_ICH8: @@ -190,15 +181,13 @@ case CHIPSET_6_SERIES_COUGAR_POINT: case CHIPSET_7_SERIES_PANTHER_POINT: case CHIPSET_BAYTRAIL: { - uint8_t size_enc; if (idx == 0) { - size_enc = desc->component.dens_old.comp1_density; + *size_enc = desc->component.dens_old.comp1_density; } else { - size_enc = desc->component.dens_old.comp2_density; + *size_enc = desc->component.dens_old.comp2_density; } - if (size_enc > 5) - return "reserved"; - return size_str[size_enc]; + *size_max = 5; + break; } case CHIPSET_8_SERIES_LYNX_POINT: case CHIPSET_8_SERIES_LYNX_POINT_LP: @@ -215,20 +204,48 @@ case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: { - uint8_t size_enc; if (idx == 0) { - size_enc = desc->component.dens_new.comp1_density; + *size_enc = desc->component.dens_new.comp1_density; } else { - size_enc = desc->component.dens_new.comp2_density; + *size_enc = desc->component.dens_new.comp2_density; } - if (size_enc > 7) - return "reserved"; - return size_str[size_enc]; + *size_max = 7; + break; } case CHIPSET_ICH_UNKNOWN: default: - return "unknown"; + msg_pwarn("Density encoding is unknown on this chipset.\n"); + return -1; } + return 1; +} + +static const char *pprint_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx) +{ + static const char * const size_str[] = { + "512 kB", /* 0000 */ + "1 MB", /* 0001 */ + "2 MB", /* 0010 */ + "4 MB", /* 0011 */ + "8 MB", /* 0100 */ + "16 MB", /* 0101 */ /* Maximum up to Lynx Point (excl.) */ + "32 MB", /* 0110 */ + "64 MB", /* 0111 */ + }; + + uint8_t size_enc; + uint8_t size_max; + + int ret = get_density(cs, desc, idx, &size_enc, &size_max); + if (ret == -2) + return NULL; + else if (ret == -1) + return "unknown"; + else if (ret == 0) + return "unused"; + else if (size_enc > size_max) + return "reserved"; + return size_str[size_enc]; } static const char *pprint_freq(enum ich_chipset cs, uint8_t value) @@ -1194,61 +1211,15 @@ \em idx in bytes or -1 if the correct size can not be determined. */ int getFCBA_component_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx) { - if (idx > 1) { - msg_perr("Only ICH SPI component index 0 or 1 are supported yet.\n"); - return -1; - } - - if (desc->content.NC == 0 && idx > 0) - return 0; - uint8_t size_enc; uint8_t size_max; - switch (cs) { - case CHIPSET_ICH8: - case CHIPSET_ICH9: - case CHIPSET_ICH10: - case CHIPSET_5_SERIES_IBEX_PEAK: - case CHIPSET_6_SERIES_COUGAR_POINT: - case CHIPSET_7_SERIES_PANTHER_POINT: - case CHIPSET_BAYTRAIL: - if (idx == 0) { - size_enc = desc->component.dens_old.comp1_density; - } else { - size_enc = desc->component.dens_old.comp2_density; - } - size_max = 5; - break; - case CHIPSET_8_SERIES_LYNX_POINT: - case CHIPSET_8_SERIES_LYNX_POINT_LP: - case CHIPSET_8_SERIES_WELLSBURG: - case CHIPSET_9_SERIES_WILDCAT_POINT: - case CHIPSET_9_SERIES_WILDCAT_POINT_LP: - case CHIPSET_100_SERIES_SUNRISE_POINT: - case CHIPSET_C620_SERIES_LEWISBURG: - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_500_SERIES_TIGER_POINT: - case CHIPSET_600_SERIES_ALDER_POINT: - case CHIPSET_METEOR_LAKE: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - case CHIPSET_ELKHART_LAKE: - if (idx == 0) { - size_enc = desc->component.dens_new.comp1_density; - } else { - size_enc = desc->component.dens_new.comp2_density; - } - size_max = 7; - break; - case CHIPSET_ICH_UNKNOWN: - default: - msg_pwarn("Density encoding is unknown on this chipset.\n"); + int ret = get_density(cs, desc, idx, &size_enc, &size_max); + if (ret < 0) return -1; - } - - if (size_enc > size_max) { + else if (ret == 0) + return 0; + else if (size_enc > size_max) { msg_perr("Density of ICH SPI component with index %d is invalid.\n" "Encoded density is 0x%x while maximum allowed is 0x%x.\n", idx, size_enc, size_max); -- To view, visit https://review.coreboot.org/c/flashrom/+/63321 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieb4e9f19f4f06becd344fe08829dabe20c3c8de0 Gerrit-Change-Number: 63321 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: .gitignore: Avoid committing libflashrom.a
by Edward O'Callaghan (Code Review) April 3, 2022

April 3, 2022

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63320 ) Change subject: .gitignore: Avoid committing libflashrom.a ...................................................................... .gitignore: Avoid committing libflashrom.a Change-Id: Ie9ca7c6ab846d0b07c16798cdd3ff5aa0a748a9b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M .gitignore 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/63320/1 diff --git a/.gitignore b/.gitignore index 7c45aba..15cc77a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +*.a *.d !Makefile.d/ *.o -- To view, visit https://review.coreboot.org/c/flashrom/+/63320 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie9ca7c6ab846d0b07c16798cdd3ff5aa0a748a9b Gerrit-Change-Number: 63320 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: pony_spi.c: Extract out get_params to simplify init
by Angel Pons (Code Review) April 3, 2022

April 3, 2022

Attention is currently required from: Nico Huber, Edward O'Callaghan, Light, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63130 ) Change subject: pony_spi.c: Extract out get_params to simplify init ...................................................................... Patch Set 3: (7 comments) File pony_spi.c: https://review.coreboot.org/c/flashrom/+/63130/comment/8833d157_f0edb292 PS3, Line 153: { nit: add space before opening brace https://review.coreboot.org/c/flashrom/+/63130/comment/10aeaede_615bf794 PS3, Line 167: bool have_device; > From my point of view: Review doesn't scale well with the size of > a patch. Even if it's a very simple patch hunk, every additional > hunk can distract from the error-prone part of a patch. Doing fixes > and simple changes like this in advance works well, FWIW. +1 For me, it's very easy to miss something when reviewing commits that do multiple things at once. I'd rather review multiple commits where each does one and only one thing. Whenever I want/need to make significant code changes, I draft in advance a "plan of action". I use several criteria to segment the changes into commits, one of them is "at most one thing/reason/idea per commit". In this case, "simplify `pony_spi` init logic" and "use `bool` type for boolean variables" are two different things, and they are conceptually independent: disregarding merge conflicts, it's possible to do them in either order, or only do one of them. When accounting for merge conflicts, I start with the easiest (most likely to be approved) commits, so that I don't have to manually rebase them if another commit needs to be fixed. That being said, there's no need to over-split things. For example, I'd retype both `have_device` and `have_prog` in the same commit, because the rationale is the same. TL;DR: Please retype the variables in a separate commit (preferably before the "extract `get_params`" commit to streamline the submission process). https://review.coreboot.org/c/flashrom/+/63130/comment/523207c5_d0f3c3b6 PS3, Line 171: return 1; If the `dev` parameter is correct (i.e. `have_device` is true) but `get_params()` returns -1 because of an error with the `type` parameter, the serial port (what `serialport_shutdown()` releases) will leak. https://review.coreboot.org/c/flashrom/+/63130/comment/3b064e8c_49fe92dc PS3, Line 181: return 1; Serial port can also leak here. https://review.coreboot.org/c/flashrom/+/63130/comment/229d3eda_23804449 PS3, Line 190: serialport_shutdown > I think you are mis-interpreting the diff presented by gerrit Anastasia. This line is OK here, but it's missing in some other return paths. https://review.coreboot.org/c/flashrom/+/63130/comment/54be3b14_cde22584 PS3, Line 232: true This is unrelated. Please do it in a separate commit. https://review.coreboot.org/c/flashrom/+/63130/comment/9797c619_2ea34563 PS3, Line 237: true Ditto -- To view, visit https://review.coreboot.org/c/flashrom/+/63130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac Gerrit-Change-Number: 63130 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 03 Apr 2022 11:26:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pony_spi.c: Extract out get_params to simplify init
by Paul Menzel (Code Review) April 2, 2022

April 2, 2022

Attention is currently required from: Edward O'Callaghan, Light, Anastasia Klimchuk. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63130 ) Change subject: pony_spi.c: Extract out get_params to simplify init ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/63130/comment/77310740_a9f5f7d3 PS3, Line 9: `commit caa0335114a81` Nit: No markup needed. Maybe even add the commit message summary in () after it. -- To view, visit https://review.coreboot.org/c/flashrom/+/63130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac Gerrit-Change-Number: 63130 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 03 Apr 2022 05:43:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) April 2, 2022

April 2, 2022

Attention is currently required from: Nico Huber, Subrata Banik, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 10: (2 comments) Patchset: PS4: > > Unresolving then because it does look like a pch300 although I am not 100% on every tiny idiosyncr […] You are right `NM` is confusing, can we comment it better? Possibly copy what you just said there near verbatim into the `ich_number_of_masters()` function? OK, I picked to follow pch300 in `prettyprint_ich_descriptor_master()` and fixed that up. `ich_number_of_masters()` was left untouched. I wonder how much of this declarative information about the differences could be encoded into just a big table instead of scattered over heaps of switch statements in the control flow itself? A lot of it doesn't seem dynamic just XXX has 3 and YYY has 4 of "thing". Resolved? File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62282/comment/62044ba6_c49c4d9d PS8, Line 1061: CHIPSET_ELKHART_LAKE > Google should have access too. Since ~SKL, both the SPI guide and FIT are […] Long story about the access ACL's there.. Can talk out of band about it || summarised as 'a lot of nonsense'. -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 10 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Apr 2022 00:30:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) April 2, 2022

April 2, 2022

Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Hello Sam McNally, build bot (Jenkins), Subrata Banik, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62282 to look at the new patch set (#10). Change subject: ichspi: Add Jasper Lake support ...................................................................... ichspi: Add Jasper Lake support Additionally, utilize CSSO (CPU Soft Strap Offset) to uniquely detect the chipset when the CSSL (CPU Soft Strap Length) field default value (0x03) on Jasper Lake is the same as Elkhart Lake. BUG=b:221175960 TEST=dedede with `flashrom -p internal --flash-size`. ``` $ flashrom -VVV -p internal --ifd -i fd -i bios -r /tmp/filename.rom <snip> Enabling hardware sequencing by default for 100+ series PCH. OK. No board enable found matching coreboot IDs vendor="Google", model="Magolor". The following protocols are supported: Programmer-specific. Probing for Programmer Opaque flash chip, 0 kB: Chip identified: GD25Q127C/GD25Q128C Hardware sequencing reports 1 attached SPI flash chip with a density of 16384 kB. There is only one partition containing the whole address space (0x000000 - 0xffffff). There are 4096 erase blocks with 4096 B each. Added layout entry 00000000 - 00ffffff named complete flash Found GigaDevice flash chip "GD25Q127C/GD25Q128C" (16384 kB, Programmer-specific) mapped at physical address 0x0000000000000000. Found GigaDevice flash chip "GD25Q127C/GD25Q128C" (16384 kB, Programmer-specific). This chip may contain one-time programmable memory. flashrom cannot read and may never be able to write it, hence it may not be able to completely clone the contents of this chip (see man page for details). Reading Status register Block protection is disabled. Reading ich descriptor... Reading 4096 bytes starting at 0x000000. done. Assuming chipset 'Jasper Lake'. Added layout entry 00000000 - 00000fff named fd Added layout entry 00381000 - 00ffffff named bios Added layout entry 00001000 - 00380fff named me restore_power_management: Re-enabling power management. Using regions: "bios", "fd". Reading Status register Block protection is disabled. Reading flash... 0x381000-0xffffff:R Reading 13103104 bytes starting at 0x381000. 000000-0x0fff:R Reading 4096 bytes starting at 0x000000. done. restore_power_management: Re-enabling power management. SUCCESS Restoring PCI config space for 00:1f:5 reg 0xdc restore_power_management: Re-enabling power management. ``` Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipset_enable.c M ich_descriptors.c M ichspi.c M programmer.h M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 36 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/62282/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 10 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: pony_spi.c: Extract out get_params to simplify init
by Nico Huber (Code Review) April 2, 2022

April 2, 2022

Attention is currently required from: Edward O'Callaghan, Light, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63130 ) Change subject: pony_spi.c: Extract out get_params to simplify init ...................................................................... Patch Set 3: (1 comment) File pony_spi.c: https://review.coreboot.org/c/flashrom/+/63130/comment/ac5dcf12_bccea0c8 PS3, Line 167: bool have_device; > A bit of nit isn't it? Is there a solid technical reason here or just personal preference? From my point of view: Review doesn't scale well with the size of a patch. Even if it's a very simple patch hunk, every additional hunk can distract from the error-prone part of a patch. Doing fixes and simple changes like this in advance works well, FWIW. -- To view, visit https://review.coreboot.org/c/flashrom/+/63130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac Gerrit-Change-Number: 63130 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 02 Apr 2022 14:19:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Daniel Campello (Code Review) April 2, 2022

April 2, 2022

Attention is currently required from: Hung-Te Lin, Nico Huber, Subrata Banik, Edward O'Callaghan. Daniel Campello has uploaded a new patch set (#17) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... flashrom.c: Implement file-based locking semantics This upstreams the ChromiumOS implementation of file-based locking for multiple instances of flashrom that could be spawned either from libflashrom (perhaps by fwupd for example) and user cli as another example. Since flashrom is programming singleton state of hardware from userspace there is no way to exclusively own the address space and therefore a file-based locking semantic is considered here. BUG=b:217629892,b:215255210 BRANCH=none TEST=nm -gD /build/brya/usr/lib64/libflashrom.so | grep "flock" Test futility update with multiple instances of flashrom running. Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M Makefile A big_lock.c A big_lock.h A file_lock.c M flashrom.c A ipc_lock.h M meson.build M tests/chip.c M tests/include/test.h M tests/lifecycle.c M tests/meson.build M tests/tests.c 12 files changed, 492 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/62213/17 -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 17 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0