Change in coreboot[master]: soc/intel/tigerlake: Move PMC PCI resources under PMC device
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified that the resource allocator is aware of and does not touch these two resources. Also verify that the MEM resource is described in the coreboot table ("e820"). Verified the memory range is also untouchable from Linux's point-of-view ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved").
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 5 files changed, 39 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/1
diff --git a/src/soc/intel/tigerlake/chip.c b/src/soc/intel/tigerlake/chip.c index c1764cd..865dc20 100644 --- a/src/soc/intel/tigerlake/chip.c +++ b/src/soc/intel/tigerlake/chip.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <console/console.h> #include <device/device.h> #include <device/pci.h> #include <fsp/api.h> @@ -132,6 +133,13 @@ soc_fill_gpio_pm_configuration(); }
+__weak void soc_pmc_read_resources(struct device *dev) {} + +static void pci_domain_set_resources(struct device *dev) +{ + assign_resources(dev->link_list); +} + static struct device_operations pci_domain_ops = { .read_resources = &pci_domain_read_resources, .set_resources = &pci_domain_set_resources, @@ -149,6 +157,15 @@ #endif };
+static struct device_operations pmc_ops = { + .read_resources = soc_pmc_read_resources, + .set_resources = noop_set_resources, +#if CONFIG(HAVE_ACPI_TABLES) + .acpi_name = &soc_acpi_name, +#endif + .scan_bus = scan_static_bus, +}; + static void soc_enable(struct device *dev) { /* Set the operations if it is a special bus type */ @@ -156,6 +173,9 @@ dev->ops = &pci_domain_ops; else if (dev->path.type == DEVICE_PATH_CPU_CLUSTER) dev->ops = &cpu_bus_ops; + else if (dev->path.type == DEVICE_PATH_PCI && + dev->path.pci.devfn == PCH_DEVFN_PMC) + dev->ops = &pmc_ops; }
struct chip_operations soc_intel_tigerlake_ops = { diff --git a/src/soc/intel/tigerlake/espi.c b/src/soc/intel/tigerlake/espi.c index ed8481a..8cc8303 100644 --- a/src/soc/intel/tigerlake/espi.c +++ b/src/soc/intel/tigerlake/espi.c @@ -197,21 +197,4 @@ soc_mirror_dmi_pcr_io_dec(); }
-/* Fill up ESPI IO resource structure inside SoC directory */ -void pch_lpc_soc_fill_io_resources(struct device *dev) -{ - /* - * PMC pci device gets hidden from PCI bus due to Silicon - * policy hence bind ACPI BASE aka ABASE (offset 0x20) with - * ESPI IO resources to ensure that ABASE falls under PCI reserved - * IO memory range. - * - * Note: Don't add any more resource with same offset 0x20 - * under this device space. - */ - pch_lpc_add_new_resource(dev, PCI_BASE_ADDRESS_4, - ACPI_BASE_ADDRESS, ACPI_BASE_SIZE, IORESOURCE_IO | - IORESOURCE_ASSIGNED | IORESOURCE_FIXED); -} - #endif diff --git a/src/soc/intel/tigerlake/include/soc/pmc.h b/src/soc/intel/tigerlake/include/soc/pmc.h index 3b58a74..7e66027f 100644 --- a/src/soc/intel/tigerlake/include/soc/pmc.h +++ b/src/soc/intel/tigerlake/include/soc/pmc.h @@ -3,6 +3,10 @@ #ifndef _SOC_TIGERLAKE_PMC_H_ #define _SOC_TIGERLAKE_PMC_H_
+#include <device/device.h> + +void soc_pmc_read_resources(struct device *dev); + /* PCI Configuration Space (D31:F2): PMC */ #define PWRMBASE 0x10 #define ABASE 0x20 diff --git a/src/soc/intel/tigerlake/pmc.c b/src/soc/intel/tigerlake/pmc.c index 136f103..c493ed6 100644 --- a/src/soc/intel/tigerlake/pmc.c +++ b/src/soc/intel/tigerlake/pmc.c @@ -91,6 +91,21 @@ config_deep_sx(config->deep_sx_config); }
+void soc_pmc_read_resources(struct device *dev) +{ + struct resource *res; + + /* Add the fixed MMIO resource */ + mmio_resource(dev, 0, PCH_PWRM_BASE_ADDRESS / KiB, PCH_PWRM_BASE_SIZE / KiB); + + /* Add the fixed I/O resource */ + res = new_resource(dev, 1); + res->base = (resource_t)ACPI_BASE_ADDRESS; + res->size = (resource_t)ACPI_BASE_SIZE; + res->limit = res->base + res->size - 1; + res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; +} + /* * Initialize PMC controller. * diff --git a/src/soc/intel/tigerlake/systemagent.c b/src/soc/intel/tigerlake/systemagent.c index 4cdca50..94caf9b 100644 --- a/src/soc/intel/tigerlake/systemagent.c +++ b/src/soc/intel/tigerlake/systemagent.c @@ -29,17 +29,6 @@ { EPBAR, EP_BASE_ADDRESS, EP_BASE_SIZE, "EPBAR" }, { REGBAR, REG_BASE_ADDRESS, REG_BASE_SIZE, "REGBAR" }, { EDRAMBAR, EDRAM_BASE_ADDRESS, EDRAM_BASE_SIZE, "EDRAMBAR" }, - /* - * PMC pci device gets hidden from PCI bus due to Silicon - * policy hence binding PMCBAR aka PWRMBASE (offset 0x10) with - * SA resources to ensure that PMCBAR falls under PCI reserved - * memory range. - * - * Note: Don't add any more resource with same offset 0x10 - * under this device space. - */ - { PCI_BASE_ADDRESS_0, PCH_PWRM_BASE_ADDRESS, PCH_PWRM_BASE_SIZE, - "PMCBAR" }, };
sa_add_fixed_mmio_resources(dev, index, soc_fixed_resources,
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified that the resource allocator is aware of and does not touch these two resources. Also verify that the MEM resource is described in the coreboot table ("e820"). Verified the memory range is also untouchable from Linux's point-of-view ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved").
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 5 files changed, 34 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 2:
If this looks OK I'll add all of the TGL boards to this commit (changing "on" to "hidden" for the PMC) so they aren't broken.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/chi... PS2, Line 155: pmc_ops could you set .init = pmc_init here and have it be executed instead of needing the BOOT_STATE_INIT_ENTRY?
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/pmc... File src/soc/intel/tigerlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/pmc... PS2, Line 94: soc_pmc_read_resources this ended up between pmc_init() and it's BOOT_STATE_INIT_ENTRY(), not necessarily a problem but usually good to keep those together.
Maybe doesn't matter if you could get rid of the BOOT_STATE_INIT_ENTRY entirely.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/chi... PS2, Line 155: pmc_ops
could you set . […]
Totally.
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/pmc... File src/soc/intel/tigerlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/pmc... PS2, Line 94: soc_pmc_read_resources
this ended up between pmc_init() and it's BOOT_STATE_INIT_ENTRY(), not necessarily a problem but usu […]
Whoops yeah, also moving pmc_init to .init of device_ops
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 50 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/chi... PS2, Line 155: pmc_ops
Totally.
Done
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/pmc... File src/soc/intel/tigerlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/41385/2/src/soc/intel/tigerlake/pmc... PS2, Line 94: soc_pmc_read_resources
Whoops yeah, also moving pmc_init to . […]
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 135: __weak void soc_pmc_read_resources(struct device *dev) {} Why is this weak?
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 154: static struct device_operations pmc_ops Why not just extern this here and make it a global object in pmc.c?
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 159: .acpi_name = &soc_acpi_name, We can define the pmc acpi_name funciton in pmc.c compilation unit and just return "PMC" from there.
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 46 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/4
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 135: __weak void soc_pmc_read_resources(struct device *dev) {}
Why is this weak?
That belied my intent to see if we can coalesce all the PMC handling under common/block, will remove for now until I see if that can be done.
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 154: static struct device_operations pmc_ops
Why not just extern this here and make it a global object in pmc. […]
Seems reasonable.
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 159: .acpi_name = &soc_acpi_name,
We can define the pmc acpi_name funciton in pmc.c compilation unit and just return "PMC" from there.
We already added PMC to soc_acpi_name for tigerlake; PCI devices usually get their ACPI name from soc_acpi_name, see 'struct device_operations pci_domain_ops' in various SoC chip.c files.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 159: .acpi_name = &soc_acpi_name,
We already added PMC to soc_acpi_name for tigerlake; PCI devices usually get their ACPI name from so […]
Yes, I'm familiar. However, there's no point in specifying soc_acpi_name() for the pmc device when it can provide it's own function or leave it to NULL and let it walk up the hierarchy (see acpi_device_name() in acpi/device.c). I personally prefer to make it explicit in the ops we're adding for device binding.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/chi... PS4, Line 20: soc_acpi_name I dislike that this is global and exposed.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/chi... PS4, Line 20: soc_acpi_name
I dislike that this is global and exposed.
it gets used by common/block/xhci, but it may not be needed there since it should fall up to the domain device.
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/pmc... File src/soc/intel/tigerlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/pmc... PS4, Line 114: #if CONFIG(HAVE_ACPI_TABLES) : .acpi_name = &soc_acpi_name, : #endif This may not be needed, it should attempt to call the function on the parent device (which should be pci_domain_ops) if this isn't defined here.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/chi... PS4, Line 20: soc_acpi_name
it gets used by common/block/xhci, but it may not be needed there since it should fall up to the dom […]
exactly
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 159: .acpi_name = &soc_acpi_name,
Yes, I'm familiar. […]
oops yeah I didn't see this comment on the previous patch...
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#5).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 43 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/41385/3/src/soc/intel/tigerlake/chi... PS3, Line 159: .acpi_name = &soc_acpi_name,
oops yeah I didn't see this comment on the previous patch...
Alright let's leave it NULL and let the hierarchy do the work. I'll also double check common/block/xhci .
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/pmc... File src/soc/intel/tigerlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/41385/4/src/soc/intel/tigerlake/pmc... PS4, Line 114: #if CONFIG(HAVE_ACPI_TABLES) : .acpi_name = &soc_acpi_name, : #endif
This may not be needed, it should attempt to call the function on the parent device (which should be […]
Ack
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#6).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/6
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#7).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/7
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/41385/7/src/soc/intel/tigerlake/pmc... File src/soc/intel/tigerlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/41385/7/src/soc/intel/tigerlake/pmc... PS7, Line 78: static void pmc_init(struct device *dev) The timing will be different when this is called. BS_DEV_INIT_CHIPS vs BS_DEV_INIT. You may want this toe be the enable() call in device_operations to make it earlier.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 7: Code-Review+2
Are you planning to push a similar change for JSL?
Also, time to drop PMC PCI_ID from https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/common/bloc...
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41385/7/src/soc/intel/tigerlake/pmc... File src/soc/intel/tigerlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/41385/7/src/soc/intel/tigerlake/pmc... PS7, Line 78: static void pmc_init(struct device *dev)
The timing will be different when this is called. BS_DEV_INIT_CHIPS vs BS_DEV_INIT. […]
Ack
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#8).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/8
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#9).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/9
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#10).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/10
Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Duncan Laurie, Paul Menzel, Nick Vaccaro, Angel Pons, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41385
to look at the new patch set (#11).
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41385/11
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 11: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 12:
Patch Set 7: Code-Review+2
Are you planning to push a similar change for JSL?
Also, time to drop PMC PCI_ID from https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/common/bloc...
Yes, I will do the same treatment for JSL. Also good catch there, the PCI driver will never bind, so may as well remove it.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 14: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 14: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
soc/intel/tigerlake: Move PMC PCI resources under PMC device
Historically in coreboot, the PMC's fixed PCI resources were described by the System Agent (the MMIO resource), and eSPI/LPC (the I/O resource). This patch moves both of those to a new Intel SoC-specific function, soc_pmc_read_resources(). On TGL, this new function takes care of providing the MMIO and I/O resources for the PMC.
BUG=b:156388055 TEST=verified on volteer that the resource allocator is aware of and does not touch these two resources: ("PCI: 00:1f.2 resource base fe000000 size 10000 align 0 gran 0 limit 0 flags f0000200 index 0 PCI: 00:1f.2 resource base 1800 size 100 align 0 gran 0 limit 18ff flags c0000100 index 1")
Also verify that the MEM resource is described in the coreboot table: ("BIOS-e820: [mem 0x00000000fe000000-0x00000000fe00ffff] reserved")
Verified the memory range is also untouchable from Linux: ("system 00:00: [mem 0xfe000000-0xffffffff] could not be reserved")
Change-Id: Ia7c6ae849aefaf549fb682416a87320907fb3fe3 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/41385 Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pmc.h M src/soc/intel/tigerlake/pmc.c M src/soc/intel/tigerlake/systemagent.c 9 files changed, 42 insertions(+), 46 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/deltaur/variants/baseboard/devicetree.cb b/src/mainboard/google/deltaur/variants/baseboard/devicetree.cb index 7350319..8d847f7 100644 --- a/src/mainboard/google/deltaur/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/deltaur/variants/baseboard/devicetree.cb @@ -305,7 +305,7 @@ end end # eSPI device pci 1f.1 off end # P2SB - device pci 1f.2 on end # PMC + device pci 1f.2 hidden end # PMC device pci 1f.3 on end # Intel HDA device pci 1f.4 on end # SMBus device pci 1f.5 on end # PCH SPI Flash Controller diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index fa99de8..5d5dcc4 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -370,7 +370,7 @@ register "irq" = "ACPI_IRQ_EDGE_LOW(GPP_C21_IRQ)" device spi 0 on end end - end # GSPI0 0xA0AA + end # GSPI0 0xA0AA device pci 1e.3 on chip drivers/spi/acpi register "name" = ""CRFP"" @@ -380,14 +380,14 @@ register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW_WAKE(GPP_C20)" device spi 0 on end end # FPMCU - end # GSPI1 0xA0AB + end # GSPI1 0xA0AB device pci 1f.0 on chip ec/google/chromeec device pnp 0c09.0 on end end - end # eSPI 0xA080 - A09F + end # eSPI 0xA080 - A09F device pci 1f.1 off end # P2SB 0xA0A0 - device pci 1f.2 on end # PMC 0xA0A1 + device pci 1f.2 hidden end # PMC 0xA0A1 device pci 1f.3 on chip drivers/generic/max98357a register "hid" = ""MX98357A"" @@ -395,7 +395,7 @@ register "sdmode_delay" = "5" device generic 0 on end end - end # Intel HD audio 0xA0C8-A0CF + end # Intel HD audio 0xA0C8-A0CF device pci 1f.4 off end # SMBus 0xA0A3 device pci 1f.5 on end # SPI 0xA0A4 device pci 1f.6 off end # GbE 0x15E1/0x15E2 diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb index 82f358e..045dc89 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb @@ -243,7 +243,7 @@ device pci 1e.3 off end # GSPI1 0xA0AB device pci 1f.0 on end # eSPI 0xA080 - A09F device pci 1f.1 on end # P2SB 0xA0A0 - device pci 1f.2 on end # PMC 0xA0A1 + device pci 1f.2 hidden end # PMC 0xA0A1 device pci 1f.3 on end # Intel HD audio 0xA0C8-A0CF device pci 1f.4 on end # SMBus 0xA0A3 device pci 1f.5 on end # SPI 0xA0A4 diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb index fec2fef..83b6c0a 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb @@ -239,7 +239,7 @@ device pci 1e.3 off end # GSPI1 0xA0AB device pci 1f.0 on end # eSPI 0xA080 - A09F device pci 1f.1 on end # P2SB 0xA0A0 - device pci 1f.2 on end # PMC 0xA0A1 + device pci 1f.2 hidden end # PMC 0xA0A1 device pci 1f.3 on end # Intel HD audio 0xA0C8-A0CF device pci 1f.4 on end # SMBus 0xA0A3 device pci 1f.5 on end # SPI 0xA0A4 diff --git a/src/soc/intel/tigerlake/chip.c b/src/soc/intel/tigerlake/chip.c index c1764cd..a923074 100644 --- a/src/soc/intel/tigerlake/chip.c +++ b/src/soc/intel/tigerlake/chip.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <console/console.h> #include <device/device.h> #include <device/pci.h> #include <fsp/api.h> @@ -151,11 +152,17 @@
static void soc_enable(struct device *dev) { - /* Set the operations if it is a special bus type */ + /* + * Set the operations if it is a special bus type or a hidden PCI + * device. + */ if (dev->path.type == DEVICE_PATH_DOMAIN) dev->ops = &pci_domain_ops; else if (dev->path.type == DEVICE_PATH_CPU_CLUSTER) dev->ops = &cpu_bus_ops; + else if (dev->path.type == DEVICE_PATH_PCI && + dev->path.pci.devfn == PCH_DEVFN_PMC) + dev->ops = &pmc_ops; }
struct chip_operations soc_intel_tigerlake_ops = { diff --git a/src/soc/intel/tigerlake/espi.c b/src/soc/intel/tigerlake/espi.c index ed8481a..8cc8303 100644 --- a/src/soc/intel/tigerlake/espi.c +++ b/src/soc/intel/tigerlake/espi.c @@ -197,21 +197,4 @@ soc_mirror_dmi_pcr_io_dec(); }
-/* Fill up ESPI IO resource structure inside SoC directory */ -void pch_lpc_soc_fill_io_resources(struct device *dev) -{ - /* - * PMC pci device gets hidden from PCI bus due to Silicon - * policy hence bind ACPI BASE aka ABASE (offset 0x20) with - * ESPI IO resources to ensure that ABASE falls under PCI reserved - * IO memory range. - * - * Note: Don't add any more resource with same offset 0x20 - * under this device space. - */ - pch_lpc_add_new_resource(dev, PCI_BASE_ADDRESS_4, - ACPI_BASE_ADDRESS, ACPI_BASE_SIZE, IORESOURCE_IO | - IORESOURCE_ASSIGNED | IORESOURCE_FIXED); -} - #endif diff --git a/src/soc/intel/tigerlake/include/soc/pmc.h b/src/soc/intel/tigerlake/include/soc/pmc.h index 3b58a74..0f72833 100644 --- a/src/soc/intel/tigerlake/include/soc/pmc.h +++ b/src/soc/intel/tigerlake/include/soc/pmc.h @@ -3,6 +3,10 @@ #ifndef _SOC_TIGERLAKE_PMC_H_ #define _SOC_TIGERLAKE_PMC_H_
+#include <device/device.h> + +extern struct device_operations pmc_ops; + /* PCI Configuration Space (D31:F2): PMC */ #define PWRMBASE 0x10 #define ABASE 0x20 diff --git a/src/soc/intel/tigerlake/pmc.c b/src/soc/intel/tigerlake/pmc.c index 136f103..fa59d46 100644 --- a/src/soc/intel/tigerlake/pmc.c +++ b/src/soc/intel/tigerlake/pmc.c @@ -75,7 +75,7 @@ write32(pmcbase + DSX_CFG, reg); }
-static void pmc_init(void *unused) +static void pmc_init(struct device *dev) { const config_t *config = config_of_soc();
@@ -91,11 +91,24 @@ config_deep_sx(config->deep_sx_config); }
-/* -* Initialize PMC controller. -* -* PMC controller gets hidden from PCI bus during FSP-Silicon init call. -* Hence PCI enumeration can't be used to initialize bus device and -* allocate resources. -*/ -BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_EXIT, pmc_init, NULL); +static void soc_pmc_read_resources(struct device *dev) +{ + struct resource *res; + + /* Add the fixed MMIO resource */ + mmio_resource(dev, 0, PCH_PWRM_BASE_ADDRESS / KiB, PCH_PWRM_BASE_SIZE / KiB); + + /* Add the fixed I/O resource */ + res = new_resource(dev, 1); + res->base = (resource_t)ACPI_BASE_ADDRESS; + res->size = (resource_t)ACPI_BASE_SIZE; + res->limit = res->base + res->size - 1; + res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; +} + +struct device_operations pmc_ops = { + .read_resources = soc_pmc_read_resources, + .set_resources = noop_set_resources, + .enable = pmc_init, + .scan_bus = scan_static_bus, +}; diff --git a/src/soc/intel/tigerlake/systemagent.c b/src/soc/intel/tigerlake/systemagent.c index 977c667..8c0a42a 100644 --- a/src/soc/intel/tigerlake/systemagent.c +++ b/src/soc/intel/tigerlake/systemagent.c @@ -32,17 +32,6 @@ { EPBAR, EP_BASE_ADDRESS, EP_BASE_SIZE, "EPBAR" }, { REGBAR, REG_BASE_ADDRESS, REG_BASE_SIZE, "REGBAR" }, { EDRAMBAR, EDRAM_BASE_ADDRESS, EDRAM_BASE_SIZE, "EDRAMBAR" }, - /* - * PMC pci device gets hidden from PCI bus due to Silicon - * policy hence binding PMCBAR aka PWRMBASE (offset 0x10) with - * SA resources to ensure that PMCBAR falls under PCI reserved - * memory range. - * - * Note: Don't add any more resource with same offset 0x10 - * under this device space. - */ - { PCI_BASE_ADDRESS_0, PCH_PWRM_BASE_ADDRESS, PCH_PWRM_BASE_SIZE, - "PMCBAR" }, };
sa_add_fixed_mmio_resources(dev, index, soc_fixed_resources,
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41385 )
Change subject: soc/intel/tigerlake: Move PMC PCI resources under PMC device ......................................................................
Patch Set 15:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3674 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3673 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3672 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3671
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)