Attention is currently required from: Benjamin Doron. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49140 )
Change subject: soc/intel/skylake/acpi: Add PEP table ......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49140/comment/ed48f280_da161083 PS3, Line 11:
Not needed. SlpS0 is not asserted externally but by the SoC.
And the pad doesn't need to be `_NF`?
It can be but it simply doesn't matter because your board doesn't use it.
So, the problem seems to be that Linux doesn't differentiate PC10-only and PC10+SlpS0 systems. Iow. there's a check missing here ... https://github.com/torvalds/linux/blob/master/drivers/platform/x86/intel_pmc...
Well, ideally the system would make it to SlpS0. I think it's correct to warn that this didn't happen. However, if the table doesn't advertise support, it can be argued that the kernel doesn't need to check the default address...
That's the point. When SlpS0 is not advertised there is no need to check/warn for S0Slp.
To me, this appears deliberate, but I couldn't be sure.
Hmm, somewhere above you wrote Linux prints an error for PC10 on vendor firmware. Did you really mean PC10 or SlpS0?
I meant PC10.
Are ASPM and L1 substates involved? The vendor firmware has the "ASPM not supported" bit set in FADT and disables L1 substates.
Yes, both are required to get to PC10.
In coreboot, I had disabled L1 substates and set ASPM to "L1" on the WLAN's root port to mitigate AER errors. I noticed now that the LAN's root port has disabled ASPM but I'm not sure why (the bit is read-write-once, I'll look into that).
Check Kconfig and devicetree for L1 states.
-
Michael Niewöhner (Code Review)