Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device ......................................................................
mb/{google,intel,siemens}/devicetree.cb: Enable CSE device
Enable CSE PCI device Bus 0: Device 0x0f: Function 0x00 to let Intel common cse block code can use this device.
Calling me_read_config32(offset) function from ramstage:
Without this CL: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
With this CL: HECI: Global Reset(Type:1) Command HECI: Global Reset success!
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I97d221ae52b4b03ecd859d708847ad77fe4bf465 --- M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/apollolake_rvp/devicetree.cb M src/mainboard/intel/leafhill/devicetree.cb M src/mainboard/intel/minnow3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb 14 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/45469/1
diff --git a/src/mainboard/google/reef/variants/baseboard/devicetree.cb b/src/mainboard/google/reef/variants/baseboard/devicetree.cb index 4c35bd2..4c93096 100644 --- a/src/mainboard/google/reef/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/reef/variants/baseboard/devicetree.cb @@ -139,6 +139,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/coral/devicetree.cb b/src/mainboard/google/reef/variants/coral/devicetree.cb index f987e1d..249e2c8 100644 --- a/src/mainboard/google/reef/variants/coral/devicetree.cb +++ b/src/mainboard/google/reef/variants/coral/devicetree.cb @@ -139,6 +139,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/pyro/devicetree.cb b/src/mainboard/google/reef/variants/pyro/devicetree.cb index 1282edb..9ed1c4c 100644 --- a/src/mainboard/google/reef/variants/pyro/devicetree.cb +++ b/src/mainboard/google/reef/variants/pyro/devicetree.cb @@ -148,6 +148,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/sand/devicetree.cb b/src/mainboard/google/reef/variants/sand/devicetree.cb index ad76a91..0b1134c 100644 --- a/src/mainboard/google/reef/variants/sand/devicetree.cb +++ b/src/mainboard/google/reef/variants/sand/devicetree.cb @@ -135,6 +135,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/snappy/devicetree.cb b/src/mainboard/google/reef/variants/snappy/devicetree.cb index a82400f..fdd84dd 100644 --- a/src/mainboard/google/reef/variants/snappy/devicetree.cb +++ b/src/mainboard/google/reef/variants/snappy/devicetree.cb @@ -144,6 +144,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/intel/apollolake_rvp/devicetree.cb b/src/mainboard/intel/apollolake_rvp/devicetree.cb index f7e82a0..dd12a5b 100644 --- a/src/mainboard/intel/apollolake_rvp/devicetree.cb +++ b/src/mainboard/intel/apollolake_rvp/devicetree.cb @@ -22,6 +22,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 on end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - PCIe-A 0 diff --git a/src/mainboard/intel/leafhill/devicetree.cb b/src/mainboard/intel/leafhill/devicetree.cb index 6c872b1..e2f2f8e 100644 --- a/src/mainboard/intel/leafhill/devicetree.cb +++ b/src/mainboard/intel/leafhill/devicetree.cb @@ -22,6 +22,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 on end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - PCIe-A 0 diff --git a/src/mainboard/intel/minnow3/devicetree.cb b/src/mainboard/intel/minnow3/devicetree.cb index 6c872b1..e2f2f8e 100644 --- a/src/mainboard/intel/minnow3/devicetree.cb +++ b/src/mainboard/intel/minnow3/devicetree.cb @@ -22,6 +22,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 on end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - PCIe-A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb index 0e72fcf..c927dcc 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb @@ -69,6 +69,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 - MACPHY diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb index bee531f..8887929 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb @@ -61,6 +61,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb index e1e79b4..9747292 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb @@ -57,6 +57,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb index be50408..89fa45e9 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb @@ -58,6 +58,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb index 33664fe..05ec8b4 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb @@ -60,6 +60,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb index a865f9f..4b0367f 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb @@ -28,6 +28,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... PS1, Line 72: Maybe you can add a space here, but it is not so important. This would then also apply to the other files.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... PS1, Line 72:
Maybe you can add a space here, but it is not so important. […]
I agree. I'd prefer if alignment would be preserved
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... PS1, Line 72:
I agree. […]
Ack
Hello Elyes HAOUAS, build bot (Jenkins), Tim Wawrzynczak, Mario Scheithauer, Angel Pons, Furquan Shaikh, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45469
to look at the new patch set (#2).
Change subject: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device ......................................................................
mb/{google,intel,siemens}/devicetree.cb: Enable CSE device
Enable CSE PCI device Bus 0: Device 0x0f: Function 0x00 to let Intel common cse block code can use this device.
Calling me_read_config32(offset) function from ramstage:
Without this CL : HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
With this CL : HECI: Global Reset(Type:1) Command HECI: Global Reset success!
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I97d221ae52b4b03ecd859d708847ad77fe4bf465 --- M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/apollolake_rvp/devicetree.cb M src/mainboard/intel/leafhill/devicetree.cb M src/mainboard/intel/minnow3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb 14 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/45469/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device ......................................................................
Patch Set 2:
(1 comment)
what about mb/google/octopus and mb/up/squared ?
https://review.coreboot.org/c/coreboot/+/45469/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45469/2//COMMIT_MSG@7 PS2, Line 7: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device mention APL?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45469/2//COMMIT_MSG@7 PS2, Line 7: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device
mention APL?
apollolake boards: enable CSE in devicetree
Subrata Banik has uploaded a new patch set (#3) to the change originally created by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
apollolake boards: Enable CSE in devicetree
Enable CSE PCI device Bus 0: Device 0x0f: Function 0x00 to let Intel common cse block code can use this device.
Calling me_read_config32(offset) function from ramstage:
Without this CL : HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
With this CL : HECI: Global Reset(Type:1) Command HECI: Global Reset success!
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I97d221ae52b4b03ecd859d708847ad77fe4bf465 --- M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/apollolake_rvp/devicetree.cb M src/mainboard/intel/leafhill/devicetree.cb M src/mainboard/intel/minnow3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb 14 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/45469/3
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45469/2//COMMIT_MSG@7 PS2, Line 7: mb/{google,intel,siemens}/devicetree.cb: Enable CSE device
apollolake boards: enable CSE in devicetree
Ack
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... PS1, Line 72:
Ack
I think it is good for the Siemens mainboards now, but the other mainboards are not changed yet.
Hello Elyes HAOUAS, build bot (Jenkins), Tim Wawrzynczak, Mario Scheithauer, Angel Pons, Furquan Shaikh, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45469
to look at the new patch set (#4).
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
apollolake boards: Enable CSE in devicetree
Enable CSE PCI device Bus 0: Device 0x0f: Function 0x00 to let Intel common cse block code can use this device.
Calling me_read_config32(offset) function from ramstage:
Without this CL : HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
With this CL : HECI: Global Reset(Type:1) Command HECI: Global Reset success!
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I97d221ae52b4b03ecd859d708847ad77fe4bf465 --- M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/apollolake_rvp/devicetree.cb M src/mainboard/intel/leafhill/devicetree.cb M src/mainboard/intel/minnow3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb M src/mainboard/up/squared/devicetree.cb 15 files changed, 15 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/45469/4
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... PS1, Line 72:
I think it is good for the Siemens mainboards now, but the other mainboards are not changed yet.
I believe we are good now with latest patchset. Thanks for your effort Mario.
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/45469/1/src/mainboard/siemens/mc_ap... PS1, Line 72:
I believe we are good now with latest patchset. Thanks for your effort Mario.
No problem, gladly.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 4: Code-Review-1
I'm not sure if we really want to enable CSE/ME/... by default
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 4: Code-Review+2
Patch Set 4: Code-Review-1
I'm not sure if we really want to enable CSE/ME/... by default
I'd rather have a board with CSTXE HECI visible from the OS than a board that may get stuck when trying to perform a global reset. Since the device is missing from the devicetree, attempting a global reset before PCI enumeration took place would result in a `dev is NULL!` halt message.
If CSTXE is not functioning normally, FSP usually hides its PCI device. Unless any user of these boards really wants the CSTXE to remain disabled, I think this patch is good to go. If there are any complaints, we can just report the device as disabled in the devicetree.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 4:
Patch Set 4: Code-Review-1
I'm not sure if we really want to enable CSE/ME/... by default
To answer your question, please look at this code https://github.com/coreboot/coreboot/blob/master/src/soc/intel/apollolake/ch...
Like every platform, coreboot will ensure to make CSE bus function disable that means prior to boot to OS (after EOP is done), CSE will not visible on bus irrespective of being listen in devicetree.cb. hence your concern is already been taken care that CSE shouldn't be default enable.
Hope this help to clarify things ?
if yes, can you please reconsider your vote?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 4: Code-Review+2
Patch Set 4:
Patch Set 4: Code-Review-1
I'm not sure if we really want to enable CSE/ME/... by default
To answer your question, please look at this code https://github.com/coreboot/coreboot/blob/master/src/soc/intel/apollolake/ch...
Like every platform, coreboot will ensure to make CSE bus function disable that means prior to boot to OS (after EOP is done), CSE will not visible on bus irrespective of being listen in devicetree.cb. hence your concern is already been taken care that CSE shouldn't be default enable.
Hope this help to clarify things ?
if yes, can you please reconsider your vote?
Got it, thank you!
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 4:
Patch Set 4: Code-Review+2
Patch Set 4:
Patch Set 4: Code-Review-1
I'm not sure if we really want to enable CSE/ME/... by default
To answer your question, please look at this code https://github.com/coreboot/coreboot/blob/master/src/soc/intel/apollolake/ch...
Like every platform, coreboot will ensure to make CSE bus function disable that means prior to boot to OS (after EOP is done), CSE will not visible on bus irrespective of being listen in devicetree.cb. hence your concern is already been taken care that CSE shouldn't be default enable.
Hope this help to clarify things ?
if yes, can you please reconsider your vote?
Got it, thank you!
Thanks a lot for review
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
apollolake boards: Enable CSE in devicetree
Enable CSE PCI device Bus 0: Device 0x0f: Function 0x00 to let Intel common cse block code can use this device.
Calling me_read_config32(offset) function from ramstage:
Without this CL : HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
With this CL : HECI: Global Reset(Type:1) Command HECI: Global Reset success!
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I97d221ae52b4b03ecd859d708847ad77fe4bf465 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45469 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Mario Scheithauer mario.scheithauer@siemens.com Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Michael Niewöhner foss@mniewoehner.de --- M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/apollolake_rvp/devicetree.cb M src/mainboard/intel/leafhill/devicetree.cb M src/mainboard/intel/minnow3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb M src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb M src/mainboard/up/squared/devicetree.cb 15 files changed, 15 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Mario Scheithauer: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved Michael Niewöhner: Looks good to me, approved
diff --git a/src/mainboard/google/reef/variants/baseboard/devicetree.cb b/src/mainboard/google/reef/variants/baseboard/devicetree.cb index 4c35bd2..da80b8e 100644 --- a/src/mainboard/google/reef/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/reef/variants/baseboard/devicetree.cb @@ -139,6 +139,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/coral/devicetree.cb b/src/mainboard/google/reef/variants/coral/devicetree.cb index f987e1d..f2fc3a6 100644 --- a/src/mainboard/google/reef/variants/coral/devicetree.cb +++ b/src/mainboard/google/reef/variants/coral/devicetree.cb @@ -139,6 +139,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/pyro/devicetree.cb b/src/mainboard/google/reef/variants/pyro/devicetree.cb index 1282edb..920431b 100644 --- a/src/mainboard/google/reef/variants/pyro/devicetree.cb +++ b/src/mainboard/google/reef/variants/pyro/devicetree.cb @@ -148,6 +148,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/sand/devicetree.cb b/src/mainboard/google/reef/variants/sand/devicetree.cb index ad76a91..aba1227 100644 --- a/src/mainboard/google/reef/variants/sand/devicetree.cb +++ b/src/mainboard/google/reef/variants/sand/devicetree.cb @@ -135,6 +135,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/google/reef/variants/snappy/devicetree.cb b/src/mainboard/google/reef/variants/snappy/devicetree.cb index a82400f..ad8c808 100644 --- a/src/mainboard/google/reef/variants/snappy/devicetree.cb +++ b/src/mainboard/google/reef/variants/snappy/devicetree.cb @@ -144,6 +144,7 @@ device generic 0 on end end end + device pci 0f.0 on end # - CSE device pci 11.0 off end # - ISH device pci 12.0 off end # - SATA device pci 13.0 off end # - Root Port 2 - PCIe-A 0 diff --git a/src/mainboard/intel/apollolake_rvp/devicetree.cb b/src/mainboard/intel/apollolake_rvp/devicetree.cb index f7e82a0..dd12a5b 100644 --- a/src/mainboard/intel/apollolake_rvp/devicetree.cb +++ b/src/mainboard/intel/apollolake_rvp/devicetree.cb @@ -22,6 +22,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 on end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - PCIe-A 0 diff --git a/src/mainboard/intel/leafhill/devicetree.cb b/src/mainboard/intel/leafhill/devicetree.cb index 6c872b1..e2f2f8e 100644 --- a/src/mainboard/intel/leafhill/devicetree.cb +++ b/src/mainboard/intel/leafhill/devicetree.cb @@ -22,6 +22,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 on end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - PCIe-A 0 diff --git a/src/mainboard/intel/minnow3/devicetree.cb b/src/mainboard/intel/minnow3/devicetree.cb index 6c872b1..e2f2f8e 100644 --- a/src/mainboard/intel/minnow3/devicetree.cb +++ b/src/mainboard/intel/minnow3/devicetree.cb @@ -22,6 +22,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 on end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - PCIe-A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb index 0e72fcf..2e43648 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl1/devicetree.cb @@ -69,6 +69,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 - MACPHY diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb index bee531f..1ac551a 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl2/devicetree.cb @@ -61,6 +61,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb index e1e79b4..bc5a9cf 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl3/devicetree.cb @@ -57,6 +57,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb index be50408..7e51666 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb @@ -58,6 +58,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb index 33664fe..b5fb33b 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb @@ -60,6 +60,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb b/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb index a865f9f..4aa8bc9 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl6/devicetree.cb @@ -28,6 +28,7 @@ device pci 0d.2 on end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio + device pci 0f.0 on end # - CSE device pci 11.0 on end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - RP 2 - PCIe A 0 diff --git a/src/mainboard/up/squared/devicetree.cb b/src/mainboard/up/squared/devicetree.cb index da2ff06..417d039 100644 --- a/src/mainboard/up/squared/devicetree.cb +++ b/src/mainboard/up/squared/devicetree.cb @@ -34,7 +34,7 @@ device pci 0d.2 off end # - SPI device pci 0d.3 off end # - Shared SRAM device pci 0e.0 on end # - Audio - device pci 0f.0 on end # - TXE + device pci 0f.0 on end # - TXE device pci 11.0 off end # - ISH device pci 12.0 on end # - SATA device pci 13.0 on end # - PCIe-A 1 - PcieRootPort[2]
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45469 )
Change subject: apollolake boards: Enable CSE in devicetree ......................................................................
Patch Set 6:
Automatic boot test returned (PASS/FAIL/TOTAL): 8/1/9 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/20046 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/20045 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/20044 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/20043 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/20042 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/20050 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/20049 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/20048 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/20047
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Mario Scheithauer (Code Review)
-
Michael Niewöhner (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)