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

Change in flashrom[master]: mec1308.c: Separate shutdown from failed init cleanup
by Anastasia Klimchuk (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52597 to look at the new patch set (#2). Change subject: mec1308.c: Separate shutdown from failed init cleanup ...................................................................... mec1308.c: Separate shutdown from failed init cleanup Shutdown function was covering two different jobs here: 1) the actual shutdown which is run at the end of the driver's lifecycle and 2) cleanup in cases when initialisation failed. Now, shutdown is only doing its main job (#1), and the driver itself is doing cleanup when init fails (#2). The good thing is that now resources are released/closed immediately in cases when init fails (vs shutdown function which was run at some point later), and the driver leaves clean space after itself if init fails. And very importantly this unlocks API change which plans to move register_shutdown inside register master API, see https://review.coreboot.org/c/flashrom/+/51761 TEST=builds and ninja test from 51487 BUG=b:185191942 Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M mec1308.c 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/52597/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52597 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Gerrit-Change-Number: 52597 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: mec1308.c: Extract params check into a function
by Anastasia Klimchuk (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Anastasia Klimchuk. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52595 to look at the new patch set (#2). Change subject: mec1308.c: Extract params check into a function ...................................................................... mec1308.c: Extract params check into a function This allows char *p to become a local variable in check_params, and it is allocated and freed within check_params function. Which means init function does not need char *p anymore, in particular does not need to free it - and this makes cleanup after failed init easier. As a good side effect, init function becomes easier to read. TEST=builds and ninja test from 51487 BUG=b:185191942 Change-Id: If5be7709e93233a2e7ea9133de50382d2524a55f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M mec1308.c 1 file changed, 16 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/52595/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52595 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5be7709e93233a2e7ea9133de50382d2524a55f Gerrit-Change-Number: 52595 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: linux_spi.c: Drop some unnecessary initialisations and checks
by Anastasia Klimchuk (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Drop some unnecessary initialisations and checks ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/52492/comment/5e6e37c8_afb22ee0 PS1, Line 7: linux_spi.c: Resolve comments left from previous patches > The summary should be about the changes, not what led to them :) […] Done Patchset: PS1: > Thanks for the quick reaction! I don't see it as your […] thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 00:06:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Drop some unnecessary initialisations and checks
by Anastasia Klimchuk (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52492 to look at the new patch set (#2). Change subject: linux_spi.c: Drop some unnecessary initialisations and checks ...................................................................... linux_spi.c: Drop some unnecessary initialisations and checks In previous patches 52283, 52284, 52285 there were some unresolved comments left, resolving [and replying to] all of that here. TEST=builds and ninja test from 51487 BUG=b:185191942 Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M linux_spi.c 1 file changed, 7 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/52492/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: cli_classic.c: implement set_wp_region operation
by Daniel Campello (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Sam McNally, Daniel Campello. Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52531 to look at the new patch set (#2). Change subject: cli_classic.c: implement set_wp_region operation ...................................................................... cli_classic.c: implement set_wp_region operation set_wp_region allows to set the wp_range based on a layout region. Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: Ibad68a038ab38b9986b0d8b5f5eb6c73b20ef381 --- M cli_classic.c M layout.c M layout.h 3 files changed, 31 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/52531/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52531 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibad68a038ab38b9986b0d8b5f5eb6c73b20ef381 Gerrit-Change-Number: 52531 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: cli_classic.c: reorder writeprotect operation processing
by Daniel Campello (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Sam McNally, Daniel Campello. Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52530 to look at the new patch set (#2). Change subject: cli_classic.c: reorder writeprotect operation processing ...................................................................... cli_classic.c: reorder writeprotect operation processing Make sure that layout is set before. Also as the comment instructs make sure that set_rw_range happens before set_wp_enable. Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: I7480d3f947aaaf30093d056226fe0c402763efdc --- M cli_classic.c 1 file changed, 64 insertions(+), 63 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/52530/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52530 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7480d3f947aaaf30093d056226fe0c402763efdc Gerrit-Change-Number: 52530 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support
by Daniel Campello (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Sam McNally, Nico Huber, Nicola Corna, Jack Rosenthal, Paul Menzel, Stefan Reinauer, David Hendricks, Arthur Heymans, Carl-Daniel Hailfinger. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 ) Change subject: layout: Add -i <region>[:<file>] support ...................................................................... Patch Set 34: (1 comment) File layout.c: https://review.coreboot.org/c/flashrom/+/23021/comment/29ba6d14_76979ab8 PS33, Line 127: name = colon ? strndup(arg, colon - arg) : strdup(arg); : file = colon ? strdup(colon + 1) : NULL; > Personally, when I wrote this, I debated between the two options and I think that the current form i […] Marking resolved -- To view, visit https://review.coreboot.org/c/flashrom/+/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 34 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: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nicola Corna <nicola(a)corna.info> Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Comment-Date: Thu, 22 Apr 2021 16:25:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jack Rosenthal <jrosenth(a)chromium.org> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: Add Spansion/Cypress S25FL256L
by Angel Pons (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Nico Huber, Thomas Walker. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52310 ) Change subject: flashchips: Add Spansion/Cypress S25FL256L ...................................................................... Patch Set 5: (1 comment) File spi25.c: https://review.coreboot.org/c/flashrom/+/52310/comment/17b928d5_c064c5fc PS5, Line 492: true > Looks like we grouped the 4BA commands at the end. Not sure if it matters, though. We could put it next to _5c -- To view, visit https://review.coreboot.org/c/flashrom/+/52310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4e8cba554d0590c94dac92aa91e9ab400efca281 Gerrit-Change-Number: 52310 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Walker <thh.walker(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Walker <thh.walker(a)gmail.com> Gerrit-Comment-Date: Thu, 22 Apr 2021 16:16:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: Add Spansion/Cypress S25FL256L
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Thomas Walker. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52310 ) Change subject: flashchips: Add Spansion/Cypress S25FL256L ...................................................................... Patch Set 5: (3 comments) File flashchips.c: https://review.coreboot.org/c/flashrom/+/52310/comment/2410635f_8d3c0df5 PS5, Line 16387: FEATURE_4BA I see no mention of EAR (extended address register) in the datasheet, so that should be FEATURE_4BA_ENTER | FEATURE_4BA_NATIVE i.e. excluding FEATURE_4BA_EXT_ADDR The problem with these big chips and their 4-byte addresses is that there are usually at least 3 different ways to do it and that makes things harder to test. https://review.coreboot.org/c/flashrom/+/52310/comment/5e6c7f69_a228885c PS5, Line 16387: .feature_bits = FEATURE_WRSR_WREN | FEATURE_4BA, Could add FEATURE_OTP. File spi25.c: https://review.coreboot.org/c/flashrom/+/52310/comment/24661c19_68cd6979 PS5, Line 492: true Looks like we grouped the 4BA commands at the end. Not sure if it matters, though. -- To view, visit https://review.coreboot.org/c/flashrom/+/52310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4e8cba554d0590c94dac92aa91e9ab400efca281 Gerrit-Change-Number: 52310 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Walker <thh.walker(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Walker <thh.walker(a)gmail.com> Gerrit-Comment-Date: Thu, 22 Apr 2021 16:12:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: chipset_enable.c: Add IDs for H310C and B365 PCHs
by Angel Pons (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Nico Huber, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52605 ) Change subject: chipset_enable.c: Add IDs for H310C and B365 PCHs ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52605/comment/ead03117_bd02ab95 PS1, Line 7: chipset_enable.c: Add IDs for H310C and B365 PCHs > As it's not in the datasheet, please mention where the 0xa2cc was […] https://linux-hardware.org/index.php?id=pci:8086-a2cc-1849-a2cc Do I paste the link in the commit message? -- To view, visit https://review.coreboot.org/c/flashrom/+/52605 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If9f0a49a0f1821e5592213e07962ee48654cdc07 Gerrit-Change-Number: 52605 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 22 Apr 2021 15:49:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021