Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79627?usp=email )
Change subject: mb/google/skyrim: Update DXIO descriptor definition ......................................................................
mb/google/skyrim: Update DXIO descriptor definition
Update definition to be more intuitive and extensible. Port descriptors will be defined as individual entities and added to the descriptor list as such.
BUG=b:281059446 TEST=builds
Change-Id: Ic5a06a7d1bdb9123a0a242a571f094ac3233d7b2 Signed-off-by: Jon Murphy jpmurphy@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/79627 Reviewed-by: Tim Van Patten timvp@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Eric Lai ericllai@google.com --- M src/mainboard/google/skyrim/Kconfig M src/mainboard/google/skyrim/port_descriptors.c M src/mainboard/google/skyrim/variants/baseboard/Makefile.inc A src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h M src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/skyrim/variants/baseboard/port_descriptors.c M src/mainboard/google/skyrim/variants/markarth/port_descriptors.c M src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c 8 files changed, 148 insertions(+), 217 deletions(-)
Approvals: Tim Van Patten: Looks good to me, approved Raul Rangel: Looks good to me, approved Eric Lai: Looks good to me, approved build bot (Jenkins): Verified
diff --git a/src/mainboard/google/skyrim/Kconfig b/src/mainboard/google/skyrim/Kconfig index 98d13dd..dc28fbc 100644 --- a/src/mainboard/google/skyrim/Kconfig +++ b/src/mainboard/google/skyrim/Kconfig @@ -141,14 +141,6 @@ default "skyrim" if BOARD_GOOGLE_SKYRIM default "winterhold" if BOARD_GOOGLE_WINTERHOLD
-config USE_VARIANT_DXIO_DESCRIPTOR - bool - default y if BOARD_GOOGLE_MARKARTH || BOARD_GOOGLE_WINTERHOLD - default n - help - Enable this to allow a variant to override the dxio descriptor values - in port_descriptors.c - config USE_SELECTIVE_GOP_INIT default y if CHROMEOS && RUN_FSP_GOP && BOARD_GOOGLE_SKYRIM
diff --git a/src/mainboard/google/skyrim/port_descriptors.c b/src/mainboard/google/skyrim/port_descriptors.c index 5a9539f..c2a019f 100644 --- a/src/mainboard/google/skyrim/port_descriptors.c +++ b/src/mainboard/google/skyrim/port_descriptors.c @@ -5,57 +5,6 @@ #include <soc/platform_descriptors.h> #include <types.h>
-#if CONFIG(USE_VARIANT_DXIO_DESCRIPTOR) -static const fsp_dxio_descriptor skyrim_mdn_dxio_descriptors[] = {}; -#else -static const fsp_dxio_descriptor skyrim_mdn_dxio_descriptors[] = { - { /* WLAN */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 0, - .end_logical_lane = 0, - .device_number = PCI_SLOT(WLAN_DEVFN), - .function_number = PCI_FUNC(WLAN_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .clk_req = CLK_REQ2, - }, - { /* SD */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 1, - .end_logical_lane = 1, - .device_number = PCI_SLOT(SD_DEVFN), - .function_number = PCI_FUNC(SD_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_hotplug = HOTPLUG_ENHANCED, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .gpio_group_id = GPIO_27, - .clk_req = CLK_REQ1, - }, - { /* SSD */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 2, - .end_logical_lane = 3, - .device_number = PCI_SLOT(NVME_DEVFN), - .function_number = PCI_FUNC(NVME_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .gpio_group_id = GPIO_6, - .clk_req = CLK_REQ0, - }, -}; -#endif - static const fsp_ddi_descriptor skyrim_mdn_ddi_descriptors[] = { { /* DDI0 - eDP */ .connector_type = DDI_EDP, @@ -88,12 +37,7 @@ const fsp_dxio_descriptor **dxio_descs, size_t *dxio_num, const fsp_ddi_descriptor **ddi_descs, size_t *ddi_num) { - if (CONFIG(USE_VARIANT_DXIO_DESCRIPTOR)) { - variant_get_dxio_descriptor(dxio_descs, dxio_num); - } else { - *dxio_descs = skyrim_mdn_dxio_descriptors; - *dxio_num = ARRAY_SIZE(skyrim_mdn_dxio_descriptors); - } + variant_get_dxio_descriptors(dxio_descs, dxio_num);
*ddi_descs = skyrim_mdn_ddi_descriptors; *ddi_num = ARRAY_SIZE(skyrim_mdn_ddi_descriptors); diff --git a/src/mainboard/google/skyrim/variants/baseboard/Makefile.inc b/src/mainboard/google/skyrim/variants/baseboard/Makefile.inc index 0f7cc89..e76a9df 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/Makefile.inc +++ b/src/mainboard/google/skyrim/variants/baseboard/Makefile.inc @@ -3,8 +3,10 @@ bootblock-y += gpio.c
romstage-y += gpio.c +romstage-y += port_descriptors.c
ramstage-y += gpio.c +ramstage-y += port_descriptors.c
verstage-$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK) += gpio.c
diff --git a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h new file mode 100644 index 0000000..40da0b4 --- /dev/null +++ b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h @@ -0,0 +1,55 @@ +#ifndef __BASEBOARD_PORT_DESCRIPTORS_H__ +#define __BASEBOARD_PORT_DESCRIPTORS_H__ + +#define WLAN_DEVFN PCIE_GPP_2_0_DEVFN +#define SD_DEVFN PCIE_GPP_2_1_DEVFN +#define NVME_DEVFN PCIE_GPP_2_2_DEVFN + +#define WLAN_DXIO_DESCRIPTOR { \ + .engine_type = PCIE_ENGINE, \ + .port_present = true, \ + .start_logical_lane = 0, \ + .end_logical_lane = 0, \ + .device_number = PCI_SLOT(WLAN_DEVFN), \ + .function_number = PCI_FUNC(WLAN_DEVFN), \ + .link_speed_capability = GEN3, \ + .turn_off_unused_lanes = true, \ + .link_aspm = ASPM_L1, \ + .link_aspm_L1_1 = true, \ + .link_aspm_L1_2 = true, \ + .clk_req = CLK_REQ2, \ +} + +#define SD_DXIO_DESCRIPTOR { \ + .engine_type = PCIE_ENGINE, \ + .port_present = true, \ + .start_logical_lane = 1, \ + .end_logical_lane = 1, \ + .device_number = PCI_SLOT(SD_DEVFN), \ + .function_number = PCI_FUNC(SD_DEVFN), \ + .link_speed_capability = GEN3, \ + .turn_off_unused_lanes = true, \ + .link_hotplug = HOTPLUG_ENHANCED, \ + .link_aspm = ASPM_L1, \ + .link_aspm_L1_1 = true, \ + .gpio_group_id = GPIO_27, \ + .clk_req = CLK_REQ1, \ +} + +#define NVME_DXIO_DESCRIPTOR { \ + .engine_type = PCIE_ENGINE, \ + .port_present = true, \ + .start_logical_lane = 2, \ + .end_logical_lane = 3, \ + .device_number = PCI_SLOT(NVME_DEVFN), \ + .function_number = PCI_FUNC(NVME_DEVFN), \ + .link_speed_capability = GEN3, \ + .turn_off_unused_lanes = true, \ + .link_aspm = ASPM_L1, \ + .link_aspm_L1_1 = true, \ + .link_aspm_L1_2 = true, \ + .gpio_group_id = GPIO_6, \ + .clk_req = CLK_REQ0, \ +} + +#endif //__BASEBOARD_PORT_DESCRIPTORS_H__ diff --git a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h index e49b7c4..f3bdf4c 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h @@ -7,10 +7,6 @@ #include <soc/pci_devs.h> #include <platform_descriptors.h>
-#define WLAN_DEVFN PCIE_GPP_2_0_DEVFN -#define SD_DEVFN PCIE_GPP_2_1_DEVFN -#define NVME_DEVFN PCIE_GPP_2_2_DEVFN - /* This function provides base GPIO configuration table. */ void baseboard_gpio_table(const struct soc_amd_gpio **gpio, size_t *size);
@@ -41,7 +37,10 @@ /* This function allows variant to override any GPIO init in romstage. */ void variant_romstage_override_gpio_table(const struct soc_amd_gpio **gpio, size_t *size);
-/* Allow variants to override the DXIO Descriptors */ -void variant_get_dxio_descriptor(const fsp_dxio_descriptor **dxio_descs, size_t *dxio_num); +/* + * This function allows a variant to override dxio descriptors passed to the FSP. + */ +void variant_get_dxio_descriptors(const fsp_dxio_descriptor **dxio_descriptor, + size_t *num);
#endif /* __BASEBOARD_VARIANTS_H__ */ diff --git a/src/mainboard/google/skyrim/variants/baseboard/port_descriptors.c b/src/mainboard/google/skyrim/variants/baseboard/port_descriptors.c new file mode 100644 index 0000000..296bb18 --- /dev/null +++ b/src/mainboard/google/skyrim/variants/baseboard/port_descriptors.c @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <baseboard/variants.h> +#include <baseboard/port_descriptors.h> +#include <soc/platform_descriptors.h> + +enum baseboard_dxio_port_id { + BASEBOARD_DXIO_WLAN, + BASEBOARD_DXIO_SD, + BASEBOARD_DXIO_STORAGE, +}; + +static fsp_dxio_descriptor skyrim_mdn_dxio_descriptors[] = { + [BASEBOARD_DXIO_WLAN] = WLAN_DXIO_DESCRIPTOR, + [BASEBOARD_DXIO_SD] = SD_DXIO_DESCRIPTOR, + [BASEBOARD_DXIO_STORAGE] = NVME_DXIO_DESCRIPTOR, +}; + +__weak void variant_get_dxio_descriptors(const fsp_dxio_descriptor **dxio_descriptor, size_t *num) +{ + *dxio_descriptor = skyrim_mdn_dxio_descriptors; + *num = ARRAY_SIZE(skyrim_mdn_dxio_descriptors); +} diff --git a/src/mainboard/google/skyrim/variants/markarth/port_descriptors.c b/src/mainboard/google/skyrim/variants/markarth/port_descriptors.c index 94eb465..e78288a 100644 --- a/src/mainboard/google/skyrim/variants/markarth/port_descriptors.c +++ b/src/mainboard/google/skyrim/variants/markarth/port_descriptors.c @@ -1,81 +1,40 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <baseboard/variants.h> +#include <baseboard/port_descriptors.h> #include <console/console.h> -#include <gpio.h> #include <soc/platform_descriptors.h> -#include <types.h>
-static const fsp_dxio_descriptor emmc_dxio_descriptors[] = { - { - /* WLAN */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 0, - .end_logical_lane = 0, - .device_number = PCI_SLOT(WLAN_DEVFN), - .function_number = PCI_FUNC(WLAN_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .clk_req = CLK_REQ2, - }, - { - /* eMMC */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 1, - .end_logical_lane = 1, - .device_number = PCI_SLOT(SD_DEVFN), - .function_number = PCI_FUNC(SD_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .gpio_group_id = GPIO_6, - .clk_req = CLK_REQ1, - }, +enum markarth_dxio_port_id { + MARKARTH_DXIO_WLAN, + MARKARTH_DXIO_STORAGE, };
-static const fsp_dxio_descriptor nvme_dxio_descriptors[] = { - { - /* WLAN */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 0, - .end_logical_lane = 0, - .device_number = PCI_SLOT(WLAN_DEVFN), - .function_number = PCI_FUNC(WLAN_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .clk_req = CLK_REQ2, - }, - { - /* SSD */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 2, - .end_logical_lane = 3, - .device_number = PCI_SLOT(NVME_DEVFN), - .function_number = PCI_FUNC(NVME_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .gpio_group_id = GPIO_6, - .clk_req = CLK_REQ0, - }, -}; +#define EMMC_DXIO_DESCRIPTOR { \ + .engine_type = PCIE_ENGINE, \ + .port_present = true, \ + .start_logical_lane = 1, \ + .end_logical_lane = 1, \ + .device_number = PCI_SLOT(SD_DEVFN), \ + .function_number = PCI_FUNC(SD_DEVFN), \ + .link_speed_capability = GEN3, \ + .turn_off_unused_lanes = true, \ + .link_aspm = ASPM_L1, \ + .link_aspm_L1_1 = true, \ + .link_aspm_L1_2 = true, \ + .gpio_group_id = GPIO_6, \ + .clk_req = CLK_REQ1, \ +}
#define EMMC_CLKREQ_GPIO 115 -void variant_get_dxio_descriptor(const fsp_dxio_descriptor **dxio_descs, size_t *dxio_num) + +static fsp_dxio_descriptor markarth_dxio_descriptors[] = { + [MARKARTH_DXIO_WLAN] = WLAN_DXIO_DESCRIPTOR, + /* This value modified at runtime, default to emmc */ + [MARKARTH_DXIO_STORAGE] = EMMC_DXIO_DESCRIPTOR, +}; + +void variant_get_dxio_descriptors(const fsp_dxio_descriptor **dxio_descriptor, size_t *num) { /* * We can determine if a device is populated based on the state of the clkreq @@ -87,11 +46,10 @@ */ if (gpio_get(EMMC_CLKREQ_GPIO)) { printk(BIOS_DEBUG, "Enabling NVMe.\n"); - *dxio_num = ARRAY_SIZE(nvme_dxio_descriptors); - *dxio_descs = nvme_dxio_descriptors; + markarth_dxio_descriptors[MARKARTH_DXIO_STORAGE] = (fsp_dxio_descriptor)NVME_DXIO_DESCRIPTOR; } else { - printk(BIOS_DEBUG, "Enabling eMMC.\n"); - *dxio_num = ARRAY_SIZE(emmc_dxio_descriptors); - *dxio_descs = emmc_dxio_descriptors; + printk(BIOS_DEBUG, "Defaulting to eMMC.\n"); } + *dxio_descriptor = markarth_dxio_descriptors; + *num = ARRAY_SIZE(markarth_dxio_descriptors); } diff --git a/src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c b/src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c index 94eb465..66a8c30 100644 --- a/src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c +++ b/src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c @@ -1,81 +1,40 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <baseboard/variants.h> +#include <baseboard/port_descriptors.h> #include <console/console.h> -#include <gpio.h> #include <soc/platform_descriptors.h> -#include <types.h>
-static const fsp_dxio_descriptor emmc_dxio_descriptors[] = { - { - /* WLAN */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 0, - .end_logical_lane = 0, - .device_number = PCI_SLOT(WLAN_DEVFN), - .function_number = PCI_FUNC(WLAN_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .clk_req = CLK_REQ2, - }, - { - /* eMMC */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 1, - .end_logical_lane = 1, - .device_number = PCI_SLOT(SD_DEVFN), - .function_number = PCI_FUNC(SD_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .gpio_group_id = GPIO_6, - .clk_req = CLK_REQ1, - }, +enum winterhold_dxio_port_id { + WINTERHOLD_DXIO_WLAN, + WINTERHOLD_DXIO_STORAGE, };
-static const fsp_dxio_descriptor nvme_dxio_descriptors[] = { - { - /* WLAN */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 0, - .end_logical_lane = 0, - .device_number = PCI_SLOT(WLAN_DEVFN), - .function_number = PCI_FUNC(WLAN_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .clk_req = CLK_REQ2, - }, - { - /* SSD */ - .engine_type = PCIE_ENGINE, - .port_present = true, - .start_logical_lane = 2, - .end_logical_lane = 3, - .device_number = PCI_SLOT(NVME_DEVFN), - .function_number = PCI_FUNC(NVME_DEVFN), - .link_speed_capability = GEN3, - .turn_off_unused_lanes = true, - .link_aspm = ASPM_L1, - .link_aspm_L1_1 = true, - .link_aspm_L1_2 = true, - .gpio_group_id = GPIO_6, - .clk_req = CLK_REQ0, - }, -}; +#define EMMC_DXIO_DESCRIPTOR { \ + .engine_type = PCIE_ENGINE, \ + .port_present = true, \ + .start_logical_lane = 1, \ + .end_logical_lane = 1, \ + .device_number = PCI_SLOT(SD_DEVFN), \ + .function_number = PCI_FUNC(SD_DEVFN), \ + .link_speed_capability = GEN3, \ + .turn_off_unused_lanes = true, \ + .link_aspm = ASPM_L1, \ + .link_aspm_L1_1 = true, \ + .link_aspm_L1_2 = true, \ + .gpio_group_id = GPIO_6, \ + .clk_req = CLK_REQ1, \ +}
#define EMMC_CLKREQ_GPIO 115 -void variant_get_dxio_descriptor(const fsp_dxio_descriptor **dxio_descs, size_t *dxio_num) + +static fsp_dxio_descriptor winterhold_dxio_descriptors[] = { + [WINTERHOLD_DXIO_WLAN] = WLAN_DXIO_DESCRIPTOR, + /* This value modified at runtime, default to emmc */ + [WINTERHOLD_DXIO_STORAGE] = EMMC_DXIO_DESCRIPTOR, +}; + +void variant_get_dxio_descriptors(const fsp_dxio_descriptor **dxio_descriptor, size_t *num) { /* * We can determine if a device is populated based on the state of the clkreq @@ -87,11 +46,10 @@ */ if (gpio_get(EMMC_CLKREQ_GPIO)) { printk(BIOS_DEBUG, "Enabling NVMe.\n"); - *dxio_num = ARRAY_SIZE(nvme_dxio_descriptors); - *dxio_descs = nvme_dxio_descriptors; + winterhold_dxio_descriptors[WINTERHOLD_DXIO_STORAGE] = (fsp_dxio_descriptor)NVME_DXIO_DESCRIPTOR; } else { - printk(BIOS_DEBUG, "Enabling eMMC.\n"); - *dxio_num = ARRAY_SIZE(emmc_dxio_descriptors); - *dxio_descs = emmc_dxio_descriptors; + printk(BIOS_DEBUG, "Defaulting to eMMC.\n"); } + *dxio_descriptor = winterhold_dxio_descriptors; + *num = ARRAY_SIZE(winterhold_dxio_descriptors); }
-
Shelley Chen (Code Review)