Hi Subrata,
On 16.03.22 17:19, Subrata Banik wrote:
Hope you are doing well. I would like bring this activity in your notice https://review.coreboot.org/q/topic:PCH_HW_SEQ_Cleanup and seek help to review the code.
thank you for bringing this to the mailing list. Not only but also because `ichspi` is maybe the most important driver in flashrom it is important to discuss changes very early. We have to avoid re- gressions at all cost, so all changes need to be fully understood before it makes sense to look into the implementation details.
The reason for this refactoring of HW sequencing SPI driver code are:
- We (Chrome OS team) recently ran into a firmware update issue on
dogfooders (test device utilise by real users and share feedback) systems, where the attempt to perform SPI flash update operation from host-cpu (using underlying flashrom utility) is silently failing. Furthermore, we debug the issue with help of Intel and figure out the problem is related to multiple master is accessing the SPI flash and operation triggered by host-cpu side using flashrom (write and erase operation) is getting timed out (due to given lesser timeout boundary) due to underlying SPI bus is occupied by other master.
Does the issue persist with the following commit applied to libflashrom, activated and integrated into futility?
commit 330088d77 [CHROMIUM]: libflashrom: Also use USE_BIG_LOCK
This would give us a hint if the issue is indeed due to multiple masters or actually due to multiple programs trying to control the same master.
This is the special case, starting from Tiger Lake Chrome Platform, where we have enable the PVAP (Protected Audio Video Path) which request CSE to perform some erase and write operation as needed.
We used flashrom on PAVP enabled systems for more than a decade without issues. Did something change in that area for Tiger Lake in particular?
- Maintain the code symmetry between coreboot and flashrom SPI hardware
sequencing operations.
Flashrom has tighter requirements compared to coreboot because we try to support all compatible chipset generations with the same code. If you want to align the two code-bases, please use flashrom as reference and patch coreboot.
Also, if you want to save some time in the long run: If we'd focus on the flashrom code-base, e.g. prepare libflashrom for easier integration into embedded environments, we could drop the redundant code in core- boot.
Cheers, Nico