Karthik Ramasubramanian has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58705 )
Change subject: mb/google/guybrush: Define ACPI Power Resources for FPMCU ......................................................................
mb/google/guybrush: Define ACPI Power Resources for FPMCU
Currently all the power sequencing for FPMCU is done explicitly in different stages of coreboot. This can all be done by adding ACPI power resources for FPMCU and clean up the unused code. Here is the expected power sequence: PowerUp : Assert EN_PWR_FP -> 3 ms delay -> De-assert FPMCU_RST_ODL Shutdown : De-assert EN_PWR_FP -> Assert FPMCU_RST_ODL Reboot : Shutdown -> 200 ms delay -> PowerUp
BUG=None TEST=Build and boot to OS in Guybrush. Ensure that the FP is able to unlock the system after the first login attempt. Ensure that the FP is able to wakeup the system. Observed that the power resource is added correctly in the FPMCU ACPI object Name (_PR0, Package (0x01) // _PR0: Power Resources for D0 { PR01 }) Name (_PR3, Package (0x01) // _PR3: Power Resources for D3hot { PR01 }) PowerResource (PR01, 0x00, 0x0000) { Method (_STA, 0, NotSerialized) // _STA: Status { Return (0x01) }
Method (_ON, 0, Serialized) // _ON_: Power On { _SB.CTXS (0x0B) _SB.STXS (0x20) _SB.STXS (0x0B) }
Method (_OFF, 0, Serialized) // _OFF: Power Off { _SB.CTXS (0x0B) _SB.CTXS (0x20) } }
Change-Id: I52322eaecf6961ff9a196ca9ab2d58b7d4599d4f Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/58705 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/guybrush/bootblock.c M src/mainboard/google/guybrush/mainboard.c M src/mainboard/google/guybrush/variants/baseboard/gpio.c M src/mainboard/google/guybrush/variants/baseboard/helpers.c M src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/guybrush/variants/guybrush/gpio.c M src/mainboard/google/guybrush/variants/guybrush/overridetree.cb M src/mainboard/google/guybrush/variants/nipperkin/gpio.c M src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb M src/mainboard/google/guybrush/variants/nipperkin/ramstage.c 10 files changed, 35 insertions(+), 180 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Rob Barnes: Looks good to me, approved
diff --git a/src/mainboard/google/guybrush/bootblock.c b/src/mainboard/google/guybrush/bootblock.c index 187b2ef..bffcb0c 100644 --- a/src/mainboard/google/guybrush/bootblock.c +++ b/src/mainboard/google/guybrush/bootblock.c @@ -99,8 +99,4 @@
gpio_configure_pads_with_override(base_gpios, base_num_gpios, override_gpios, override_num_gpios); - - /* FPMCU check needs to happen after EC initialization for FW_CONFIG bits */ - if (variant_has_fpmcu()) - variant_fpmcu_reset(); } diff --git a/src/mainboard/google/guybrush/mainboard.c b/src/mainboard/google/guybrush/mainboard.c index b2d669c..c50304d 100644 --- a/src/mainboard/google/guybrush/mainboard.c +++ b/src/mainboard/google/guybrush/mainboard.c @@ -209,13 +209,7 @@ pm_write32(PM_ESPI_INTR_CTRL, PM_ESPI_DEV_INTR_MASK & ~(BIT(1))); }
-static void mainboard_final(void *chip_info) -{ - variant_finalize_gpios(); -} - struct chip_operations mainboard_ops = { .init = mainboard_init, .enable_dev = mainboard_enable, - .final = mainboard_final, }; diff --git a/src/mainboard/google/guybrush/variants/baseboard/gpio.c b/src/mainboard/google/guybrush/variants/baseboard/gpio.c index 66b40be..fb01789 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/gpio.c +++ b/src/mainboard/google/guybrush/variants/baseboard/gpio.c @@ -18,7 +18,7 @@ /* WAKE_L */ PAD_NF_SCI(GPIO_2, WAKE_L, PULL_NONE, EDGE_LOW), /* EN_PWR_FP */ - PAD_GPO(GPIO_3, HIGH), + PAD_GPO(GPIO_3, LOW), /* SOC_PEN_DETECT_ODL */ PAD_WAKE(GPIO_4, PULL_NONE, EDGE_HIGH, S0i3), /* SD_AUX_RESET_L */ @@ -33,7 +33,7 @@ PAD_SCI(GPIO_9, PULL_NONE, EDGE_HIGH), /* S0A3 */ PAD_NF(GPIO_10, S0A3, PULL_NONE), - /* SOC_FP_RST_L - Brought high in finalize */ + /* SOC_FP_RST_L */ PAD_GPO(GPIO_11, LOW), /* SLP_S3_GATED */ PAD_GPO(GPIO_12, LOW), @@ -291,20 +291,6 @@ PAD_NFO(GPIO_26, PCIE_RST_L, HIGH), };
-static const struct soc_amd_gpio fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_3, LOW), -}; - -static const struct soc_amd_gpio fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_3), -}; - const struct soc_amd_gpio *__weak variant_pcie_gpio_table(size_t *size) { *size = ARRAY_SIZE(pcie_gpio_table); @@ -355,49 +341,6 @@
const __weak struct soc_amd_gpio *variant_sleep_gpio_table(size_t *size) { - if (acpi_get_sleep_type() == ACPI_S5) - return variant_fpmcu_shutdown_gpio_table(size); - *size = ARRAY_SIZE(sleep_gpio_table); return sleep_gpio_table; } - -const __weak struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_shutdown_gpio_table); - return fpmcu_shutdown_gpio_table; -} - -const __weak struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_disable_gpio_table); - return fpmcu_disable_gpio_table; -} - -__weak void variant_fpmcu_reset(void) -{ - size_t size; - const struct soc_amd_gpio *gpio_table; - - if (acpi_get_sleep_type() == ACPI_S3) - return; - /* If the system is not resuming from S3, power off the FPMCU */ - gpio_table = variant_fpmcu_shutdown_gpio_table(&size); - gpio_configure_pads(gpio_table, size); -} - -__weak void variant_finalize_gpios(void) -{ - size_t size; - const struct soc_amd_gpio *gpio_table; - - if (variant_has_fpmcu()) { - if (acpi_get_sleep_type() == ACPI_S3) - return; - /* Deassert the FPMCU reset to enable the FPMCU */ - gpio_set(GPIO_11, 1); /* FPMCU_RST_L */ - } else { - gpio_table = variant_fpmcu_disable_gpio_table(&size); - gpio_configure_pads(gpio_table, size); - } -} diff --git a/src/mainboard/google/guybrush/variants/baseboard/helpers.c b/src/mainboard/google/guybrush/variants/baseboard/helpers.c index 04c05bb..36f7b05 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/helpers.c +++ b/src/mainboard/google/guybrush/variants/baseboard/helpers.c @@ -4,13 +4,6 @@ #include <device/device.h> #include <soc/gpio.h>
-WEAK_DEV_PTR(fpmcu); - -bool variant_has_fpmcu(void) -{ - return is_dev_enabled(DEV_PTR(fpmcu)); -} - bool __weak variant_has_pcie_wwan(void) { return false; diff --git a/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h index 6cc96f8..955d170 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h @@ -40,19 +40,6 @@ /* This function provides GPIO settings before entering sleep. */ const struct soc_amd_gpio *variant_sleep_gpio_table(size_t *size);
-/* This function provides GPIO settings for fpmcu shutdown. */ -const struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size); - -/* This function provides GPIO settings for fpmcu disable. */ -const struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size); - -/* Finalize GPIOs, such as FPMCU power */ -void variant_finalize_gpios(void); - -void variant_fpmcu_reset(void); - -bool variant_has_fpmcu(void); - bool variant_has_pcie_wwan(void);
void variant_update_dxio_descriptors(fsp_dxio_descriptor *dxio_descriptors); diff --git a/src/mainboard/google/guybrush/variants/guybrush/gpio.c b/src/mainboard/google/guybrush/variants/guybrush/gpio.c index 4634c5a..bf8e999 100644 --- a/src/mainboard/google/guybrush/variants/guybrush/gpio.c +++ b/src/mainboard/google/guybrush/variants/guybrush/gpio.c @@ -24,7 +24,7 @@ /* Unused */ PAD_NC(GPIO_85), /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), + PAD_GPO(GPIO_32, LOW), };
/* This table is used by guybrush variant with board version >= 2. */ @@ -38,7 +38,7 @@ /* Unused */ PAD_NC(GPIO_85), /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), + PAD_GPO(GPIO_32, LOW), };
static const struct soc_amd_gpio override_early_gpio_table[] = { @@ -67,20 +67,6 @@ PAD_GPO(GPIO_69, HIGH), };
-static const struct soc_amd_gpio fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_32, LOW), -}; - -static const struct soc_amd_gpio fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_32), -}; - const struct soc_amd_gpio *variant_override_gpio_table(size_t *size) { uint32_t board_version = board_id(); @@ -118,15 +104,3 @@ *size = ARRAY_SIZE(bid2_pcie_gpio_table); return bid2_pcie_gpio_table; } - -const struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_shutdown_gpio_table); - return fpmcu_shutdown_gpio_table; -} - -const struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_disable_gpio_table); - return fpmcu_disable_gpio_table; -} diff --git a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb index 92b1980..01876c8 100644 --- a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb +++ b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb @@ -172,6 +172,10 @@ register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW(GPIO_21)" register "wake" = "GEVENT_5" register "uart" = "ACPI_UART_RAW_DEVICE(3000000, 64)" + register "has_power_resource" = "1" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPIO_11)" + register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_32)" + register "enable_delay_ms" = "3" device generic 0 alias fpmcu on probe FP FP_PRESENT end diff --git a/src/mainboard/google/guybrush/variants/nipperkin/gpio.c b/src/mainboard/google/guybrush/variants/nipperkin/gpio.c index 6a12ad3..4d6e045 100644 --- a/src/mainboard/google/guybrush/variants/nipperkin/gpio.c +++ b/src/mainboard/google/guybrush/variants/nipperkin/gpio.c @@ -23,7 +23,7 @@ /* Unused */ PAD_NC(GPIO_85), /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), + PAD_GPO(GPIO_32, LOW), };
/* This table is used by nipperkin variant with board version >= 2. */ @@ -60,38 +60,6 @@ PAD_NC(GPIO_69), };
-/* This table is used by nipperkin variant with board version < 2. */ -static const struct soc_amd_gpio bid1_fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_32, LOW), -}; - -/* This table is used by nipperkin variant with board version >= 2. */ -static const struct soc_amd_gpio bid2_fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_3, LOW), -}; - -/* This table is used by nipperkin variant with board version < 2. */ -static const struct soc_amd_gpio bid1_fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_32), -}; - -/* This table is used by nipperkin variant with board version >= 2. */ -static const struct soc_amd_gpio bid2_fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_3), -}; - const struct soc_amd_gpio *variant_override_gpio_table(size_t *size) { uint32_t board_version = board_id(); @@ -123,29 +91,3 @@ *size = ARRAY_SIZE(bid2_override_pcie_gpio_table); return bid2_override_pcie_gpio_table; } - -const struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size) -{ - uint32_t board_version = board_id(); - - if (board_version < 2) { - *size = ARRAY_SIZE(bid1_fpmcu_shutdown_gpio_table); - return bid1_fpmcu_shutdown_gpio_table; - } - - *size = ARRAY_SIZE(bid2_fpmcu_shutdown_gpio_table); - return bid2_fpmcu_shutdown_gpio_table; -} - -const struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size) -{ - uint32_t board_version = board_id(); - - if (board_version < 2) { - *size = ARRAY_SIZE(bid1_fpmcu_disable_gpio_table); - return bid1_fpmcu_disable_gpio_table; - } - - *size = ARRAY_SIZE(bid2_fpmcu_disable_gpio_table); - return bid2_fpmcu_disable_gpio_table; -} diff --git a/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb b/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb index 48b04fb..3599b28 100644 --- a/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb +++ b/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb @@ -154,6 +154,10 @@ register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW(GPIO_21)" register "wake" = "GEVENT_5" register "uart" = "ACPI_UART_RAW_DEVICE(3000000, 64)" + register "has_power_resource" = "1" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPIO_11)" + register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_3)" + register "enable_delay_ms" = "3" device generic 0 alias fpmcu on probe FP FP_PRESENT end diff --git a/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c b/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c index 1042805..c2f4fbf 100644 --- a/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c +++ b/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c @@ -4,18 +4,36 @@ #include <boardid.h> #include <device/device.h> #include <drivers/i2c/tpm/chip.h> +#include <drivers/uart/acpi/chip.h> #include <soc/gpio.h>
-void variant_devtree_update(void) +static void cr50_devtree_update(void) { - uint32_t board_ver = board_id(); const struct device *cr50_dev = DEV_PTR(cr50); struct drivers_i2c_tpm_config *cfg; struct acpi_gpio cr50_irq_gpio = ACPI_GPIO_IRQ_EDGE_LOW(GPIO_3);
+ cfg = config_of(cr50_dev); + cfg->irq_gpio = cr50_irq_gpio; +} + +static void fpmcu_devtree_update(void) +{ + const struct device *fpmcu_dev = DEV_PTR(fpmcu); + struct drivers_uart_acpi_config *cfg; + struct acpi_gpio fpmcu_enable_gpio = ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_32); + + cfg = config_of(fpmcu_dev); + cfg->enable_gpio = fpmcu_enable_gpio; +} + +void variant_devtree_update(void) +{ + uint32_t board_ver = board_id(); + if (board_ver > 1) return;
- cfg = config_of(cr50_dev); - cfg->irq_gpio = cr50_irq_gpio; + cr50_devtree_update(); + fpmcu_devtree_update(); }