diff options
author | Leah Rowe <vimuser@noreply.codeberg.org> | 2024-10-27 18:44:41 +0000 |
---|---|---|
committer | Leah Rowe <vimuser@noreply.codeberg.org> | 2024-10-27 18:44:41 +0000 |
commit | 6c78942290df512b6ee922ca408404f57e3bb8c2 (patch) | |
tree | 02d344ba2b30ad0867482dcb75076fc207fe76c1 /config/coreboot | |
parent | 237fa1e3c18365794bf5bf525df99a460c821192 (diff) | |
parent | b257662e55c7904b0d6f50ad688a5b8423503604 (diff) |
Merge pull request 'config/coreboot/default: Update MEC5035 patches' (#244) from nic3-14159/lbmk:mec5035-updates into master
Reviewed-on: https://codeberg.org/libreboot/lbmk/pulls/244
Diffstat (limited to 'config/coreboot')
-rw-r--r-- | config/coreboot/default/patches/0037-ec-dell-mec5035-Replace-defines-with-enums.patch | 91 | ||||
-rw-r--r-- | config/coreboot/default/patches/0038-ec-dell-mec5035-Add-S3-suspend-SMI-handler.patch (renamed from config/coreboot/default/patches/0037-ec-dell-mec5035-Add-S3-suspend-SMI-handler.patch) | 74 | ||||
-rw-r--r-- | config/coreboot/default/patches/0038-mb-dell-Add-S3-SMI-handler-for-SNB-IVB-Latitudes.patch | 31 | ||||
-rw-r--r-- | config/coreboot/default/patches/0066-mb-dell-Add-S3-SMI-handler-for-Dell-Latitudes.patch | 70 | ||||
-rw-r--r-- | config/coreboot/default/patches/0067-ec-dell-mec5035-Route-power-button-event-to-host.patch | 92 |
5 files changed, 305 insertions, 53 deletions
diff --git a/config/coreboot/default/patches/0037-ec-dell-mec5035-Replace-defines-with-enums.patch b/config/coreboot/default/patches/0037-ec-dell-mec5035-Replace-defines-with-enums.patch new file mode 100644 index 00000000..77011bce --- /dev/null +++ b/config/coreboot/default/patches/0037-ec-dell-mec5035-Replace-defines-with-enums.patch @@ -0,0 +1,91 @@ +From 7420608acfca1790756fd80d718b737352379dbe Mon Sep 17 00:00:00 2001 +From: Nicholas Chin <nic.c3.14@gmail.com> +Date: Tue, 28 May 2024 17:23:21 -0600 +Subject: [PATCH 37/38] ec/dell/mec5035: Replace defines with enums + +Instead of using defines for command IDs and argument values, use enums +to provide more type safety. This also has the effect of moving the +command IDs to a more central location instead of defines spread out +throughout the header. + +Change-Id: I788531e8b70e79541213853f177326d217235ef2 +Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com> +Reviewed-on: https://review.coreboot.org/c/coreboot/+/82998 +Tested-by: build bot (Jenkins) <no-reply@coreboot.org> +Reviewed-by: Felix Singer <service+coreboot-gerrit@felixsinger.de> +--- + src/ec/dell/mec5035/mec5035.c | 10 +++++----- + src/ec/dell/mec5035/mec5035.h | 20 ++++++++++++-------- + 2 files changed, 17 insertions(+), 13 deletions(-) + +diff --git a/src/ec/dell/mec5035/mec5035.c b/src/ec/dell/mec5035/mec5035.c +index 68b6b2f7fb..dffbb7960c 100644 +--- a/src/ec/dell/mec5035/mec5035.c ++++ b/src/ec/dell/mec5035/mec5035.c +@@ -66,17 +66,17 @@ static enum cb_err write_mailbox_regs(const u8 *data, u8 start, u8 count) + return CB_SUCCESS; + } + +-static void ec_command(u8 cmd) ++static void ec_command(enum mec5035_cmd cmd) + { + outb(0, MAILBOX_INDEX); +- outb(cmd, MAILBOX_DATA); ++ outb((u8)cmd, MAILBOX_DATA); + wait_ec(); + } + +-u8 mec5035_mouse_touchpad(u8 setting) ++u8 mec5035_mouse_touchpad(enum ec_mouse_setting setting) + { +- u8 buf[15] = {0}; +- write_mailbox_regs(&setting, 2, 1); ++ u8 buf[15] = {(u8)setting}; ++ write_mailbox_regs(buf, 2, 1); + ec_command(CMD_MOUSE_TP); + /* The vendor firmware reads 15 bytes starting at index 1, presumably + to get some sort of return code. Though I don't know for sure if +diff --git a/src/ec/dell/mec5035/mec5035.h b/src/ec/dell/mec5035/mec5035.h +index fa15a9d621..32f791cb01 100644 +--- a/src/ec/dell/mec5035/mec5035.h ++++ b/src/ec/dell/mec5035/mec5035.h +@@ -7,16 +7,20 @@ + + #define NUM_REGISTERS 32 + ++enum mec5035_cmd { ++ CMD_MOUSE_TP = 0x1a, ++ CMD_RADIO_CTRL = 0x2b, ++ CMD_CPU_OK = 0xc2, ++}; ++ + /* Touchpad (TP) and mouse related. The EC seems to + default to 0 which results in the TP not working. */ +-#define CMD_MOUSE_TP 0x1a +-#define SERIAL_MOUSE 0 /* Disable TP, force use of a serial mouse */ +-#define PS2_MOUSE 1 /* Disable TP when using a PS/2 mouse */ +-#define TP_PS2_MOUSE 2 /* Leave TP enabled when using a PS/2 mouse */ +- +-#define CMD_CPU_OK 0xc2 ++enum ec_mouse_setting { ++ SERIAL_MOUSE = 0, /* Disable TP, force use of a serial mouse */ ++ PS2_MOUSE, /* Disable TP when using a PS/2 mouse */ ++ TP_PS2_MOUSE /* Leave TP enabled when using a PS/2 mouse */ ++}; + +-#define CMD_RADIO_CTRL 0x2b + #define RADIO_CTRL_NUM_ARGS 3 + enum ec_radio_dev { + RADIO_WLAN = 0, +@@ -29,7 +33,7 @@ enum ec_radio_state { + RADIO_ON + }; + +-u8 mec5035_mouse_touchpad(u8 setting); ++u8 mec5035_mouse_touchpad(enum ec_mouse_setting setting); + void mec5035_cpu_ok(void); + void mec5035_early_init(void); + void mec5035_control_radio(enum ec_radio_dev device, enum ec_radio_state state); +-- +2.47.0 + diff --git a/config/coreboot/default/patches/0037-ec-dell-mec5035-Add-S3-suspend-SMI-handler.patch b/config/coreboot/default/patches/0038-ec-dell-mec5035-Add-S3-suspend-SMI-handler.patch index a4fce1d1..7c96d9f9 100644 --- a/config/coreboot/default/patches/0037-ec-dell-mec5035-Add-S3-suspend-SMI-handler.patch +++ b/config/coreboot/default/patches/0038-ec-dell-mec5035-Add-S3-suspend-SMI-handler.patch @@ -1,16 +1,37 @@ -From 726d0001bda013a094779149828e2bc1be581fa8 Mon Sep 17 00:00:00 2001 +From 762f5d95d2314c3d09c1562d36d111dcdb9c8b93 Mon Sep 17 00:00:00 2001 From: Nicholas Chin <nic.c3.14@gmail.com> Date: Fri, 3 May 2024 11:03:32 -0600 -Subject: [PATCH 37/65] ec/dell/mec5035: Add S3 suspend SMI handler +Subject: [PATCH 38/38] ec/dell/mec5035: Add S3 suspend SMI handler + +This is necessary for S3 resume to work on SNB and newer Dell Latitude +laptops. If a command isn't sent, the EC cuts power to the DIMMs, +preventing the system from resuming. These commands were found using an +FPGA to log all LPC bus transactions between the host and the EC and +then narrowing down which ones were actually necessary. + +Interestingly, the command IDs appear to be identical to those in +ec/google/wilco, the EC used on Dell Latitude Chromebooks, and that EC +implements a similar S3 SMI handler as the one implemented in this +commit. The Wilco EC Kconfig does suggest that its firmware is a +modified version of Dell's usual Latitude EC firmware, so the +similarities seem to be intentional. + +These similarities also identified a command to enable or disable wake +sources like the power button and lid switch, and this was added to the +SMI handler to disable lid wake as the system does not yet resume +properly from a like wake with coreboot. + +Tested on the Latitude E6430 (Ivy Bridge) and the Precision M6800 +(Haswell, not yet pushed). Change-Id: I655868aba46911d128f6c24f410dc6fdf83f3070 Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com> --- src/ec/dell/mec5035/Makefile.mk | 1 + src/ec/dell/mec5035/mec5035.c | 14 ++++++++++++++ - src/ec/dell/mec5035/mec5035.h | 19 +++++++++++++++++++ + src/ec/dell/mec5035/mec5035.h | 22 ++++++++++++++++++++++ src/ec/dell/mec5035/smihandler.c | 17 +++++++++++++++++ - 4 files changed, 51 insertions(+) + 4 files changed, 54 insertions(+) create mode 100644 src/ec/dell/mec5035/smihandler.c diff --git a/src/ec/dell/mec5035/Makefile.mk b/src/ec/dell/mec5035/Makefile.mk @@ -25,20 +46,13 @@ index 4ebdd811f9..be557e4599 100644 endif diff --git a/src/ec/dell/mec5035/mec5035.c b/src/ec/dell/mec5035/mec5035.c -index 68b6b2f7fb..33bf046634 100644 +index dffbb7960c..85c2ab0140 100644 --- a/src/ec/dell/mec5035/mec5035.c +++ b/src/ec/dell/mec5035/mec5035.c @@ -94,6 +94,20 @@ void mec5035_control_radio(enum ec_radio_dev dev, enum ec_radio_state state) ec_command(CMD_RADIO_CTRL); } -+void mec5035_sleep_enable(void) -+{ -+ u8 buf[SLEEP_EN_NUM_ARGS] = {3, 0}; -+ write_mailbox_regs(buf, 2, SLEEP_EN_NUM_ARGS); -+ ec_command(CMD_SLEEP_ENABLE); -+} -+ +void mec5035_change_wake(u8 source, enum ec_wake_change change) +{ + u8 buf[ACPI_WAKEUP_NUM_ARGS] = {change, source, 0, 0x40}; @@ -46,14 +60,21 @@ index 68b6b2f7fb..33bf046634 100644 + ec_command(CMD_ACPI_WAKEUP_CHANGE); +} + ++void mec5035_sleep_enable(void) ++{ ++ u8 buf[SLEEP_EN_NUM_ARGS] = {3, 0}; ++ write_mailbox_regs(buf, 2, SLEEP_EN_NUM_ARGS); ++ ec_command(CMD_SLEEP_ENABLE); ++} ++ void mec5035_early_init(void) { /* If this isn't sent the EC shuts down the system after about 15 diff --git a/src/ec/dell/mec5035/mec5035.h b/src/ec/dell/mec5035/mec5035.h -index fa15a9d621..069616fbc5 100644 +index 32f791cb01..8d4fded28b 100644 --- a/src/ec/dell/mec5035/mec5035.h +++ b/src/ec/dell/mec5035/mec5035.h -@@ -4,6 +4,7 @@ +@@ -4,12 +4,15 @@ #define _EC_DELL_MEC5035_H_ #include <stdint.h> @@ -61,37 +82,46 @@ index fa15a9d621..069616fbc5 100644 #define NUM_REGISTERS 32 -@@ -29,9 +30,27 @@ enum ec_radio_state { + enum mec5035_cmd { + CMD_MOUSE_TP = 0x1a, + CMD_RADIO_CTRL = 0x2b, ++ CMD_ACPI_WAKEUP_CHANGE = 0x4a, ++ CMD_SLEEP_ENABLE = 0x64, + CMD_CPU_OK = 0xc2, + }; + +@@ -33,9 +36,28 @@ enum ec_radio_state { RADIO_ON }; -+#define CMD_ACPI_WAKEUP_CHANGE 0x4a +#define ACPI_WAKEUP_NUM_ARGS 4 +enum ec_wake_change { + WAKE_OFF = 0, + WAKE_ON +}; ++ ++/* Copied from ec/google/wilco/commands.h. Not sure if these all apply */ +enum ec_acpi_wake_events { + EC_ACPI_WAKE_PWRB = BIT(0), /* Wake up by power button */ + EC_ACPI_WAKE_LID = BIT(1), /* Wake up by lid switch */ + EC_ACPI_WAKE_RTC = BIT(5), /* Wake up by RTC */ +}; + -+#define CMD_SLEEP_ENABLE 0x64 +#define SLEEP_EN_NUM_ARGS 2 + - u8 mec5035_mouse_touchpad(u8 setting); + u8 mec5035_mouse_touchpad(enum ec_mouse_setting setting); void mec5035_cpu_ok(void); void mec5035_early_init(void); void mec5035_control_radio(enum ec_radio_dev device, enum ec_radio_state state); -+void mec5035_sleep(int slp_type); +void mec5035_change_wake(u8 source, enum ec_wake_change change); +void mec5035_sleep_enable(void); ++ ++void mec5035_smi_sleep(int slp_type); #endif /* _EC_DELL_MEC5035_H_ */ diff --git a/src/ec/dell/mec5035/smihandler.c b/src/ec/dell/mec5035/smihandler.c new file mode 100644 -index 0000000000..1db834773d +index 0000000000..958733bf97 --- /dev/null +++ b/src/ec/dell/mec5035/smihandler.c @@ -0,0 +1,17 @@ @@ -102,7 +132,7 @@ index 0000000000..1db834773d +#include <ec/acpi/ec.h> +#include "mec5035.h" + -+void mec5035_sleep(int slp_type) ++void mec5035_smi_sleep(int slp_type) +{ + switch (slp_type) { + case ACPI_S3: @@ -113,5 +143,5 @@ index 0000000000..1db834773d + } +} -- -2.39.5 +2.47.0 diff --git a/config/coreboot/default/patches/0038-mb-dell-Add-S3-SMI-handler-for-SNB-IVB-Latitudes.patch b/config/coreboot/default/patches/0038-mb-dell-Add-S3-SMI-handler-for-SNB-IVB-Latitudes.patch deleted file mode 100644 index d1db5146..00000000 --- a/config/coreboot/default/patches/0038-mb-dell-Add-S3-SMI-handler-for-SNB-IVB-Latitudes.patch +++ /dev/null @@ -1,31 +0,0 @@ -From 451b3e3e334d350c060444d79bc964bd90ec1152 Mon Sep 17 00:00:00 2001 -From: Nicholas Chin <nic.c3.14@gmail.com> -Date: Fri, 3 May 2024 16:31:12 -0600 -Subject: [PATCH 38/65] mb/dell/: Add S3 SMI handler for SNB/IVB Latitudes - -This should fix S3 suspend on these systems - -Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com> ---- - src/mainboard/dell/snb_ivb_latitude/smihandler.c | 9 +++++++++ - 1 file changed, 9 insertions(+) - create mode 100644 src/mainboard/dell/snb_ivb_latitude/smihandler.c - -diff --git a/src/mainboard/dell/snb_ivb_latitude/smihandler.c b/src/mainboard/dell/snb_ivb_latitude/smihandler.c -new file mode 100644 -index 0000000000..334d7b1a5f ---- /dev/null -+++ b/src/mainboard/dell/snb_ivb_latitude/smihandler.c -@@ -0,0 +1,9 @@ -+/* SPDX-License-Identifier: GPL-2.0-only */ -+ -+#include <cpu/x86/smm.h> -+#include <ec/dell/mec5035/mec5035.h> -+ -+void mainboard_smi_sleep(u8 slp_typ) -+{ -+ mec5035_sleep(slp_typ); -+} --- -2.39.5 - diff --git a/config/coreboot/default/patches/0066-mb-dell-Add-S3-SMI-handler-for-Dell-Latitudes.patch b/config/coreboot/default/patches/0066-mb-dell-Add-S3-SMI-handler-for-Dell-Latitudes.patch new file mode 100644 index 00000000..d58968a1 --- /dev/null +++ b/config/coreboot/default/patches/0066-mb-dell-Add-S3-SMI-handler-for-Dell-Latitudes.patch @@ -0,0 +1,70 @@ +From 0fe1d4b9fe56a0f27a6ff39cfb94d63559b729b8 Mon Sep 17 00:00:00 2001 +From: Nicholas Chin <nic.c3.14@gmail.com> +Date: Fri, 3 May 2024 16:31:12 -0600 +Subject: [PATCH 66/67] mb/dell: Add S3 SMI handler for Dell Latitudes + +Integrate the previously added mec5035_smi_sleep() function into +mainboard code to fix S3 suspend on the SNB/IVB Latitudes and the E7240. +The E6400 does not require the EC command to sucessfully suspend and +resume from S3, though sending it does enable the breathing effect on +the power LED while in S3. Without it, all LEDs turn off during S3. + +Change-Id: Ic0d887f75be13c3fb9f6df62153ac458895e0283 +Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com> +--- + src/mainboard/dell/e7240/smihandler.c | 9 +++++++++ + src/mainboard/dell/gm45_latitude/smihandler.c | 9 +++++++++ + src/mainboard/dell/snb_ivb_latitude/smihandler.c | 9 +++++++++ + 3 files changed, 27 insertions(+) + create mode 100644 src/mainboard/dell/e7240/smihandler.c + create mode 100644 src/mainboard/dell/gm45_latitude/smihandler.c + create mode 100644 src/mainboard/dell/snb_ivb_latitude/smihandler.c + +diff --git a/src/mainboard/dell/e7240/smihandler.c b/src/mainboard/dell/e7240/smihandler.c +new file mode 100644 +index 0000000000..00e55b51db +--- /dev/null ++++ b/src/mainboard/dell/e7240/smihandler.c +@@ -0,0 +1,9 @@ ++/* SPDX-License-Identifier: GPL-2.0-only */ ++ ++#include <cpu/x86/smm.h> ++#include <ec/dell/mec5035/mec5035.h> ++ ++void mainboard_smi_sleep(u8 slp_typ) ++{ ++ mec5035_smi_sleep(slp_typ); ++} +diff --git a/src/mainboard/dell/gm45_latitude/smihandler.c b/src/mainboard/dell/gm45_latitude/smihandler.c +new file mode 100644 +index 0000000000..00e55b51db +--- /dev/null ++++ b/src/mainboard/dell/gm45_latitude/smihandler.c +@@ -0,0 +1,9 @@ ++/* SPDX-License-Identifier: GPL-2.0-only */ ++ ++#include <cpu/x86/smm.h> ++#include <ec/dell/mec5035/mec5035.h> ++ ++void mainboard_smi_sleep(u8 slp_typ) ++{ ++ mec5035_smi_sleep(slp_typ); ++} +diff --git a/src/mainboard/dell/snb_ivb_latitude/smihandler.c b/src/mainboard/dell/snb_ivb_latitude/smihandler.c +new file mode 100644 +index 0000000000..00e55b51db +--- /dev/null ++++ b/src/mainboard/dell/snb_ivb_latitude/smihandler.c +@@ -0,0 +1,9 @@ ++/* SPDX-License-Identifier: GPL-2.0-only */ ++ ++#include <cpu/x86/smm.h> ++#include <ec/dell/mec5035/mec5035.h> ++ ++void mainboard_smi_sleep(u8 slp_typ) ++{ ++ mec5035_smi_sleep(slp_typ); ++} +-- +2.47.0 + diff --git a/config/coreboot/default/patches/0067-ec-dell-mec5035-Route-power-button-event-to-host.patch b/config/coreboot/default/patches/0067-ec-dell-mec5035-Route-power-button-event-to-host.patch new file mode 100644 index 00000000..d8662e40 --- /dev/null +++ b/config/coreboot/default/patches/0067-ec-dell-mec5035-Route-power-button-event-to-host.patch @@ -0,0 +1,92 @@ +From 32118fa1d21fad517f85cee5eb4edff5f2fd91ea Mon Sep 17 00:00:00 2001 +From: Nicholas Chin <nic.c3.14@gmail.com> +Date: Tue, 18 Jun 2024 21:31:08 -0600 +Subject: [PATCH 67/67] ec/dell/mec5035: Route power button event to host + +If command 0x3e with an argument of 1 isn't sent to the EC, pressing the +power button results in the EC powering off the system without letting +the OS cleanly shutting itself down. This command and argument tells the +EC to route power button events to the host so that it can determine +what to do. + +The EC command was identified from the ec/google/wilco code, which is +used for Dell's Latitude Chromebooks. According to the EC_GOOGLE_WILCO +Kconfig help text, those ECs run a modified version of Dell's typical +Latitude EC firmware, so it is likely that the two firmware +implementations use similar commands. Examining LPC traffic between the +host and the EC on the Latitude E6400 did reveal that the same command +was being sent by the vendor firmware to the EC, but this does not +confirm that it has the same meaning as the command from the Wilco code. +Sending the command using inb/outb calls in a userspace C program while +running coreboot without this patch did allow subsequent power button +events to be handled by the host, confirming that the command was indeed +the same. + +Change-Id: I5ded315270c0e1efbbc90cfa9d9d894b872e99a2 +Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com> +--- + src/ec/dell/mec5035/mec5035.c | 8 ++++++++ + src/ec/dell/mec5035/mec5035.h | 7 +++++++ + 2 files changed, 15 insertions(+) + +diff --git a/src/ec/dell/mec5035/mec5035.c b/src/ec/dell/mec5035/mec5035.c +index 85c2ab0140..bdae929a27 100644 +--- a/src/ec/dell/mec5035/mec5035.c ++++ b/src/ec/dell/mec5035/mec5035.c +@@ -94,6 +94,13 @@ void mec5035_control_radio(enum ec_radio_dev dev, enum ec_radio_state state) + ec_command(CMD_RADIO_CTRL); + } + ++void mec5035_power_button_route(enum ec_power_button_route target) ++{ ++ u8 buf = (u8)target; ++ write_mailbox_regs(&buf, 2, 1); ++ ec_command(CMD_POWER_BUTTON_TO_HOST); ++} ++ + void mec5035_change_wake(u8 source, enum ec_wake_change change) + { + u8 buf[ACPI_WAKEUP_NUM_ARGS] = {change, source, 0, 0x40}; +@@ -121,6 +128,7 @@ static void mec5035_init(struct device *dev) + /* Unconditionally use this argument for now as this setting + is probably the most sensible default out of the 3 choices. */ + mec5035_mouse_touchpad(TP_PS2_MOUSE); ++ mec5035_power_button_route(HOST); + + pc_keyboard_init(NO_AUX_DEVICE); + +diff --git a/src/ec/dell/mec5035/mec5035.h b/src/ec/dell/mec5035/mec5035.h +index 8d4fded28b..51422598c4 100644 +--- a/src/ec/dell/mec5035/mec5035.h ++++ b/src/ec/dell/mec5035/mec5035.h +@@ -11,6 +11,7 @@ + enum mec5035_cmd { + CMD_MOUSE_TP = 0x1a, + CMD_RADIO_CTRL = 0x2b, ++ CMD_POWER_BUTTON_TO_HOST = 0x3e, + CMD_ACPI_WAKEUP_CHANGE = 0x4a, + CMD_SLEEP_ENABLE = 0x64, + CMD_CPU_OK = 0xc2, +@@ -36,6 +37,11 @@ enum ec_radio_state { + RADIO_ON + }; + ++enum ec_power_button_route { ++ EC = 0, ++ HOST ++}; ++ + #define ACPI_WAKEUP_NUM_ARGS 4 + enum ec_wake_change { + WAKE_OFF = 0, +@@ -55,6 +61,7 @@ u8 mec5035_mouse_touchpad(enum ec_mouse_setting setting); + void mec5035_cpu_ok(void); + void mec5035_early_init(void); + void mec5035_control_radio(enum ec_radio_dev device, enum ec_radio_state state); ++void mec5035_power_button_route(enum ec_power_button_route target); + void mec5035_change_wake(u8 source, enum ec_wake_change change); + void mec5035_sleep_enable(void); + +-- +2.47.0 + |