Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43890 )
Change subject: mb/purism/librem_skl: Relocate devicetree FSP settings ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... File src/mainboard/purism/librem_skl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 160: # FIXME: corresponding device entry is missing
If so, then I can drop the FIXME and replace it with `device pci 04. […]
fine by me
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 168: # FIXME: does not match devicetree!
The enabled PCI device or the "HeciEnabled" value? If the former, we can check if HeciEnabled = 1 ma […]
the former. though I think by the time it goes to print the HECI status cse_enabled returns false so I've patched that out. I'll have to test if disabling the PCI device in coreboot is problematic. in librem_whl I have it reversed and a note that not enabling HECI in FSP causes suspend issues
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 169: register "HeciEnabled" = "0"
Probably not, and the PCI device should be hidden then. […]
it is hidden, by FSP
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 195: # FIXME: This seems specific to Librem 15
So... if only librem 15 enables 1c. […]
FSP RP coalescing moves RP5 to RP1 on both 13/15 variants. Since that happens before coreboot walks the PCI bus, it's probably not necessary to enable RP5 at all