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

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
+