I could split this into two patches, one that just hardcodes the DRHD values, and another that "eases" FSP's job. Should I reuse this change ID for DRHD hardcoding or register writes?
based on the commit subject, the former IMO
3 comments:
File src/soc/intel/skylake/chip_fsp20.c:
// We set that in coreboot already
params->PchIoApicBdfValid = 0;
The FSP has four UPDs for this: The trio for Bus:Dev.Fun values, and a fourth "Valid" UPD. […]
makes sense, but the comment and the code don't without that context.
I'd argue a comment such as: "// APIC Bus:Dev.Fun set in coreboot, so ignore UPDs" would be more useful to anyone reading the code outside of this review :)
File src/soc/intel/skylake/include/soc/systemagent.h:
Patch Set #2, Line 65: Hardcoded default values for PCI Bus:Dev.Fun for IOAPIC and HPET
IBDF values come from chip_fsp20.c […]
right, but they appear to be common across multiple Intel SoC's, so I assume it's documented somewhere publicly?
File src/soc/intel/skylake/romstage/romstage_fsp20.c:
// We set that in coreboot already
m_cfg->PchHpetBdfValid = 0;
FSP-M writes the Bus:Dev.Fun for the HPET (done here), whereas FSP-S writes the Bus:Dev. […]
ah, I wasn't looking carefully enough at the UPDs. My comment for the one chip_fsp20.c applies here as well, since it's unclear what coreboot sets or how setting the UPD to zero relates
To view, visit change 35108. To unsubscribe, or for help writing mail filters, visit settings.