Nico Huber 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:
Did you compare testing results between CML-H and other platforms like SKL/KBL-H?
Yes, SHL/KBL H also has total 8 SATA port. The only difference is SPD bit definitions there. thats we have already handled like this
https://github.com/coreboot/coreboot/blob/master/src/soc/intel/common/block/...
Difference between H and LP would be for SATA, LP has 0-2 implemented and rest 3-7 reserved.
I wonder why I had no problems with this on X11SSM-F (100series PCH-H) so far
May your SATA direct connect Port is between 0-2, else you would also see this issue for sure.
I have 8 SATA Ports and use 2-6 and there are no problems
You seems lucky that in ur case FSP might have already written to SPD register and as this SPD register is RW/O hence coreboot next writes has no effect. But if FSP skips settings those SPD and coreboot only know abut (ports 0-2) and disables other ports it would have result the same what i'm seeing now.
So now question is do you like to over commit that FSP will always things proper and coreboot might not need to program SPD bits at finalize stage ? This code might be like double insurance in coreboot to handle such misses from FSP.
I would prefer to get rid of this driver. We could do it step by step, e.g. for every platform where it causes trouble, test (from the OS, .final might be too early) if FSP configured things, and if so, remove the PCI IDs for that platform from this driver. Leave a comment to make sure they are not added back by accident. If we end up with an empty PCI ID list eventually, we can drop the driver.
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.