Re: [PATCH v2 1/1] platform/x86: asus-wmi: disable USB0 hub on ROG Ally before suspend
From: Luke Jones
Date: Mon Nov 27 2023 - 19:58:04 EST
On Mon, Nov 27 2023 at 11:39:09 PM +01:00:00, Hans de Goede
<hdegoede@xxxxxxxxxx> wrote:
Hi,
On 11/27/23 21:17, Luke Jones wrote:
On Mon, Nov 27 2023 at 09:53:13 AM +01:00:00, Hans de Goede
<hdegoede@xxxxxxxxxx> wrote:
Hi,
On 11/27/23 00:05, Luke D. Jones wrote:
ASUS have worked around an issue in XInput where it doesn't
support USB
selective suspend, which causes suspend issues in Windows. They
worked
around this by adjusting the MCU firmware to disable the USB0
hub when
the screen is switched off during the Microsoft DSM suspend path
in ACPI.
The issue we have with this however is one of timing - the call
the tells
the MCU to this isn't able to complete before suspend is done so
we call
this in a prepare() and add a small msleep() to ensure it is
done. This
must be done before the screen is switched off to prevent a
variety of
possible races.
Further to this the MCU powersave option must also be disabled
as it can
cause a number of issues such as:
- unreliable resume connection of N-Key
- complete loss of N-Key if the power is plugged in while
suspended
Disabling the powersave option prevents this.
Without this the MCU is unable to initialise itself correctly on
resume.
Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx>
Thanks, patch looks good to me, except that all the new lines
seem to use 4 spaces rather then a tab char as indent.
Apologies for the previous HTML email.
I must be going mad... are you sure? I've checked the patch file I
submitted. Run checkpatch on it. Checked my email copy, and checked
in lore... I can't see where space chars are?
So I just checked the copy in patchwork:
https://patchwork.kernel.org/project/platform-driver-x86/patch/20231126230521.125708-2-luke@xxxxxxxxxx/
and you are rights, no 4 spaces there.
Where as if you look further down in this reply, where the original
patch is quoted the 4 spaces are right there, so now I'm wondering
if maybe my mail client introduced the problem when I was replying ?
(replies to other patches preserve the tabs just fine).
So this is weird, but lets just forget about it, just some weird
glitch ...
Sun flares and cosmic particles.
I'm comfortable with this being merged and doing through stable also. I
did many many more tests this morning, along with a half dozen other
people and this solution appears to be the only reliable option. When
ASUS provide a way to turn off the MCU unplug feature I will update
with a new patch (and add the TODO).
With that fixed you can add my:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
to the next version.
Regards,
Hans
---
drivers/platform/x86/asus-wmi.c | 50
++++++++++++++++++++++
include/linux/platform_data/x86/asus-wmi.h | 3 ++
2 files changed, 53 insertions(+)
diff --git a/drivers/platform/x86/asus-wmi.c
b/drivers/platform/x86/asus-wmi.c
index 6a79f16233ab..4ba33dfebfd4 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -16,6 +16,7 @@
#include <linux/acpi.h>
#include <linux/backlight.h>
#include <linux/debugfs.h>
+#include <linux/delay.h>
#include <linux/dmi.h>
#include <linux/fb.h>
#include <linux/hwmon.h>
@@ -132,6 +133,11 @@ module_param(fnlock_default, bool, 0444);
#define ASUS_SCREENPAD_BRIGHT_MAX 255
#define ASUS_SCREENPAD_BRIGHT_DEFAULT 60
+/* Controls the power state of the USB0 hub on ROG Ally which
input is on */
+#define ASUS_USB0_PWR_EC0_CSEE "\\_SB.PCI0.SBRG.EC0.CSEE"
+/* 300ms so far seems to produce a reliable result on AC and
battery */
+#define ASUS_USB0_PWR_EC0_CSEE_WAIT 300
+
static const char * const ashs_ids[] = { "ATK4001", "ATK4002",
NULL };
static int throttle_thermal_policy_write(struct asus_wmi *);
@@ -300,6 +306,9 @@ struct asus_wmi {
bool fnlock_locked;
+ /* The ROG Ally device requires the MCU USB device be
disconnected before suspend */
+ bool ally_mcu_usb_switch;
+
struct asus_wmi_debug debug;
struct asus_wmi_driver *driver;
@@ -4488,6 +4497,8 @@ static int asus_wmi_add(struct
platform_device *pdev)
asus->nv_temp_tgt_available = asus_wmi_dev_is_present(asus,
ASUS_WMI_DEVID_NV_THERM_TARGET);
asus->panel_overdrive_available =
asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PANEL_OD);
asus->mini_led_mode_available =
asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_MINI_LED_MODE);
+ asus->ally_mcu_usb_switch = acpi_has_method(NULL,
ASUS_USB0_PWR_EC0_CSEE)
+ && dmi_match(DMI_BOARD_NAME, "RC71L");
err = fan_boost_mode_check_present(asus);
if (err)
@@ -4654,6 +4665,43 @@ static int asus_hotk_resume(struct device
*device)
asus_wmi_fnlock_update(asus);
asus_wmi_tablet_mode_get_state(asus);
+
+ return 0;
+}
+
+static int asus_hotk_resume_early(struct device *device)
+{
+ struct asus_wmi *asus = dev_get_drvdata(device);
+
+ if (asus->ally_mcu_usb_switch) {
+ if (ACPI_FAILURE(acpi_execute_simple_method(NULL,
ASUS_USB0_PWR_EC0_CSEE, 0xB8)))
+ dev_err(device, "ROG Ally MCU failed to connect USB
dev\n");
+ else
+ msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
+ }
+ return 0;
+}
+
+static int asus_hotk_prepare(struct device *device)
+{
+ struct asus_wmi *asus = dev_get_drvdata(device);
+ int result, err;
+
+ if (asus->ally_mcu_usb_switch) {
+ /* When powersave is enabled it causes many issues with
resume of USB hub */
+ result = asus_wmi_get_devstate_simple(asus,
ASUS_WMI_DEVID_MCU_POWERSAVE);
+ if (result == 1) {
+ dev_warn(device, "MCU powersave enabled, disabling
to prevent resume issues");
+ err =
asus_wmi_set_devstate(ASUS_WMI_DEVID_MCU_POWERSAVE, 0, &result);
+ if (err || result != 1)
+ dev_err(device, "Failed to set MCU powersave
mode: %d\n", err);
+ }
+ /* sleep required to ensure USB0 is disabled before
sleep continues */
+ if (ACPI_FAILURE(acpi_execute_simple_method(NULL,
ASUS_USB0_PWR_EC0_CSEE, 0xB7)))
+ dev_err(device, "ROG Ally MCU failed to disconnect
USB dev\n");
+ else
+ msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
+ }
return 0;
}
@@ -4701,6 +4749,8 @@ static const struct dev_pm_ops asus_pm_ops
= {
.thaw = asus_hotk_thaw,
.restore = asus_hotk_restore,
.resume = asus_hotk_resume,
+ .resume_early = asus_hotk_resume_early,
+ .prepare = asus_hotk_prepare,
};
/* Registration
***************************************************************/
diff --git a/include/linux/platform_data/x86/asus-wmi.h
b/include/linux/platform_data/x86/asus-wmi.h
index 63e630276499..ab1c7deff118 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -114,6 +114,9 @@
/* Charging mode - 1=Barrel, 2=USB */
#define ASUS_WMI_DEVID_CHARGE_MODE 0x0012006C
+/* MCU powersave mode */
+#define ASUS_WMI_DEVID_MCU_POWERSAVE 0x001200E2
+
/* epu is connected? 1 == true */
#define ASUS_WMI_DEVID_EGPU_CONNECTED 0x00090018
/* egpu on/off */