Attention is currently required from: Zheng Bao.
Hello Zheng Bao,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/83646?usp=email
to review the following change.
Change subject: mb/google/skyrim: Combine the variants function ......................................................................
mb/google/skyrim: Combine the variants function
BUG=b:346716300 TEST=Build
Change-Id: I981e9c52c8e5fa32296e2e43be47411557133691 Signed-off-by: Zheng Bao fishbaozi@gmail.com --- R src/mainboard/google/skyrim/variants/baseboard/port_descriptors_combo.c M src/mainboard/google/skyrim/variants/markarth/Makefile.mk M src/mainboard/google/skyrim/variants/winterhold/Makefile.mk D src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c 4 files changed, 13 insertions(+), 68 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/83646/1
diff --git a/src/mainboard/google/skyrim/variants/markarth/port_descriptors.c b/src/mainboard/google/skyrim/variants/baseboard/port_descriptors_combo.c similarity index 77% rename from src/mainboard/google/skyrim/variants/markarth/port_descriptors.c rename to src/mainboard/google/skyrim/variants/baseboard/port_descriptors_combo.c index e78288a..6701890 100644 --- a/src/mainboard/google/skyrim/variants/markarth/port_descriptors.c +++ b/src/mainboard/google/skyrim/variants/baseboard/port_descriptors_combo.c @@ -5,9 +5,9 @@ #include <console/console.h> #include <soc/platform_descriptors.h>
-enum markarth_dxio_port_id { - MARKARTH_DXIO_WLAN, - MARKARTH_DXIO_STORAGE, +enum combo_dxio_port_id { + COMBO_DXIO_WLAN, + COMBO_DXIO_STORAGE, };
#define EMMC_DXIO_DESCRIPTOR { \ @@ -28,10 +28,10 @@
#define EMMC_CLKREQ_GPIO 115
-static fsp_dxio_descriptor markarth_dxio_descriptors[] = { - [MARKARTH_DXIO_WLAN] = WLAN_DXIO_DESCRIPTOR, +static fsp_dxio_descriptor combo_dxio_descriptors[] = { + [COMBO_DXIO_WLAN] = WLAN_DXIO_DESCRIPTOR, /* This value modified at runtime, default to emmc */ - [MARKARTH_DXIO_STORAGE] = EMMC_DXIO_DESCRIPTOR, + [COMBO_DXIO_STORAGE] = EMMC_DXIO_DESCRIPTOR, };
void variant_get_dxio_descriptors(const fsp_dxio_descriptor **dxio_descriptor, size_t *num) @@ -46,10 +46,10 @@ */ if (gpio_get(EMMC_CLKREQ_GPIO)) { printk(BIOS_DEBUG, "Enabling NVMe.\n"); - markarth_dxio_descriptors[MARKARTH_DXIO_STORAGE] = (fsp_dxio_descriptor)NVME_DXIO_DESCRIPTOR; + combo_dxio_descriptors[COMBO_DXIO_STORAGE] = (fsp_dxio_descriptor)NVME_DXIO_DESCRIPTOR; } else { printk(BIOS_DEBUG, "Defaulting to eMMC.\n"); } - *dxio_descriptor = markarth_dxio_descriptors; - *num = ARRAY_SIZE(markarth_dxio_descriptors); + *dxio_descriptor = combo_dxio_descriptors; + *num = ARRAY_SIZE(combo_dxio_descriptors); } diff --git a/src/mainboard/google/skyrim/variants/markarth/Makefile.mk b/src/mainboard/google/skyrim/variants/markarth/Makefile.mk index f9c705a..f797ca6 100644 --- a/src/mainboard/google/skyrim/variants/markarth/Makefile.mk +++ b/src/mainboard/google/skyrim/variants/markarth/Makefile.mk @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-or-later
subdirs-y += ./memory -romstage-y += port_descriptors.c +romstage-y += ../baseboard/port_descriptors_combo.c ramstage-y += gpio.c -ramstage-y += port_descriptors.c +ramstage-y += ../baseboard/port_descriptors_combo.c diff --git a/src/mainboard/google/skyrim/variants/winterhold/Makefile.mk b/src/mainboard/google/skyrim/variants/winterhold/Makefile.mk index 26b1ce2..14fb26a 100644 --- a/src/mainboard/google/skyrim/variants/winterhold/Makefile.mk +++ b/src/mainboard/google/skyrim/variants/winterhold/Makefile.mk @@ -2,7 +2,7 @@
subdirs-y += ./memory
-romstage-y += port_descriptors.c +romstage-y += ../baseboard/port_descriptors_combo.c
ramstage-y += gpio.c -ramstage-y += port_descriptors.c +ramstage-y += ../baseboard/port_descriptors_combo.c diff --git a/src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c b/src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c deleted file mode 100644 index 66a8c30..0000000 --- a/src/mainboard/google/skyrim/variants/winterhold/port_descriptors.c +++ /dev/null @@ -1,55 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <baseboard/variants.h> -#include <baseboard/port_descriptors.h> -#include <console/console.h> -#include <soc/platform_descriptors.h> - -enum winterhold_dxio_port_id { - WINTERHOLD_DXIO_WLAN, - WINTERHOLD_DXIO_STORAGE, -}; - -#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 - -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 - * signal. If the device is present, the clkreq is held low by the device. If - * no device is present, clkreq is pulled high by an external pull-up. - * - * This allows checking the state of the NVMe device clkreq signal and enabling - * either eMMC or NVMe based on that. - */ - if (gpio_get(EMMC_CLKREQ_GPIO)) { - printk(BIOS_DEBUG, "Enabling NVMe.\n"); - winterhold_dxio_descriptors[WINTERHOLD_DXIO_STORAGE] = (fsp_dxio_descriptor)NVME_DXIO_DESCRIPTOR; - } else { - printk(BIOS_DEBUG, "Defaulting to eMMC.\n"); - } - *dxio_descriptor = winterhold_dxio_descriptors; - *num = ARRAY_SIZE(winterhold_dxio_descriptors); -}
-
Bao Zheng (Code Review)