Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41674 )
Change subject: soc/intel/common/block/sata: Fix SATA detection issue between Ports 3-7 ......................................................................
Patch Set 4: -Code-Review
Patch Set 4:
If you want to keep and fix the driver, I suggest to first test at what stage (silicon init / notification phase?) FSP configures things. coreboot's .final seems to run after the PCI Enum Complete phase, but before the last two phases.
FSP does this in NotifyPhaseApi() - Begin [Phase: 00000020] Coreboot does it in Finalize devices... PCI: 00:17.0 final
This is odd, if FSP runs first and should write a write-once register, coreboot should not have any effect, right? Maybe there is a bug in your FSP?
Yes, looks like in my case FSP doesn't even programming those register in notify phase and coreboot is doing wrong 0-2 bit enabling results into SATA device not detected over Port 4. Hence I don't want to much dependent on FSP for such programming at first place and make sure coreboot doing things right if such condition do arise in future.
I don't want to rely on FSP either. But it's there. And Nate once commented that the point of using Intel's blobs is to have a single upstream where bug fixes are gathered so every Intel customer can get them. If we cover FSP bugs up, we defeat its purpose. Don't worry, I don't mind much. If I'd ever do a port for a new Intel platform, I would not use FSP-S anyway. It causes too much trouble.
If different FSP binaries do different things with these registers and then we have to do different things in coreboot, maybe we should make this platform-specific.