Robert Zieba has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/68138 )
Change subject: soc/amd/cezanne: Factor out common GPP clk req code ......................................................................
soc/amd/cezanne: Factor out common GPP clk req code
This commit factors out the `gpp_dxio_update_clk_req_config` function as it will be useful for other AMD SoCs.
BUG=b:250009974 TEST=Ran on nipperkin device, verified clk req settings match enabled devices
Change-Id: I9a4c72d8e980993c76a1b128f17b65b0db972a03 Signed-off-by: Robert Zieba robertzieba@google.com --- M src/soc/amd/cezanne/chip.h M src/soc/amd/cezanne/fch.c A src/soc/amd/common/block/include/amdblocks/pci_gpp.h M src/soc/amd/common/block/pci/pcie_gpp.c 4 files changed, 126 insertions(+), 94 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/68138/1
diff --git a/src/soc/amd/cezanne/chip.h b/src/soc/amd/cezanne/chip.h index 00eb560..14d4a4d 100644 --- a/src/soc/amd/cezanne/chip.h +++ b/src/soc/amd/cezanne/chip.h @@ -5,6 +5,7 @@
#include <amdblocks/chip.h> #include <amdblocks/i2c.h> +#include <amdblocks/pci_gpp.h> #include <gpio.h> #include <soc/i2c.h> #include <soc/southbridge.h> @@ -12,12 +13,6 @@ #include <types.h> #include <vendorcode/amd/fsp/cezanne/FspUsb.h>
-enum gpp_clk_req { - GPP_CLK_ON, /* GPP clock always on; default */ - GPP_CLK_REQ, /* GPP clock controlled by corresponding #CLK_REQx pin */ - GPP_CLK_OFF, /* GPP clk off */ -}; - struct soc_amd_cezanne_config { struct soc_amd_common_config common_config; u8 i2c_scl_reset; diff --git a/src/soc/amd/cezanne/fch.c b/src/soc/amd/cezanne/fch.c index b791037..5009fc6 100644 --- a/src/soc/amd/cezanne/fch.c +++ b/src/soc/amd/cezanne/fch.c @@ -3,6 +3,7 @@ #include <amdblocks/acpi.h> #include <amdblocks/acpimmio.h> #include <amdblocks/amd_pci_util.h> +#include <amdblocks/pci_gpp.h> #include <amdblocks/gpio.h> #include <amdblocks/i2c.h> #include <amdblocks/smi.h> @@ -134,93 +135,6 @@ pm_write16(PWR_RESET_CFG, pm_read16(PWR_RESET_CFG) | TOGGLE_ALL_PWR_GOOD); }
-/* Update gpp glk req config based on DXIO descriptors and enabled devices. */ -static void gpp_dxio_update_clk_req_config(enum gpp_clk_req *gpp_clk_config, - size_t gpp_clk_config_num) -{ - const fsp_dxio_descriptor *dxio_descs = NULL; - const fsp_ddi_descriptor *ddi_descs = NULL; - size_t dxio_num = 0; - size_t ddi_num = 0; - - mainboard_get_dxio_ddi_descriptors(&dxio_descs, &dxio_num, &ddi_descs, &ddi_num); - if (dxio_descs == NULL) { - printk(BIOS_WARNING, - "No DXIO descriptors found, GPP clk req may not reflect enabled devices\n"); - return; - } - - for (int i = 0; i < dxio_num; i++) { - const fsp_dxio_descriptor *dxio_desc = &dxio_descs[i]; - - /* Only consider PCIe and unused engine types. */ - if (dxio_desc->engine_type != PCIE_ENGINE - && dxio_desc->engine_type != UNUSED_ENGINE) - continue; - enum cpm_clk_req dxio_clk_req = dxio_desc->clk_req; - - /* CLK_DISABLE means there's no corresponding clk req line in use */ - if (dxio_clk_req == CLK_DISABLE) - continue; - - /* - * dxio_clk_req is only 4 bits so having CLK_ENABLE as a value for - * a descriptor should cause a compiler error. 0xF isn't a - * valid clk_req value according to AMD's internal code either. - * This is here to draw attention in case this code is ever used - * in a situation where this has changed. - */ - if (dxio_clk_req == (CLK_ENABLE & 0xF)) { - printk(BIOS_WARNING, - "CLK_ENABLE is an invalid clk_req value for PCIe device %d.%d, DXIO descriptor %d\n", - dxio_desc->device_number, dxio_desc->function_number, i); - continue; - } - - /* cpm_clk_req 0 is CLK_DISABLE */ - int gpp_req_index = dxio_clk_req - CLK_REQ0; - /* Ensure that our index is valid */ - if (gpp_req_index < 0 || gpp_req_index >= gpp_clk_config_num) { - printk(BIOS_ERR, "Failed to convert DXIO clk req value %d to GPP clk req index for PCIe device %d.%d, DXIO descriptor %d, clk req settings may be incorrect\n", - dxio_clk_req, dxio_desc->device_number, - dxio_desc->function_number, i); - continue; - } - - const struct device *pci_device = pcidev_path_on_root( - PCI_DEVFN(dxio_desc->device_number, dxio_desc->function_number)); - if (pci_device == NULL) { - gpp_clk_config[gpp_req_index] = GPP_CLK_OFF; - printk(BIOS_WARNING, - "Cannot find PCIe device %d.%d, disabling GPP clk req %d, DXIO descriptor %d\n", - dxio_desc->device_number, dxio_desc->function_number, i, - gpp_req_index); - continue; - } - - /* PCIe devices haven't been fully set up yet, so directly read the vendor id - * and device id to determine if a device is physically present. If a device - * is not present then the id should be 0xffffffff. 0x00000000, 0xffff0000, - * and 0x0000ffff are there to account for any odd failure cases. */ - u32 id = pci_read_config32(pci_device, PCI_VENDOR_ID); - bool enabled = pci_device->enabled && (id != 0xffffffff) && - (id != 0x00000000) && (id != 0x0000ffff) && (id != 0xffff0000); - - /* Inform of possible mismatches between devices and SoC gpp_clk_config. */ - if (!enabled && gpp_clk_config[gpp_req_index] != GPP_CLK_OFF) { - gpp_clk_config[gpp_req_index] = GPP_CLK_OFF; - printk(BIOS_INFO, - "PCIe device %d.%d disabled, disabling GPP clk req %d, DXIO descriptor %d\n", - dxio_desc->device_number, dxio_desc->function_number, - gpp_req_index, i); - } else if (enabled && gpp_clk_config[gpp_req_index] == GPP_CLK_OFF) { - printk(BIOS_INFO, - "PCIe device %d.%d enabled, GPP clk req is off, DXIO descriptor %d\n", - dxio_desc->device_number, dxio_desc->function_number, i); - } - } -} - /* Configure the general purpose PCIe clock outputs according to the devicetree settings */ static void gpp_clk_setup(void) { @@ -239,7 +153,7 @@
uint32_t gpp_clk_ctl = misc_read32(GPP_CLK_CNTRL);
- gpp_dxio_update_clk_req_config(&cfg->gpp_clk_config[0], GPP_CLK_OUTPUT_COUNT); + pcie_gpp_dxio_update_clk_req_config(&cfg->gpp_clk_config[0], GPP_CLK_OUTPUT_COUNT); for (int i = 0; i < GPP_CLK_OUTPUT_COUNT; i++) { gpp_clk_ctl &= ~GPP_CLK_REQ_MASK(gpp_clk_shift_lut[i]);
diff --git a/src/soc/amd/common/block/include/amdblocks/pci_gpp.h b/src/soc/amd/common/block/include/amdblocks/pci_gpp.h new file mode 100644 index 0000000..e205883 --- /dev/null +++ b/src/soc/amd/common/block/include/amdblocks/pci_gpp.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef AMD_BLOCK_PCI_GPP_H +#define AMD_BLOCK_PCI_GPP_H + +#include <soc/platform_descriptors.h> + +enum gpp_clk_req { + GPP_CLK_ON, /* GPP clock always on; default */ + GPP_CLK_REQ, /* GPP clock controlled by corresponding #CLK_REQx pin */ + GPP_CLK_OFF, /* GPP clk off */ +}; + +void pcie_gpp_dxio_update_clk_req_config(enum gpp_clk_req *gpp_clk_config, + size_t gpp_clk_config_num); + +#endif \ No newline at end of file diff --git a/src/soc/amd/common/block/pci/pcie_gpp.c b/src/soc/amd/common/block/pci/pcie_gpp.c index 2e37935..a222bdb 100644 --- a/src/soc/amd/common/block/pci/pcie_gpp.c +++ b/src/soc/amd/common/block/pci/pcie_gpp.c @@ -4,12 +4,14 @@ #include <acpi/acpigen.h> #include <acpi/acpigen_pci.h> #include <amdblocks/amd_pci_util.h> +#include <amdblocks/pci_gpp.h> #include <assert.h> #include <device/device.h> #include <device/pci.h> #include <device/pci_ids.h> #include <device/pciexp.h> #include <soc/pci_devs.h> +#include <soc/platform_descriptors.h> #include <stdio.h> #include <stdlib.h>
@@ -48,6 +50,93 @@ acpigen_pop_len(); /* Scope */ }
+/* Update gpp glk req config based on DXIO descriptors and enabled devices. */ +void pcie_gpp_dxio_update_clk_req_config(enum gpp_clk_req *gpp_clk_config, + size_t gpp_clk_config_num) +{ + const fsp_dxio_descriptor *dxio_descs = NULL; + const fsp_ddi_descriptor *ddi_descs = NULL; + size_t dxio_num = 0; + size_t ddi_num = 0; + + mainboard_get_dxio_ddi_descriptors(&dxio_descs, &dxio_num, &ddi_descs, &ddi_num); + if (dxio_descs == NULL) { + printk(BIOS_WARNING, + "No DXIO descriptors found, GPP clk req may not reflect enabled devices\n"); + return; + } + + for (int i = 0; i < dxio_num; i++) { + const fsp_dxio_descriptor *dxio_desc = &dxio_descs[i]; + + /* Only consider PCIe and unused engine types. */ + if (dxio_desc->engine_type != PCIE_ENGINE + && dxio_desc->engine_type != UNUSED_ENGINE) + continue; + enum cpm_clk_req dxio_clk_req = dxio_desc->clk_req; + + /* CLK_DISABLE means there's no corresponding clk req line in use */ + if (dxio_clk_req == CLK_DISABLE) + continue; + + /* + * dxio_clk_req is only 4 bits so having CLK_ENABLE as a value for + * a descriptor should cause a compiler error. 0xF isn't a + * valid clk_req value according to AMD's internal code either. + * This is here to draw attention in case this code is ever used + * in a situation where this has changed. + */ + if (dxio_clk_req == (CLK_ENABLE & 0xF)) { + printk(BIOS_WARNING, + "CLK_ENABLE is an invalid clk_req value for PCIe device %d.%d, DXIO descriptor %d\n", + dxio_desc->device_number, dxio_desc->function_number, i); + continue; + } + + /* cpm_clk_req 0 is CLK_DISABLE */ + int gpp_req_index = dxio_clk_req - CLK_REQ0; + /* Ensure that our index is valid */ + if (gpp_req_index < 0 || gpp_req_index >= gpp_clk_config_num) { + printk(BIOS_ERR, "Failed to convert DXIO clk req value %d to GPP clk req index for PCIe device %d.%d, DXIO descriptor %d, clk req settings may be incorrect\n", + dxio_clk_req, dxio_desc->device_number, + dxio_desc->function_number, i); + continue; + } + + const struct device *pci_device = pcidev_path_on_root( + PCI_DEVFN(dxio_desc->device_number, dxio_desc->function_number)); + if (pci_device == NULL) { + gpp_clk_config[gpp_req_index] = GPP_CLK_OFF; + printk(BIOS_WARNING, + "Cannot find PCIe device %d.%d, disabling GPP clk req %d, DXIO descriptor %d\n", + dxio_desc->device_number, dxio_desc->function_number, i, + gpp_req_index); + continue; + } + + /* PCIe devices haven't been fully set up yet, so directly read the vendor id + * and device id to determine if a device is physically present. If a device + * is not present then the id should be 0xffffffff. 0x00000000, 0xffff0000, + * and 0x0000ffff are there to account for any odd failure cases. */ + u32 id = pci_read_config32(pci_device, PCI_VENDOR_ID); + bool enabled = pci_device->enabled && (id != 0xffffffff) && + (id != 0x00000000) && (id != 0x0000ffff) && (id != 0xffff0000); + + /* Inform of possible mismatches between devices and SoC gpp_clk_config. */ + if (!enabled && gpp_clk_config[gpp_req_index] != GPP_CLK_OFF) { + gpp_clk_config[gpp_req_index] = GPP_CLK_OFF; + printk(BIOS_INFO, + "PCIe device %d.%d disabled, disabling GPP clk req %d, DXIO descriptor %d\n", + dxio_desc->device_number, dxio_desc->function_number, + gpp_req_index, i); + } else if (enabled && gpp_clk_config[gpp_req_index] == GPP_CLK_OFF) { + printk(BIOS_INFO, + "PCIe device %d.%d enabled, GPP clk req is off, DXIO descriptor %d\n", + dxio_desc->device_number, dxio_desc->function_number, i); + } + } +} + static struct device_operations internal_pcie_gpp_ops = { .read_resources = pci_bus_read_resources, .set_resources = pci_dev_set_resources,
-
Robert Zieba (Code Review)