Attention is currently required from: Patrick Georgi, Rizwan Qureshi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Nico Huber, Martin Roth - Personal, Caveh Jalali, David Hendricks, Tim Wawrzynczak, Nick Vaccaro, 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 12:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/61854/comment/208fba67_bb2c08ea PS7, Line 17: Without this synchronisation being implemented, flashrom is running : into below error: : : Erasing and writing flash chip... Timeout error between offset : 0x0061c000 and 0x0061c03f (= 0x0061c000 + 63)! FAILED! : Uh oh. Erase/write failed. Checking if anything has changed.
Now I'm feeling trolled. How often have I told you that the SCIP bit isn't related to sync'ing multiple processes? I've lost count.
I guess after all, it's best if you'd write a commit message about SCPI only and leave the confusion out of it. Also, please don't add a reference to the bug tracker. I think that would be the best way to avoid further confusion. If you still have questions, please ask your colleagues. No need to bother independent open-source projects with Google-internal confusion.
if removing the bug id makes you happy and let this CL see the light, I'm happy to move in that way.
Please let me know if anything we need to help for moving this CL?
https://review.coreboot.org/c/flashrom/+/61854/comment/e9e51a4c_68d0797d PS7, Line 25: TEST=Concurrent flashrom access is not throwing timeout.
how since then so many years AU worked without being bothered about checking this SCIP bit in HW SEQ on older platform, I don't have that answer either with me. But for sure SW SEQ platform do use this SCIP bit checking.
cros-flashrom used software sequencing for a long time because some implementations of hardware sequencing did not provide a way to write all of the status registers on recent flash chips. This meant that write protection could not be set up using hwseq.
Yes David, you are spot on, I know what you mean here, SPI status register 2/3 is not supported using hw seq, hence a hybrid model is the only way. But starting from ADL, chipset doesn't support the sw seq, so sw seq is the only way out here. (I remember reading this is some ADL spec but can double confirm)
As you pointed out, software sequencing has checked the SCIP bit since commit 01d05914 when Carl-Daniel added it over a decade ago.
Yes.
Obviously because futility moved from calling cros flashrom to using lib-
flashrom lately. Why do I know that?
Don't want to argue but there is no evidence about other chipsets adopted libflashrom lately shows the same failure during AU except ADL. So, we are covering all possible to angle to fix this issue and adopt Intel's recommendation.
Please let me know if anything we need to help for moving this CL?
-
Subrata Banik (Code Review)