Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43890 )
Change subject: mb/purism/librem_skl: Relocate devicetree FSP settings ......................................................................
mb/purism/librem_skl: Relocate devicetree FSP settings
Tested with BUILD_TIMELESS=1, Librem 15 v4 does not change.
Change-Id: Ia968c7ab0b7b305c0c32f43eafc4d25d670a135e Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/purism/librem_skl/devicetree.cb 1 file changed, 48 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/43890/1
diff --git a/src/mainboard/purism/librem_skl/devicetree.cb b/src/mainboard/purism/librem_skl/devicetree.cb index 51f6ba4..81f2e31 100644 --- a/src/mainboard/purism/librem_skl/devicetree.cb +++ b/src/mainboard/purism/librem_skl/devicetree.cb @@ -45,29 +45,12 @@
# FSP Configuration register "ProbelessTrace" = "0" - register "EnableLan" = "0" - register "EnableSata" = "1" - register "SataSalpSupport" = "0" - register "SataMode" = "0" - register "SataPortsEnable[0]" = "1" - register "SataPortsEnable[1]" = "0" - register "SataPortsEnable[2]" = "1" - register "SataPortsDevSlp[0]" = "0" - register "SataPortsDevSlp[2]" = "0" - register "EnableAzalia" = "1" register "DspEnable" = "0" register "IoBufferOwnership" = "0" - register "EnableTraceHub" = "0" register "SsicPortEnable" = "0" - register "SmbusEnable" = "1" register "Cio2Enable" = "0" - register "ScsEmmcEnabled" = "0" - register "ScsEmmcHs400Enabled" = "0" - register "ScsSdCardEnabled" = "0" register "PttSwitch" = "0" register "SkipExtGfxScan" = "1" - register "Device4Enable" = "1" - register "HeciEnabled" = "0" register "SaGv" = "3" register "PmConfigSlpS3MinAssert" = "2" # 50ms register "PmConfigSlpS4MinAssert" = "1" # 1s @@ -154,10 +137,6 @@ .dc_loadline = 420, }"
- # Enable Root Ports 5 and 9 - register "PcieRpEnable[4]" = "1" - register "PcieRpEnable[8]" = "1" - # PL2 override 25W register "power_limits_config" = "{ .tdp_pl2_override = 25, @@ -177,27 +156,60 @@ device domain 0 on device pci 00.0 on end # Host Bridge device pci 02.0 on end # Integrated Graphics Device + + # FIXME: corresponding device entry is missing + register "Device4Enable" = "1" + device pci 14.0 on end # USB xHCI device pci 14.1 on end # USB xDCI (OTG) device pci 14.2 on end # Thermal Subsystem - device pci 16.0 on end # Management Engine Interface 1 + device pci 16.0 on # Management Engine Interface 1 + + # FIXME: does not match devicetree! + register "HeciEnabled" = "0" + end device pci 16.1 off end # Management Engine Interface 2 device pci 16.2 off end # Management Engine IDE-R device pci 16.3 off end # Management Engine KT Redirection device pci 16.4 off end # Management Engine Interface 3 - device pci 17.0 on end # SATA + device pci 17.0 on # SATA + register "EnableSata" = "1" + register "SataSalpSupport" = "0" + register "SataMode" = "0" + + register "SataPortsEnable" = "{ \ + [0] = 1, \ + [2] = 1, \ + }" + register "SataPortsDevSlp" = "{ \ + [0] = 0, \ + [2] = 0, \ + }" + end device pci 1c.0 on end # PCI Express Port 1 device pci 1c.1 off end # PCI Express Port 2 device pci 1c.2 off end # PCI Express Port 3 device pci 1c.3 off end # PCI Express Port 4 - device pci 1c.4 off end # PCI Express Port 5 + device pci 1c.4 off # PCI Express Port 5 + + # FIXME: Maybe it's due to FSP coalescing? + register "PcieRpEnable[4]" = "1" + end device pci 1c.5 off end # PCI Express Port 6 device pci 1c.6 off end # PCI Express Port 7 device pci 1c.7 off end # PCI Express Port 8 - device pci 1d.0 on end # PCI Express Port 9 + device pci 1d.0 on # PCI Express Port 9 + register "PcieRpEnable[8]" = "1" + end device pci 1d.1 off end # PCI Express Port 10 device pci 1d.2 off end # PCI Express Port 11 device pci 1d.3 off end # PCI Express Port 12 + + # eMMC/SD are disabled + register "ScsEmmcEnabled" = "0" + register "ScsEmmcHs400Enabled" = "0" + register "ScsSdCardEnabled" = "0" + device pci 1f.0 on chip drivers/pc80/tpm device pnp 0c31.0 on end @@ -205,9 +217,17 @@ end # LPC Interface device pci 1f.1 on end # P2SB device pci 1f.2 on end # Power Management Controller - device pci 1f.3 on end # Intel HDA - device pci 1f.4 on end # SMBus + device pci 1f.3 on # Intel HDA + register "EnableAzalia" = "1" + end + device pci 1f.4 on # SMBus + register "SmbusEnable" = "1" + end device pci 1f.5 on end # PCH SPI - device pci 1f.6 off end # GbE + device pci 1f.6 off # GbE + register "EnableLan" = "0" + end + + register "EnableTraceHub" = "0" end end
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43890
to look at the new patch set (#2).
Change subject: mb/purism/librem_skl: Relocate devicetree FSP settings ......................................................................
mb/purism/librem_skl: Relocate devicetree FSP settings
Tested with BUILD_TIMELESS=1, Librem 15 v4 does not change.
Change-Id: Ia968c7ab0b7b305c0c32f43eafc4d25d670a135e Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/purism/librem_skl/devicetree.cb 1 file changed, 48 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/43890/2
Felix Singer 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: Code-Review+2
Michael Niewöhner 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: Code-Review+2
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: Code-Review-1
(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 so? coreboot doesn't do anything with it
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 168: # FIXME: does not match devicetree! this is necessary in order to have the ME status registers printed in cbmem
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 169: register "HeciEnabled" = "0" not sure this has any effect when ME disabled via HAP
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 195: # FIXME: This seems specific to Librem 15 it's not, WiFi is attached to RP5 on all librem_skl variants
Angel Pons 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
so? coreboot doesn't do anything with it
If so, then I can drop the FIXME and replace it with `device pci 04.0 on end` when we drop "Device4Enable"
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 168: # FIXME: does not match devicetree!
this is necessary in order to have the ME status registers printed in cbmem
The enabled PCI device or the "HeciEnabled" value? If the former, we can check if HeciEnabled = 1 makes any difference.
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 169: register "HeciEnabled" = "0"
not sure this has any effect when ME disabled via HAP
Probably not, and the PCI device should be hidden then. Would be good to test because most boards have these conflicting settings (device pci is on, HeciEnabled = 0)
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 195: # FIXME: This seems specific to Librem 15
it's not, WiFi is attached to RP5 on all librem_skl variants
So... if only librem 15 enables 1c.4 in devicetree, how does librem 13 work?
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
Angel Pons 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:
(1 comment)
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 195: # FIXME: This seems specific to Librem 15
FSP RP coalescing moves RP5 to RP1 on both 13/15 variants. […]
Ew, coalescing...
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:
(1 comment)
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 195: # FIXME: This seems specific to Librem 15
Ew, coalescing...
I agree, and I'd disable it just to have a proper mapping but I don't know of any other consequences from doing so
Hello Felix Singer, build bot (Jenkins), Matt DeVillier, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43890
to look at the new patch set (#3).
Change subject: mb/purism/librem_skl: Relocate devicetree settings ......................................................................
mb/purism/librem_skl: Relocate devicetree settings
Tested with BUILD_TIMELESS=1, Librem 15 v4 does not change.
Change-Id: Ia968c7ab0b7b305c0c32f43eafc4d25d670a135e Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/purism/librem_skl/devicetree.cb 1 file changed, 48 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/43890/3
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43890 )
Change subject: mb/purism/librem_skl: Relocate devicetree settings ......................................................................
Patch Set 3:
(1 comment)
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 195: # FIXME: This seems specific to Librem 15
I agree, and I'd disable it just to have a proper mapping but I don't know of any other consequences […]
guys, wait... devtree is updated accordingly by coreboot, so you don't have to even think about coalescing
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43890 )
Change subject: mb/purism/librem_skl: Relocate devicetree settings ......................................................................
Patch Set 3:
(5 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 189: device pci 1c.0 on end # PCI Express Port 1 see below, this can be disabled then
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 195: # FIXME: This seems specific to Librem 15
guys, wait... […]
see CB:35985, CB:36702, CB:36651 ;; rp 5 must be enabled
https://review.coreboot.org/c/coreboot/+/43890/3/src/mainboard/purism/librem... File src/mainboard/purism/librem_skl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43890/3/src/mainboard/purism/librem... PS3, Line 189: on off, see discussion on coalescing; should be done in another commit, though
https://review.coreboot.org/c/coreboot/+/43890/3/src/mainboard/purism/librem... PS3, Line 193: off do not change this from on, see discussion on coalescing
https://review.coreboot.org/c/coreboot/+/43890/3/src/mainboard/purism/librem... PS3, Line 195: # FSP coalescing moves this to RP 1 remove that comment; one does not need to care about coalescing here since devtree reflects the reset state
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43890 )
Change subject: mb/purism/librem_skl: Relocate devicetree settings ......................................................................
Patch Set 3:
(2 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 189: device pci 1c.0 on end # PCI Express Port 1
see below, this can be disabled then
not in this commit, though
https://review.coreboot.org/c/coreboot/+/43890/2/src/mainboard/purism/librem... PS2, Line 195: # FIXME: This seems specific to Librem 15
see CB:35985, CB:36702, CB:36651 ;; rp 5 must be enabled
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43890 )
Change subject: mb/purism/librem_skl: Relocate devicetree settings ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43890/3/src/mainboard/purism/librem... File src/mainboard/purism/librem_skl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43890/3/src/mainboard/purism/librem... PS3, Line 193: off
do not change this from on, see discussion on coalescing
I didn't change it
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43890 )
Change subject: mb/purism/librem_skl: Relocate devicetree settings ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Felix Singer (Code Review)
-
Matt DeVillier (Code Review)
-
Michael Niewöhner (Code Review)