Attention is currently required from: Nico Huber, Patrick Georgi, Martin Roth, Caveh Jalali, Stefan Reinauer, Tim Wawrzynczak, Sridhar Siricilla, Angel Pons, Alex Levin, Nick Vaccaro, YH Lin, Boris Mittelberg.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61854 )
Change subject: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
So, if this code change is considered a missed step in HW sequencing recommendation, then this could still be merged..correct?
A version of this change that takes the applicable context into account
(e.g. other waiting loops in this driver, implications when multiple
instances would reach this point in the code simultaneously) would be
accepted immediately.
As for the failure that was noticed, it is when flashrom was being used in 2 modes
- flashrom standalone
- via futility which uses libflashrom implementation
The standalone flashrom already has a fix for the issue, i.e, lockfile mechanism.
libflashrom did not utilize/inherit the lockfile feature and did not acquire the lock. Which allowed other instances to run. This is already being fixed https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+...
AFAIK, there is no such locking (yet) in this project. However, I fully
agree that file-based locking is the correct way. And it's interesting
to see that the problem is already solved for the fork. Makes me wonder
why Subrata asks how to solve such things here.
Isn't the HW based locking is much better than file based locking and how one can achieve file based locking even in low-level fw like coreboot spi driver or depthcharge ?
The reason, we opted for file based locking is to have a short term approach and share a fix rather waiting here in loop in code review :-). we are looking for a long term fix inside flashrom to have a `_wait_before` executing run execution.
--
To view, visit
https://review.coreboot.org/c/flashrom/+/61854
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484
Gerrit-Change-Number: 61854
Gerrit-PatchSet: 6
Gerrit-Owner: Subrata Banik
subratabanik@google.com
Gerrit-Reviewer: Alex Levin
levinale@chromium.org
Gerrit-Reviewer: Boris Mittelberg
bmbm@google.com
Gerrit-Reviewer: Caveh Jalali
caveh@chromium.org
Gerrit-Reviewer: Edward O'Callaghan
quasisec@chromium.org
Gerrit-Reviewer: Martin Roth
martinroth@google.com
Gerrit-Reviewer: Nick Vaccaro
nvaccaro@google.com
Gerrit-Reviewer: Nico Huber
nico.h@gmx.de
Gerrit-Reviewer: Patrick Georgi
patrick@coreboot.org
Gerrit-Reviewer: Rizwan Qureshi
rizwan.qureshi@intel.com
Gerrit-Reviewer: Sridhar Siricilla
sridhar.siricilla@intel.com
Gerrit-Reviewer: Stefan Reinauer
stefan.reinauer@coreboot.org
Gerrit-Reviewer: Tim Wawrzynczak
twawrzynczak@chromium.org
Gerrit-Reviewer: YH Lin
yueherngl@chromium.org
Gerrit-Reviewer: build bot (Jenkins)
no-reply@coreboot.org
Gerrit-CC: Angel Pons
th3fanbus@gmail.com
Gerrit-CC: Paul Menzel
paulepanter@mailbox.org
Gerrit-Attention: Nico Huber
nico.h@gmx.de
Gerrit-Attention: Patrick Georgi
patrick@coreboot.org
Gerrit-Attention: Martin Roth
martinroth@google.com
Gerrit-Attention: Caveh Jalali
caveh@chromium.org
Gerrit-Attention: Stefan Reinauer
stefan.reinauer@coreboot.org
Gerrit-Attention: Tim Wawrzynczak
twawrzynczak@chromium.org
Gerrit-Attention: Sridhar Siricilla
sridhar.siricilla@intel.com
Gerrit-Attention: Angel Pons
th3fanbus@gmail.com
Gerrit-Attention: Alex Levin
levinale@chromium.org
Gerrit-Attention: Nick Vaccaro
nvaccaro@google.com
Gerrit-Attention: YH Lin
yueherngl@chromium.org
Gerrit-Attention: Boris Mittelberg
bmbm@google.com
Gerrit-Comment-Date: Mon, 14 Feb 2022 14:31:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber
nico.h@gmx.de
Comment-In-Reply-To: Subrata Banik
subratabanik@google.com
Gerrit-MessageType: comment