config/coreboot/default: Update MEC5035 patches

- Update the MEC5035 S3 patches to the versions that were sent upstream
  to prevent conflicts with subsequent patches for that EC.
- Update the patch that enables the S3 SMI handler in mainboard code so
  that all Latitudes use the handler.
- Add a new patch that tells the EC to route power button events to the
  host so that the OS can decide what to do. Without it, the EC powers
  off the system without letting the OS cleanly shut down.

Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com>

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
+