Attention is currently required from: Nico Huber, Caveh Jalali, Tim Wawrzynczak, Rizwan Qureshi, Edward O'Callaghan, Angel Pons, Nick Vaccaro, Alex Levin. 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 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/61854/comment/a15b4b76_3cb054e8 PS2, Line 23: BUG=b:215255210
Please describe the issue you encountered.
I have added the issue description below @Nico. So far we don't know if this fix will able to fix the original issue hence tried to capture at the high level what SW expects us to implement and what we missed.
Patchset:
PS2:
The whole driver works synchronously, i.e. waits for any cycle to finish that it started itself. So unless I miss something, checking SCIP would only serve us in case of hardware failures or another program trying to use the SPI controller at the same time (e.g. broken SMM). In both cases, I'd say all bets are off anyway and we should definitely bail out.
Datasheets say to check the SCIP bit; we could just do so once (without waiting) and if the bit is set bail out.
@Nico, there are multiple HW agents who also has access to the SPI controller, example: CSE. While you are flashing the SPI using flashrom on DUT at S0, there are cases when we are seeing some flashrom failure issue and debug data that we have so far is not very conclusive to say that CSE is doing anything wrong. There might be case where both CSE and Host CPU attempt to initiate the transaction without any sync mechanism. Apparently on multiple master SPI access case, SW programming says, don't initiate next command unless SCIP is set to zero.
We have verified CSE FW use this implementation to avoid race scenarios but none of HW seq implementation that we have in coreboot/depthcharge/flashrom adheres to the same recommendation so far.