[PATCH 0/2] hid-asus: asus-wmi: refactor Ally suspend/resume

[PATCH 0/2] hid-asus: asus-wmi: refactor Ally suspend/resume

[Luke Jones]

This short series refactors the Ally suspend/resume functionality in the
asus-wmi driver along with adding support for ROG Ally MCU version checking.

The version checking is then used to toggle the use of older CSEE call hacks
that were initially used to combat Ally suspend/wake issues arising from the MCU
not clearing a particular flag on resume. ASUS have since corrected this
especially for Linux in newer firmware versions.

- hid-asus requests the MCU version and displays a warning if the version is
  older than the one that fixes the issue.
- hid-asus awill also toggle the CSEE hack off, and mcu_powersave to on if the
version is high enough.

Luke D. Jones (2):
  hid-asus: check ROG Ally MCU version and warn
  platform/x86: asus-wmi: Refactor Ally suspend/resume

 drivers/hid/hid-asus.c                     | 101 ++++++++++++++++-
 drivers/platform/x86/asus-wmi.c            | 124 ++++++++++++++-------
 include/linux/platform_data/x86/asus-wmi.h |  15 +++
 3 files changed, 199 insertions(+), 41 deletions(-)

--
2.48.1

[PATCH 1/2] hid-asus: check ROG Ally MCU version and warn

[Luke Jones]

From: "Luke D. Jones" <luke@ljones.dev>

ASUS have fixed suspend issues arising from a flag not being cleared in
the MCU FW in both the ROG Ally 1 and the ROG Ally X.

Implement a check and a warning to encourage users to update the FW to
a minimum supported version.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/hid/hid-asus.c | 97 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 95 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 46e3e42f9eb5..e1e60b80115a 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -52,6 +52,10 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
+#define ROG_ALLY_REPORT_SIZE 64
+#define ROG_ALLY_X_MIN_MCU 313
+#define ROG_ALLY_MIN_MCU 319
+
 #define SUPPORT_KBD_BACKLIGHT BIT(0)
 
 #define MAX_TOUCH_MAJOR 8
@@ -84,6 +88,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_MEDION_E1239T		BIT(10)
 #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
 #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
+#define QUIRK_ROG_ALLY_XPAD		BIT(13)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS | \
@@ -534,9 +539,89 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
 	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
 }
 
+/*
+ * We don't care about any other part of the string except the version section.
+ * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
+ */
+static int mcu_parse_version_string(const u8 *response, size_t response_size)
+{
+	const u8 *end = response + response_size;
+	const u8 *p = response;
+	int dots, err;
+	long version;
+
+	dots = 0;
+	while (p < end && dots < 2) {
+		if (*p++ == '.')
+			dots++;
+	}
+
+	if (dots != 2 || p >= end)
+		return -EINVAL;
+
+	err = kstrtol((const char *)p, 10, &version);
+	if (err || version < 0)
+		return -EINVAL;
+
+	return version;
+}
+
+static int mcu_request_version(struct hid_device *hdev)
+{
+	const u8 request[] = { 0x5a, 0x05, 0x03, 0x31, 0x00, 0x20 };
+	u8 *response;
+	int ret;
+
+	response = kzalloc(ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+	if (!response)
+		return -ENOMEM;
+
+	ret = asus_kbd_set_report(hdev, request, sizeof(request));
+	if (ret < 0)
+		goto out;
+
+	ret = hid_hw_raw_request(hdev, FEATURE_REPORT_ID, response,
+				ROG_ALLY_REPORT_SIZE, HID_FEATURE_REPORT,
+				HID_REQ_GET_REPORT);
+	if (ret < 0)
+		goto out;
+
+	ret = mcu_parse_version_string(response, ROG_ALLY_REPORT_SIZE);
+
+out:
+	if (ret < 0)
+		hid_err(hdev, "Failed to get MCU version: %d, response: %*ph\n",
+					ret, ROG_ALLY_REPORT_SIZE, response);
+	kfree(response);
+	return ret;
+}
+
+static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
+{
+	int min_version = ROG_ALLY_X_MIN_MCU;
+	int version;
+
+	version = mcu_request_version(hdev);
+	if (version < 0)
+		return;
+
+	if (idProduct == USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY)
+		min_version = ROG_ALLY_MIN_MCU;
+
+	hid_info(hdev, "Ally device MCU version: %d\n", version);
+	if (version < min_version) {
+		hid_warn(hdev,
+			 "The MCU firmware version must be %d or greater\n"
+			 "Please update your MCU with official ASUS firmware release\n",
+			 min_version);
+	}
+}
+
 static int asus_kbd_register_leds(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+	struct usb_interface *intf;
+	struct usb_device *udev;
 	unsigned char kbd_func;
 	int ret;
 
@@ -560,6 +645,14 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 			if (ret < 0)
 				return ret;
 		}
+
+		if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+			intf = to_usb_interface(hdev->dev.parent);
+			udev = interface_to_usbdev(intf);
+			validate_mcu_fw_version(hdev,
+				le16_to_cpu(udev->descriptor.idProduct));
+		}
+
 	} else {
 		/* Initialize keyboard */
 		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
@@ -1280,10 +1373,10 @@ static const struct hid_device_id asus_devices[] = {
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD},
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
 	  QUIRK_ROG_CLAYMORE_II_KEYBOARD },
-- 
2.48.1

Re: [PATCH 1/2] hid-asus: check ROG Ally MCU version and warn

[Mario Limonciello]

On 2/25/2025 00:17, Luke Jones wrote:
> From: "Luke D. Jones" <luke@ljones.dev>
> 
> ASUS have fixed suspend issues arising from a flag not being cleared in
> the MCU FW in both the ROG Ally 1 and the ROG Ally X.
> 
> Implement a check and a warning to encourage users to update the FW to
> a minimum supported version.
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>   drivers/hid/hid-asus.c | 97 +++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 95 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 46e3e42f9eb5..e1e60b80115a 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -52,6 +52,10 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>   #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>   #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>   
> +#define ROG_ALLY_REPORT_SIZE 64
> +#define ROG_ALLY_X_MIN_MCU 313
> +#define ROG_ALLY_MIN_MCU 319
> +
>   #define SUPPORT_KBD_BACKLIGHT BIT(0)
>   
>   #define MAX_TOUCH_MAJOR 8
> @@ -84,6 +88,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>   #define QUIRK_MEDION_E1239T		BIT(10)
>   #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
>   #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> +#define QUIRK_ROG_ALLY_XPAD		BIT(13)
>   
>   #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
>   						 QUIRK_NO_INIT_REPORTS | \
> @@ -534,9 +539,89 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
>   	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
>   }
>   
> +/*
> + * We don't care about any other part of the string except the version section.
> + * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
> + */
> +static int mcu_parse_version_string(const u8 *response, size_t response_size)
> +{
> +	const u8 *end = response + response_size;
> +	const u8 *p = response;
> +	int dots, err;
> +	long version;
> +
> +	dots = 0;
> +	while (p < end && dots < 2) {
> +		if (*p++ == '.')
> +			dots++;
> +	}
> +

I think it would be good to use strsep() here.

> +	if (dots != 2 || p >= end)
> +		return -EINVAL;
> +
> +	err = kstrtol((const char *)p, 10, &version);
> +	if (err || version < 0)
> +		return -EINVAL;
> +
> +	return version;
> +}
> +
> +static int mcu_request_version(struct hid_device *hdev)
> +{
> +	const u8 request[] = { 0x5a, 0x05, 0x03, 0x31, 0x00, 0x20 };
> +	u8 *response;
> +	int ret;
> +
> +	response = kzalloc(ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
> +	if (!response)
> +		return -ENOMEM;
> +
> +	ret = asus_kbd_set_report(hdev, request, sizeof(request));
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = hid_hw_raw_request(hdev, FEATURE_REPORT_ID, response,
> +				ROG_ALLY_REPORT_SIZE, HID_FEATURE_REPORT,
> +				HID_REQ_GET_REPORT);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = mcu_parse_version_string(response, ROG_ALLY_REPORT_SIZE);
> +
> +out:
> +	if (ret < 0)
> +		hid_err(hdev, "Failed to get MCU version: %d, response: %*ph\n",
> +					ret, ROG_ALLY_REPORT_SIZE, response);
> +	kfree(response);
> +	return ret;
> +}
> +
> +static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
> +{
> +	int min_version = ROG_ALLY_X_MIN_MCU;
> +	int version;
> +
> +	version = mcu_request_version(hdev);
> +	if (version < 0)
> +		return;
> +
> +	if (idProduct == USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY)
> +		min_version = ROG_ALLY_MIN_MCU;
> +
> +	hid_info(hdev, "Ally device MCU version: %d\n", version);
> +	if (version < min_version) {
> +		hid_warn(hdev,
> +			 "The MCU firmware version must be %d or greater\n"

What do you think about:

"The MCU firmware version must be %d or greater to avoid issues with 
suspend.\n"

> +			 "Please update your MCU with official ASUS firmware release\n",
> +			 min_version);
> +	}
> +}

Thinking forward to any future hypothetical devices that don't have a 
min MCU version type of bug I have a suggestion that you put the 
min_version into a lookup method of some sort.

So the flow can be something like this:

static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
{

	int min_version = get_min_version(idProduct);

	if (!min_version)
		return;
	.
	.
	.
}

Or you can do a switch/case instead of get_min_version().

static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
{

	int min_version;

	switch(idProduct) {
	case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY:
		min_version = ROG_ALLY_MIN_MCU;
		break;
	case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLYX:
		min_version = ROG_ALLYX_MIN_MCU;
		break;
	default:
		return;
	}

	.
	.
	.
}

That way you have really straight forward logic that 
validate_mcu_version only runs on devices that you specify.

> +
>   static int asus_kbd_register_leds(struct hid_device *hdev)
>   {
>   	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> +	struct usb_interface *intf;
> +	struct usb_device *udev;
>   	unsigned char kbd_func;
>   	int ret;
>   
> @@ -560,6 +645,14 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>   			if (ret < 0)
>   				return ret;
>   		}
> +
> +		if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> +			intf = to_usb_interface(hdev->dev.parent);
> +			udev = interface_to_usbdev(intf);
> +			validate_mcu_fw_version(hdev,
> +				le16_to_cpu(udev->descriptor.idProduct));
> +		}
> +
>   	} else {
>   		/* Initialize keyboard */
>   		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> @@ -1280,10 +1373,10 @@ static const struct hid_device_id asus_devices[] = {
>   	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>   	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD},
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>   	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>   	    USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
>   	  QUIRK_ROG_CLAYMORE_II_KEYBOARD },

Re: [PATCH 1/2] hid-asus: check ROG Ally MCU version and warn

[Luke Jones]

On Tue, 2025-02-25 at 07:16 -0800, Mario Limonciello wrote:
> On 2/25/2025 00:17, Luke Jones wrote:
> > From: "Luke D. Jones" <luke@ljones.dev>
> > 
> > ASUS have fixed suspend issues arising from a flag not being
> > cleared in
> > the MCU FW in both the ROG Ally 1 and the ROG Ally X.
> > 
> > Implement a check and a warning to encourage users to update the FW
> > to
> > a minimum supported version.
> > 
> > Signed-off-by: Luke D. Jones <luke@ljones.dev>
> > ---
> >   drivers/hid/hid-asus.c | 97
> > +++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 95 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 46e3e42f9eb5..e1e60b80115a 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -52,6 +52,10 @@ MODULE_DESCRIPTION("Asus HID Keyboard and
> > TouchPad");
> >   #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> >   #define FEATURE_KBD_LED_REPORT_ID2 0x5e
> >   
> > +#define ROG_ALLY_REPORT_SIZE 64
> > +#define ROG_ALLY_X_MIN_MCU 313
> > +#define ROG_ALLY_MIN_MCU 319
> > +
> >   #define SUPPORT_KBD_BACKLIGHT BIT(0)
> >   
> >   #define MAX_TOUCH_MAJOR 8
> > @@ -84,6 +88,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and
> > TouchPad");
> >   #define QUIRK_MEDION_E1239T		BIT(10)
> >   #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
> >   #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> > +#define QUIRK_ROG_ALLY_XPAD		BIT(13)
> >   
> >   #define
> > I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
> >   						
> > QUIRK_NO_INIT_REPORTS | \
> > @@ -534,9 +539,89 @@ static bool
> > asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> >   	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> >   }
> >   
> > +/*
> > + * We don't care about any other part of the string except the
> > version section.
> > + * Example strings: FGA80100.RC72LA.312_T01,
> > FGA80100.RC71LS.318_T01
> > + */
> > +static int mcu_parse_version_string(const u8 *response, size_t
> > response_size)
> > +{
> > +	const u8 *end = response + response_size;
> > +	const u8 *p = response;
> > +	int dots, err;
> > +	long version;
> > +
> > +	dots = 0;
> > +	while (p < end && dots < 2) {
> > +		if (*p++ == '.')
> > +			dots++;
> > +	}
> > +
> 
> I think it would be good to use strsep() here.
> 
> > +	if (dots != 2 || p >= end)
> > +		return -EINVAL;
> > +
> > +	err = kstrtol((const char *)p, 10, &version);
> > +	if (err || version < 0)
> > +		return -EINVAL;
> > +
> > +	return version;
> > +}
> > +
> > +static int mcu_request_version(struct hid_device *hdev)
> > +{
> > +	const u8 request[] = { 0x5a, 0x05, 0x03, 0x31, 0x00, 0x20
> > };
> > +	u8 *response;
> > +	int ret;
> > +
> > +	response = kzalloc(ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
> > +	if (!response)
> > +		return -ENOMEM;
> > +
> > +	ret = asus_kbd_set_report(hdev, request, sizeof(request));
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	ret = hid_hw_raw_request(hdev, FEATURE_REPORT_ID,
> > response,
> > +				ROG_ALLY_REPORT_SIZE,
> > HID_FEATURE_REPORT,
> > +				HID_REQ_GET_REPORT);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	ret = mcu_parse_version_string(response,
> > ROG_ALLY_REPORT_SIZE);
> > +
> > +out:
> > +	if (ret < 0)
> > +		hid_err(hdev, "Failed to get MCU version: %d,
> > response: %*ph\n",
> > +					ret, ROG_ALLY_REPORT_SIZE,
> > response);
> > +	kfree(response);
> > +	return ret;
> > +}
> > +
> > +static void validate_mcu_fw_version(struct hid_device *hdev, int
> > idProduct)
> > +{
> > +	int min_version = ROG_ALLY_X_MIN_MCU;
> > +	int version;
> > +
> > +	version = mcu_request_version(hdev);
> > +	if (version < 0)
> > +		return;
> > +
> > +	if (idProduct == USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY)
> > +		min_version = ROG_ALLY_MIN_MCU;
> > +
> > +	hid_info(hdev, "Ally device MCU version: %d\n", version);
> > +	if (version < min_version) {
> > +		hid_warn(hdev,
> > +			 "The MCU firmware version must be %d or
> > greater\n"
> 
> What do you think about:
> 
> "The MCU firmware version must be %d or greater to avoid issues with 
> suspend.\n"
> 

I wasn't sure if I should be as explicit. But since it is directly
related I guess that should be fine. Will do for V2. 

> > +			 "Please update your MCU with official
> > ASUS firmware release\n",
> > +			 min_version);
> > +	}
> > +}
> 
> Thinking forward to any future hypothetical devices that don't have a
> min MCU version type of bug I have a suggestion that you put the 
> min_version into a lookup method of some sort.
> 
> So the flow can be something like this:
> 
> static void validate_mcu_fw_version(struct hid_device *hdev, int
> idProduct)
> {
> 
> 	int min_version = get_min_version(idProduct);
> 
> 	if (!min_version)
> 		return;
> 	.
> 	.
> 	.
> }
> 
> Or you can do a switch/case instead of get_min_version().
> 
> static void validate_mcu_fw_version(struct hid_device *hdev, int
> idProduct)
> {
> 
> 	int min_version;
> 
> 	switch(idProduct) {
> 	case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY:
> 		min_version = ROG_ALLY_MIN_MCU;
> 		break;
> 	case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLYX:
> 		min_version = ROG_ALLYX_MIN_MCU;
> 		break;
> 	default:
> 		return;
> 	}
> 
> 	.
> 	.
> 	.
> }
> 
> That way you have really straight forward logic that 
> validate_mcu_version only runs on devices that you specify.
> 

I actually had a switch/case to start with. Not sure why I changed it
now. I'll go back to that as it's very clear.

> > +
> >   static int asus_kbd_register_leds(struct hid_device *hdev)
> >   {
> >   	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> > +	struct usb_interface *intf;
> > +	struct usb_device *udev;
> >   	unsigned char kbd_func;
> >   	int ret;
> >   
> > @@ -560,6 +645,14 @@ static int asus_kbd_register_leds(struct
> > hid_device *hdev)
> >   			if (ret < 0)
> >   				return ret;
> >   		}
> > +
> > +		if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> > +			intf = to_usb_interface(hdev->dev.parent);
> > +			udev = interface_to_usbdev(intf);
> > +			validate_mcu_fw_version(hdev,
> > +				le16_to_cpu(udev-
> > >descriptor.idProduct));
> > +		}
> > +
> >   	} else {
> >   		/* Initialize keyboard */
> >   		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > @@ -1280,10 +1373,10 @@ static const struct hid_device_id
> > asus_devices[] = {
> >   	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> >   	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
> > -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> > +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD |
> > QUIRK_ROG_ALLY_XPAD},
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> >   	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
> > -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> > +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD |
> > QUIRK_ROG_ALLY_XPAD },
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> >   	    USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
> >   	  QUIRK_ROG_CLAYMORE_II_KEYBOARD },
> 

[PATCH 2/2] platform/x86: asus-wmi: Refactor Ally suspend/resume

[Luke Jones]

From: "Luke D. Jones" <luke@ljones.dev>

Adjust how the CSEE direct call hack is used.

The results of months of testing combined with help from ASUS to
determine the actual cause of suspend issues has resulted in this
refactoring which immensely improves the reliability for devices which
do not have the following minimum MCU FW version:
- ROG Ally X: 313
- ROG Ally 1: 319

For MCU FW versions that match the minimum or above the CSEE hack is
disabled and mcu_powersave set to on by default as there are no
negatives beyond a slightly slower device reinitialization due to the
MCU being powered off.

As this is set only at module load time, it is still possible for
mcu_powersave sysfs attributes to change it at runtime if so desired.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/hid/hid-asus.c                     |   4 +
 drivers/platform/x86/asus-wmi.c            | 124 ++++++++++++++-------
 include/linux/platform_data/x86/asus-wmi.h |  15 +++
 3 files changed, 104 insertions(+), 39 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index e1e60b80115a..58794c9024cf 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -614,6 +614,9 @@ static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
 			 "The MCU firmware version must be %d or greater\n"
 			 "Please update your MCU with official ASUS firmware release\n",
 			 min_version);
+	} else {
+		set_ally_mcu_hack(false);
+		set_ally_mcu_powersave(true);
 	}
 }
 
@@ -1420,4 +1423,5 @@ static struct hid_driver asus_driver = {
 };
 module_hid_driver(asus_driver);
 
+MODULE_IMPORT_NS("ASUS_WMI");
 MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 38ef778e8c19..9dba88a29e2c 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -142,16 +142,20 @@ module_param(fnlock_default, bool, 0444);
 #define ASUS_MINI_LED_2024_STRONG	0x01
 #define ASUS_MINI_LED_2024_OFF		0x02
 
-/* 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 1500
+/*
+ * The period required to wait after screen off/on/s2idle.check in MS.
+ * Time here greatly impacts the wake behaviour. Used in suspend/wake.
+ */
+#define ASUS_USB0_PWR_EC0_CSEE_WAIT	600
+#define ASUS_USB0_PWR_EC0_CSEE_OFF	0xB7
+#define ASUS_USB0_PWR_EC0_CSEE_ON	0xB8
 
 static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static int throttle_thermal_policy_write(struct asus_wmi *);
 
-static const struct dmi_system_id asus_ally_mcu_quirk[] = {
+static const struct dmi_system_id asus_rog_ally_device[] = {
 	{
 		.matches = {
 			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
@@ -274,9 +278,6 @@ struct asus_wmi {
 	u32 tablet_switch_dev_id;
 	bool tablet_switch_inverted;
 
-	/* The ROG Ally device requires the MCU USB device be disconnected before suspend */
-	bool ally_mcu_usb_switch;
-
 	enum fan_type fan_type;
 	enum fan_type gpu_fan_type;
 	enum fan_type mid_fan_type;
@@ -335,6 +336,9 @@ struct asus_wmi {
 	struct asus_wmi_driver *driver;
 };
 
+/* Global to allow setting externally without requiring driver data */
+static bool use_ally_mcu_hack;
+
 /* WMI ************************************************************************/
 
 static int asus_wmi_evaluate_method3(u32 method_id,
@@ -549,7 +553,7 @@ static int asus_wmi_get_devstate(struct asus_wmi *asus, u32 dev_id, u32 *retval)
 	return 0;
 }
 
-static int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param,
+int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param,
 				 u32 *retval)
 {
 	return asus_wmi_evaluate_method(ASUS_WMI_METHODID_DEVS, dev_id,
@@ -1343,6 +1347,38 @@ static ssize_t nv_temp_target_show(struct device *dev,
 static DEVICE_ATTR_RW(nv_temp_target);
 
 /* Ally MCU Powersave ********************************************************/
+
+/*
+ * The HID driver needs to check MCU version and set this to false if the MCU FW
+ * version is >= the minimum requirements. New FW do not need the hacks.
+ */
+void set_ally_mcu_hack(bool enabled)
+{
+	use_ally_mcu_hack = enabled;
+	pr_info("Disabled Ally MCU suspend quirks");
+}
+EXPORT_SYMBOL_NS_GPL(set_ally_mcu_hack, "ASUS_WMI");
+
+/*
+ * mcu_powersave should be enabled always, as it is fixed in MCU FW versions:
+ * - v313 for Ally X
+ * - v319 for Ally 1
+ * The HID driver checks MCU versions and so should set this if requirements match
+ */
+void set_ally_mcu_powersave(bool enabled)
+{
+	int result, err;
+
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_MCU_POWERSAVE, enabled, &result);
+	if (err)
+		pr_warn("Failed to set MCU powersave: %d\n", err);
+	if (result > 1)
+		pr_warn("Failed to set MCU powersave (result): 0x%x\n", result);
+
+	pr_info("Set mcu_powersave to enabled");
+}
+EXPORT_SYMBOL_NS_GPL(set_ally_mcu_powersave, "ASUS_WMI");
+
 static ssize_t mcu_powersave_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
@@ -4711,6 +4747,18 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_platform;
 
+	use_ally_mcu_hack = acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
+				&& dmi_check_system(asus_rog_ally_device);
+	if (use_ally_mcu_hack && dmi_match(DMI_BOARD_NAME, "RC71")) {
+		/*
+		 * These steps ensure the device is in a valid good state, this is
+		 * especially important for the Ally 1 after a reboot.
+		 */
+		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
+					   ASUS_USB0_PWR_EC0_CSEE_ON);
+		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
+	}
+
 	/* ensure defaults for tunables */
 	asus->ppt_pl2_sppt = 5;
 	asus->ppt_pl1_spl = 5;
@@ -4723,8 +4771,6 @@ static int asus_wmi_add(struct platform_device *pdev)
 	asus->egpu_enable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU);
 	asus->dgpu_disable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_DGPU);
 	asus->kbd_rgb_state_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE);
-	asus->ally_mcu_usb_switch = acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
-						&& dmi_check_system(asus_ally_mcu_quirk);
 
 	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_MINI_LED_MODE))
 		asus->mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
@@ -4910,34 +4956,6 @@ static int asus_hotk_resume(struct device *device)
 	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) {
-		/* sleep required to prevent USB0 being yanked then reappearing rapidly */
-		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);
-
-	if (asus->ally_mcu_usb_switch) {
-		/* 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;
-}
-
 static int asus_hotk_restore(struct device *device)
 {
 	struct asus_wmi *asus = dev_get_drvdata(device);
@@ -4978,11 +4996,34 @@ static int asus_hotk_restore(struct device *device)
 	return 0;
 }
 
+static void asus_ally_s2idle_restore(void)
+{
+	if (use_ally_mcu_hack) {
+		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
+					   ASUS_USB0_PWR_EC0_CSEE_ON);
+		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
+	}
+}
+
+static int asus_hotk_prepare(struct device *device)
+{
+	if (use_ally_mcu_hack) {
+		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
+					   ASUS_USB0_PWR_EC0_CSEE_OFF);
+		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
+	}
+	return 0;
+}
+
+/* Use only for Ally devices due to the wake_on_ac */
+static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = {
+	.restore = asus_ally_s2idle_restore,
+};
+
 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,
 };
 
@@ -5010,6 +5051,10 @@ static int asus_wmi_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops);
+	if (ret)
+		pr_warn("failed to register LPS0 sleep handler in asus-wmi\n");
+
 	return asus_wmi_add(pdev);
 }
 
@@ -5042,6 +5087,7 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver);
 
 void asus_wmi_unregister_driver(struct asus_wmi_driver *driver)
 {
+	acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops);
 	platform_device_unregister(driver->platform_device);
 	platform_driver_unregister(&driver->platform_driver);
 	used = false;
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 783e2a336861..a32cb8865b2f 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -158,8 +158,23 @@
 #define ASUS_WMI_DSTS_LIGHTBAR_MASK	0x0000000F
 
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
+void set_ally_mcu_hack(bool enabled);
+void set_ally_mcu_powersave(bool enabled);
+int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
 int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
 #else
+static inline void set_ally_mcu_hack(bool enabled)
+{
+	return -ENODEV;
+}
+static inline void set_ally_mcu_powersave(bool enabled)
+{
+	return -ENODEV;
+}
+static inline int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval)
+{
+	return -ENODEV;
+}
 static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
 					   u32 *retval)
 {
-- 
2.48.1

RE: [PATCH 2/2] platform/x86: asus-wmi: Refactor Ally suspend/resume

[Antheas Kapenekakis]

Hi Luke,
setting MCU powersave too close to the boot-up sequence can cause the
controller of the original Ally to malfunction. Which is why you created
this little init sequence in which you call CSEE manually. In addition,
MCU powersave (which is called Extreme Standby in Windows and you named
incorrectly in asus-wmi), causes a very large 8 second delay in the resume
process. It should under no circumstance be set enabled by default.

Users that want it can enable it manually. Following, distributions that
want it and lack the appropriate configuration interface can use a udev
rule with an 8 second delay to enable it, and then, the udev rule should
first check if mcu_powersave is disabled before enabling it. This is
because writing to it even with the same value causes an issue regardless.

Therefore, please remove both parts of it from the second patch in your
series and produce a v2, which contains no hints of CSEE. When you do:

Suggested-by: Antheas Kapenekakis <lkml@antheas.dev>

Following, when you do finish the new Asus Armoury patch series, please
rename MCU powersave to extreme standby, and add a suggested-by in the
appropriate patch. Since to avoid user confusion, the names should match
their windows branding.

During our testing of the controller, we found that the lack of a delay
causes the RGB of both the Ally and the Ally X to malfunction, so this is
a small nack for me (the old quirk is preferable in that regard). But then
again, this patch series is not getting anywhere close to our users even
if it is accepted, so you can do as you wish (given appropriate attribution).

Best,
Antheas

Re: [PATCH 2/2] platform/x86: asus-wmi: Refactor Ally suspend/resume

[Luke Jones]

On Tue, 2025-02-25 at 15:15 +0100, Antheas Kapenekakis wrote:
> Hi Luke,
> setting MCU powersave too close to the boot-up sequence can cause the
> controller of the original Ally to malfunction.

Read the code. It doesn't, as it is set by hid driver. Zero issues in
all our testing.

> Which is why you created
> this little init sequence in which you call CSEE manually. In
> addition,

No it wasn't.

> MCU powersave (which is called Extreme Standby in Windows and you
> named
> incorrectly in asus-wmi), causes a very large 8 second delay in the

That is a UI label. ASUS call it Mcu Powersave in internal emails to
me. It is also called *MCUPowerSaving in their source code.

> resume
> process. It should under no circumstance be set enabled by default.
> 
> Users that want it can enable it manually. Following, distributions
> that
> want it and lack the appropriate configuration interface can use a
> udev
> rule with an 8 second delay to enable it, and then, the udev rule
> should
> first check if mcu_powersave is disabled before enabling it. This is
> because writing to it even with the same value causes an issue
> regardless.
> 

No it does not.

> Therefore, please remove both parts of it from the second patch in
> your
> series and produce a v2, which contains no hints of CSEE. When you
> do:
> 
> Suggested-by: Antheas Kapenekakis <lkml@antheas.dev>
> 

No. You've suggested changes with zero testing, simply in an attempt to
get a tag.

> Following, when you do finish the new Asus Armoury patch series,
> please
> rename MCU powersave to extreme standby, and add a suggested-by in
> the
> appropriate patch. Since to avoid user confusion, the names should
> match
> their windows branding.
> 

No. See first response.

> During our testing of the controller, we found that the lack of a
> delay
> causes the RGB of both the Ally and the Ally X to malfunction, so

This is to do with your own code in userspace. 

> this is
> a small nack for me (the old quirk is preferable in that regard). But
> then
> again, this patch series is not getting anywhere close to our users
> even
> if it is accepted, so you can do as you wish (given appropriate
> attribution).
> 

Your message attempts to frame this as a personal matter ("small nack
for me," "you can do as you wish") rather than providing substantive
technical feedback appropriate for LKML.

To be clear: This patch is submitted for mainline Linux kernel
consideration, not your downstream project. Your NACK lacks technical
merit relevant to mainline development, as you yourself acknowledge
these changes would not affect your users.

As I am the maintainer on this driver I will proceed with the
submission process as there are no valid technical objections to
address. Further non-technical commentary on this thread is
unnecessary.

Cheers,
Luke.

Re: [PATCH 2/2] platform/x86: asus-wmi: Refactor Ally suspend/resume

[Antheas Kapenekakis]

Hi Luke,
using your maintainer status and your NDA creds to veto my comments
will not go well for you if you end up breaking the driver.

Yes, I have done extensive testing with setting the powersave
parameter. I have supported it since September. It is very easy to
break the controller if you start writing to it randomly. In fact,
when I pushed it out [1], it never got out of testing because I got
three user reports about it so I had to make a revised version [2].
Notice there is a day difference between the versions. We never needed
a boot workaround for the original ally, and if you ask me, the reason
your driver here includes that is because in all your testing you set
the powersave parameter.

When it comes to RGB, without a quirk it looks to me as if the MCU
hardlocks and stops accepting RGB commands. I am pretty sure I wrote
to it manually, but excuse me if I am wrong as there have been 5
months since I did the testing for it.

In your new armoury driver, you tried to make internal names
accessible to users via the BIOS interface to make it more user
friendly. In this case, you need to use the names Asus uses in
Windows, otherwise users will get confused. Failing that, asus-wmi is
a perfectly fine driver as it is, with mcu_powersave as an internal
parameter.

As far as I am concerned, the whole Ally issue boils down to the
Modern Standby firmware notifications in the Linux kernel being in the
wrong place. You can see it in the Microsoft documentation [3]. The
Display Off call should happen when the display turns off, not after
the kernel has suspended. This change will need to eventually happen,
as Asus is not the only handheld manufacturer doing this. All
manufacturers turn off their xpad controller via those notifications,
including Lenovo, GPD, OneXPlayer, and MSI. Fortunately, this issue
seems to only affect asus in a major way, although I suspect it causes
a quality degradation in other manufacturers as well.

Given that I do have a custom kernel now and i can rewrite it as I see
fit, I am giving you carte blanche when it comes to the mainline
kernel. As far as I am concerned, my Ally 1st gen/X integration
finished last November. So I will not be revisiting it or doing any
additional testing to validate any new claims or changes. I will just
be removing them and pointing users to my kernel.

Best,
Antheas

[1] https://github.com/hhd-dev/adjustor/releases/tag/v3.5.0
[2] https://github.com/hhd-dev/adjustor/releases/tag/v3.5.1
[3] https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-firmware-notifications

Re: [PATCH 2/2] platform/x86: asus-wmi: Refactor Ally suspend/resume

[Mario Limonciello]

On 2/25/2025 00:17, Luke Jones wrote:
> From: "Luke D. Jones" <luke@ljones.dev>
> 
> Adjust how the CSEE direct call hack is used.
> 
> The results of months of testing combined with help from ASUS to
> determine the actual cause of suspend issues has resulted in this
> refactoring which immensely improves the reliability for devices which
> do not have the following minimum MCU FW version:
> - ROG Ally X: 313
> - ROG Ally 1: 319
> 
> For MCU FW versions that match the minimum or above the CSEE hack is
> disabled and mcu_powersave set to on by default as there are no
> negatives beyond a slightly slower device reinitialization due to the
> MCU being powered off.
> 
> As this is set only at module load time, it is still possible for
> mcu_powersave sysfs attributes to change it at runtime if so desired.
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>   drivers/hid/hid-asus.c                     |   4 +
>   drivers/platform/x86/asus-wmi.c            | 124 ++++++++++++++-------
>   include/linux/platform_data/x86/asus-wmi.h |  15 +++
>   3 files changed, 104 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e1e60b80115a..58794c9024cf 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -614,6 +614,9 @@ static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
>   			 "The MCU firmware version must be %d or greater\n"
>   			 "Please update your MCU with official ASUS firmware release\n",
>   			 min_version);
> +	} else {
> +		set_ally_mcu_hack(false);

Rather than calling this to set a global, how about just unregistering 
the s2idle devops?

> +		set_ally_mcu_powersave(true);
>   	}
>   }
>   
> @@ -1420,4 +1423,5 @@ static struct hid_driver asus_driver = {
>   };
>   module_hid_driver(asus_driver);
>   
> +MODULE_IMPORT_NS("ASUS_WMI");
>   MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 38ef778e8c19..9dba88a29e2c 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -142,16 +142,20 @@ module_param(fnlock_default, bool, 0444);
>   #define ASUS_MINI_LED_2024_STRONG	0x01
>   #define ASUS_MINI_LED_2024_OFF		0x02
>   
> -/* 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 1500
> +/*
> + * The period required to wait after screen off/on/s2idle.check in MS.
> + * Time here greatly impacts the wake behaviour. Used in suspend/wake.
> + */
> +#define ASUS_USB0_PWR_EC0_CSEE_WAIT	600
> +#define ASUS_USB0_PWR_EC0_CSEE_OFF	0xB7
> +#define ASUS_USB0_PWR_EC0_CSEE_ON	0xB8
>   
>   static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
>   
>   static int throttle_thermal_policy_write(struct asus_wmi *);
>   
> -static const struct dmi_system_id asus_ally_mcu_quirk[] = {
> +static const struct dmi_system_id asus_rog_ally_device[] = {
>   	{
>   		.matches = {
>   			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> @@ -274,9 +278,6 @@ struct asus_wmi {
>   	u32 tablet_switch_dev_id;
>   	bool tablet_switch_inverted;
>   
> -	/* The ROG Ally device requires the MCU USB device be disconnected before suspend */
> -	bool ally_mcu_usb_switch;
> -
>   	enum fan_type fan_type;
>   	enum fan_type gpu_fan_type;
>   	enum fan_type mid_fan_type;
> @@ -335,6 +336,9 @@ struct asus_wmi {
>   	struct asus_wmi_driver *driver;
>   };
>   
> +/* Global to allow setting externally without requiring driver data */
> +static bool use_ally_mcu_hack;
> +
>   /* WMI ************************************************************************/
>   
>   static int asus_wmi_evaluate_method3(u32 method_id,
> @@ -549,7 +553,7 @@ static int asus_wmi_get_devstate(struct asus_wmi *asus, u32 dev_id, u32 *retval)
>   	return 0;
>   }
>   
> -static int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param,
> +int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param,
>   				 u32 *retval)
>   {
>   	return asus_wmi_evaluate_method(ASUS_WMI_METHODID_DEVS, dev_id,
> @@ -1343,6 +1347,38 @@ static ssize_t nv_temp_target_show(struct device *dev,
>   static DEVICE_ATTR_RW(nv_temp_target);
>   
>   /* Ally MCU Powersave ********************************************************/
> +
> +/*
> + * The HID driver needs to check MCU version and set this to false if the MCU FW
> + * version is >= the minimum requirements. New FW do not need the hacks.
> + */
> +void set_ally_mcu_hack(bool enabled)
> +{
> +	use_ally_mcu_hack = enabled;
> +	pr_info("Disabled Ally MCU suspend quirks");
> +}
> +EXPORT_SYMBOL_NS_GPL(set_ally_mcu_hack, "ASUS_WMI");
> +
> +/*
> + * mcu_powersave should be enabled always, as it is fixed in MCU FW versions:
> + * - v313 for Ally X
> + * - v319 for Ally 1
> + * The HID driver checks MCU versions and so should set this if requirements match
> + */
> +void set_ally_mcu_powersave(bool enabled)
> +{
> +	int result, err;
> +
> +	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_MCU_POWERSAVE, enabled, &result);
> +	if (err)
> +		pr_warn("Failed to set MCU powersave: %d\n", err);
> +	if (result > 1)
> +		pr_warn("Failed to set MCU powersave (result): 0x%x\n", result);
> +
> +	pr_info("Set mcu_powersave to enabled");
> +}
> +EXPORT_SYMBOL_NS_GPL(set_ally_mcu_powersave, "ASUS_WMI");
> +
>   static ssize_t mcu_powersave_show(struct device *dev,
>   				   struct device_attribute *attr, char *buf)
>   {
> @@ -4711,6 +4747,18 @@ static int asus_wmi_add(struct platform_device *pdev)
>   	if (err)
>   		goto fail_platform;
>   
> +	use_ally_mcu_hack = acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
> +				&& dmi_check_system(asus_rog_ally_device);
> +	if (use_ally_mcu_hack && dmi_match(DMI_BOARD_NAME, "RC71")) {
> +		/*
> +		 * These steps ensure the device is in a valid good state, this is
> +		 * especially important for the Ally 1 after a reboot.
> +		 */
> +		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
> +					   ASUS_USB0_PWR_EC0_CSEE_ON);
> +		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
> +	}
> +
>   	/* ensure defaults for tunables */
>   	asus->ppt_pl2_sppt = 5;
>   	asus->ppt_pl1_spl = 5;
> @@ -4723,8 +4771,6 @@ static int asus_wmi_add(struct platform_device *pdev)
>   	asus->egpu_enable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU);
>   	asus->dgpu_disable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_DGPU);
>   	asus->kbd_rgb_state_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE);
> -	asus->ally_mcu_usb_switch = acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
> -						&& dmi_check_system(asus_ally_mcu_quirk);
>   
>   	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_MINI_LED_MODE))
>   		asus->mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
> @@ -4910,34 +4956,6 @@ static int asus_hotk_resume(struct device *device)
>   	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) {
> -		/* sleep required to prevent USB0 being yanked then reappearing rapidly */
> -		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);
> -
> -	if (asus->ally_mcu_usb_switch) {
> -		/* 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;
> -}
> -
>   static int asus_hotk_restore(struct device *device)
>   {
>   	struct asus_wmi *asus = dev_get_drvdata(device);
> @@ -4978,11 +4996,34 @@ static int asus_hotk_restore(struct device *device)
>   	return 0;
>   }
>   
> +static void asus_ally_s2idle_restore(void)
> +{
> +	if (use_ally_mcu_hack) {
> +		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
> +					   ASUS_USB0_PWR_EC0_CSEE_ON);
> +		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
> +	}
> +}
> +
> +static int asus_hotk_prepare(struct device *device)
> +{
> +	if (use_ally_mcu_hack) {
> +		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
> +					   ASUS_USB0_PWR_EC0_CSEE_OFF);
> +		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
> +	}
> +	return 0;
> +}
> +
> +/* Use only for Ally devices due to the wake_on_ac */
> +static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = {
> +	.restore = asus_ally_s2idle_restore,
> +};
> +
>   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,
>   };
>   
> @@ -5010,6 +5051,10 @@ static int asus_wmi_probe(struct platform_device *pdev)
>   			return ret;
>   	}
>   
> +	ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops);
> +	if (ret)
> +		pr_warn("failed to register LPS0 sleep handler in asus-wmi\n");
> +
>   	return asus_wmi_add(pdev);
>   }
>   
> @@ -5042,6 +5087,7 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver);
>   
>   void asus_wmi_unregister_driver(struct asus_wmi_driver *driver)
>   {
> +	acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops);
>   	platform_device_unregister(driver->platform_device);
>   	platform_driver_unregister(&driver->platform_driver);
>   	used = false;
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 783e2a336861..a32cb8865b2f 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -158,8 +158,23 @@
>   #define ASUS_WMI_DSTS_LIGHTBAR_MASK	0x0000000F
>   
>   #if IS_REACHABLE(CONFIG_ASUS_WMI)
> +void set_ally_mcu_hack(bool enabled);
> +void set_ally_mcu_powersave(bool enabled);
> +int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
>   int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
>   #else
> +static inline void set_ally_mcu_hack(bool enabled)
> +{
> +	return -ENODEV;
> +}
> +static inline void set_ally_mcu_powersave(bool enabled)
> +{
> +	return -ENODEV;
> +}
> +static inline int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval)
> +{
> +	return -ENODEV;
> +}
>   static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
>   					   u32 *retval)
>   {

Re: [PATCH 2/2] platform/x86: asus-wmi: Refactor Ally suspend/resume

[Luke Jones]

On Tue, 2025-02-25 at 07:18 -0800, Mario Limonciello wrote:
> On 2/25/2025 00:17, Luke Jones wrote:
> > From: "Luke D. Jones" <luke@ljones.dev>
> > 
> > Adjust how the CSEE direct call hack is used.
> > 
> > The results of months of testing combined with help from ASUS to
> > determine the actual cause of suspend issues has resulted in this
> > refactoring which immensely improves the reliability for devices
> > which
> > do not have the following minimum MCU FW version:
> > - ROG Ally X: 313
> > - ROG Ally 1: 319
> > 
> > For MCU FW versions that match the minimum or above the CSEE hack
> > is
> > disabled and mcu_powersave set to on by default as there are no
> > negatives beyond a slightly slower device reinitialization due to
> > the
> > MCU being powered off.
> > 
> > As this is set only at module load time, it is still possible for
> > mcu_powersave sysfs attributes to change it at runtime if so
> > desired.
> > 
> > Signed-off-by: Luke D. Jones <luke@ljones.dev>
> > ---
> >   drivers/hid/hid-asus.c                     |   4 +
> >   drivers/platform/x86/asus-wmi.c            | 124 ++++++++++++++--
> > -----
> >   include/linux/platform_data/x86/asus-wmi.h |  15 +++
> >   3 files changed, 104 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index e1e60b80115a..58794c9024cf 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -614,6 +614,9 @@ static void validate_mcu_fw_version(struct
> > hid_device *hdev, int idProduct)
> >   			 "The MCU firmware version must be %d or
> > greater\n"
> >   			 "Please update your MCU with official
> > ASUS firmware release\n",
> >   			 min_version);
> > +	} else {
> > +		set_ally_mcu_hack(false);
> 
> Rather than calling this to set a global, how about just
> unregistering 
> the s2idle devops?
> 

The main reason would be because `dev_pm_ops` is used to activate the
hack also and I need to block that too. This seemed the safest and
easiest way.

Ideally I would just remove the entire hack, but as there can still be
a few people out there with older versions I don't think that is wise
at all. Maybe in 6 months times we can revisit it.

Cheers,
Luke.

> > +		set_ally_mcu_powersave(true);
> >   	}
> >   }
> >   
> > @@ -1420,4 +1423,5 @@ static struct hid_driver asus_driver = {
> >   };
> >   module_hid_driver(asus_driver);
> >   
> > +MODULE_IMPORT_NS("ASUS_WMI");
> >   MODULE_LICENSE("GPL");
> > diff --git a/drivers/platform/x86/asus-wmi.c
> > b/drivers/platform/x86/asus-wmi.c
> > index 38ef778e8c19..9dba88a29e2c 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -142,16 +142,20 @@ module_param(fnlock_default, bool, 0444);
> >   #define ASUS_MINI_LED_2024_STRONG	0x01
> >   #define ASUS_MINI_LED_2024_OFF		0x02
> >   
> > -/* 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 1500
> > +/*
> > + * The period required to wait after screen off/on/s2idle.check in
> > MS.
> > + * Time here greatly impacts the wake behaviour. Used in
> > suspend/wake.
> > + */
> > +#define ASUS_USB0_PWR_EC0_CSEE_WAIT	600
> > +#define ASUS_USB0_PWR_EC0_CSEE_OFF	0xB7
> > +#define ASUS_USB0_PWR_EC0_CSEE_ON	0xB8
> >   
> >   static const char * const ashs_ids[] = { "ATK4001", "ATK4002",
> > NULL };
> >   
> >   static int throttle_thermal_policy_write(struct asus_wmi *);
> >   
> > -static const struct dmi_system_id asus_ally_mcu_quirk[] = {
> > +static const struct dmi_system_id asus_rog_ally_device[] = {
> >   	{
> >   		.matches = {
> >   			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> > @@ -274,9 +278,6 @@ struct asus_wmi {
> >   	u32 tablet_switch_dev_id;
> >   	bool tablet_switch_inverted;
> >   
> > -	/* The ROG Ally device requires the MCU USB device be
> > disconnected before suspend */
> > -	bool ally_mcu_usb_switch;
> > -
> >   	enum fan_type fan_type;
> >   	enum fan_type gpu_fan_type;
> >   	enum fan_type mid_fan_type;
> > @@ -335,6 +336,9 @@ struct asus_wmi {
> >   	struct asus_wmi_driver *driver;
> >   };
> >   
> > +/* Global to allow setting externally without requiring driver
> > data */
> > +static bool use_ally_mcu_hack;
> > +
> >   /* WMI
> > *******************************************************************
> > *****/
> >   
> >   static int asus_wmi_evaluate_method3(u32 method_id,
> > @@ -549,7 +553,7 @@ static int asus_wmi_get_devstate(struct
> > asus_wmi *asus, u32 dev_id, u32 *retval)
> >   	return 0;
> >   }
> >   
> > -static int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param,
> > +int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param,
> >   				 u32 *retval)
> >   {
> >   	return asus_wmi_evaluate_method(ASUS_WMI_METHODID_DEVS,
> > dev_id,
> > @@ -1343,6 +1347,38 @@ static ssize_t nv_temp_target_show(struct
> > device *dev,
> >   static DEVICE_ATTR_RW(nv_temp_target);
> >   
> >   /* Ally MCU Powersave
> > ********************************************************/
> > +
> > +/*
> > + * The HID driver needs to check MCU version and set this to false
> > if the MCU FW
> > + * version is >= the minimum requirements. New FW do not need the
> > hacks.
> > + */
> > +void set_ally_mcu_hack(bool enabled)
> > +{
> > +	use_ally_mcu_hack = enabled;
> > +	pr_info("Disabled Ally MCU suspend quirks");
> > +}
> > +EXPORT_SYMBOL_NS_GPL(set_ally_mcu_hack, "ASUS_WMI");
> > +
> > +/*
> > + * mcu_powersave should be enabled always, as it is fixed in MCU
> > FW versions:
> > + * - v313 for Ally X
> > + * - v319 for Ally 1
> > + * The HID driver checks MCU versions and so should set this if
> > requirements match
> > + */
> > +void set_ally_mcu_powersave(bool enabled)
> > +{
> > +	int result, err;
> > +
> > +	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_MCU_POWERSAVE,
> > enabled, &result);
> > +	if (err)
> > +		pr_warn("Failed to set MCU powersave: %d\n", err);
> > +	if (result > 1)
> > +		pr_warn("Failed to set MCU powersave (result):
> > 0x%x\n", result);
> > +
> > +	pr_info("Set mcu_powersave to enabled");
> > +}
> > +EXPORT_SYMBOL_NS_GPL(set_ally_mcu_powersave, "ASUS_WMI");
> > +
> >   static ssize_t mcu_powersave_show(struct device *dev,
> >   				   struct device_attribute *attr,
> > char *buf)
> >   {
> > @@ -4711,6 +4747,18 @@ static int asus_wmi_add(struct
> > platform_device *pdev)
> >   	if (err)
> >   		goto fail_platform;
> >   
> > +	use_ally_mcu_hack = acpi_has_method(NULL,
> > ASUS_USB0_PWR_EC0_CSEE)
> > +				&&
> > dmi_check_system(asus_rog_ally_device);
> > +	if (use_ally_mcu_hack && dmi_match(DMI_BOARD_NAME,
> > "RC71")) {
> > +		/*
> > +		 * These steps ensure the device is in a valid
> > good state, this is
> > +		 * especially important for the Ally 1 after a
> > reboot.
> > +		 */
> > +		acpi_execute_simple_method(NULL,
> > ASUS_USB0_PWR_EC0_CSEE,
> > +					  
> > ASUS_USB0_PWR_EC0_CSEE_ON);
> > +		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
> > +	}
> > +
> >   	/* ensure defaults for tunables */
> >   	asus->ppt_pl2_sppt = 5;
> >   	asus->ppt_pl1_spl = 5;
> > @@ -4723,8 +4771,6 @@ static int asus_wmi_add(struct
> > platform_device *pdev)
> >   	asus->egpu_enable_available =
> > asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU);
> >   	asus->dgpu_disable_available =
> > asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_DGPU);
> >   	asus->kbd_rgb_state_available =
> > asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE);
> > -	asus->ally_mcu_usb_switch = acpi_has_method(NULL,
> > ASUS_USB0_PWR_EC0_CSEE)
> > -						&&
> > dmi_check_system(asus_ally_mcu_quirk);
> >   
> >   	if (asus_wmi_dev_is_present(asus,
> > ASUS_WMI_DEVID_MINI_LED_MODE))
> >   		asus->mini_led_dev_id =
> > ASUS_WMI_DEVID_MINI_LED_MODE;
> > @@ -4910,34 +4956,6 @@ static int asus_hotk_resume(struct device
> > *device)
> >   	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) {
> > -		/* sleep required to prevent USB0 being yanked
> > then reappearing rapidly */
> > -		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);
> > -
> > -	if (asus->ally_mcu_usb_switch) {
> > -		/* 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;
> > -}
> > -
> >   static int asus_hotk_restore(struct device *device)
> >   {
> >   	struct asus_wmi *asus = dev_get_drvdata(device);
> > @@ -4978,11 +4996,34 @@ static int asus_hotk_restore(struct device
> > *device)
> >   	return 0;
> >   }
> >   
> > +static void asus_ally_s2idle_restore(void)
> > +{
> > +	if (use_ally_mcu_hack) {
> > +		acpi_execute_simple_method(NULL,
> > ASUS_USB0_PWR_EC0_CSEE,
> > +					  
> > ASUS_USB0_PWR_EC0_CSEE_ON);
> > +		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
> > +	}
> > +}
> > +
> > +static int asus_hotk_prepare(struct device *device)
> > +{
> > +	if (use_ally_mcu_hack) {
> > +		acpi_execute_simple_method(NULL,
> > ASUS_USB0_PWR_EC0_CSEE,
> > +					  
> > ASUS_USB0_PWR_EC0_CSEE_OFF);
> > +		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
> > +	}
> > +	return 0;
> > +}
> > +
> > +/* Use only for Ally devices due to the wake_on_ac */
> > +static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = {
> > +	.restore = asus_ally_s2idle_restore,
> > +};
> > +
> >   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,
> >   };
> >   
> > @@ -5010,6 +5051,10 @@ static int asus_wmi_probe(struct
> > platform_device *pdev)
> >   			return ret;
> >   	}
> >   
> > +	ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops);
> > +	if (ret)
> > +		pr_warn("failed to register LPS0 sleep handler in
> > asus-wmi\n");
> > +
> >   	return asus_wmi_add(pdev);
> >   }
> >   
> > @@ -5042,6 +5087,7 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver);
> >   
> >   void asus_wmi_unregister_driver(struct asus_wmi_driver *driver)
> >   {
> > +	acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops);
> >   	platform_device_unregister(driver->platform_device);
> >   	platform_driver_unregister(&driver->platform_driver);
> >   	used = false;
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h
> > b/include/linux/platform_data/x86/asus-wmi.h
> > index 783e2a336861..a32cb8865b2f 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -158,8 +158,23 @@
> >   #define ASUS_WMI_DSTS_LIGHTBAR_MASK	0x0000000F
> >   
> >   #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > +void set_ally_mcu_hack(bool enabled);
> > +void set_ally_mcu_powersave(bool enabled);
> > +int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32
> > *retval);
> >   int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
> > u32 *retval);
> >   #else
> > +static inline void set_ally_mcu_hack(bool enabled)
> > +{
> > +	return -ENODEV;
> > +}
> > +static inline void set_ally_mcu_powersave(bool enabled)
> > +{
> > +	return -ENODEV;
> > +}
> > +static inline int asus_wmi_set_devstate(u32 dev_id, u32
> > ctrl_param, u32 *retval)
> > +{
> > +	return -ENODEV;
> > +}
> >   static inline int asus_wmi_evaluate_method(u32 method_id, u32
> > arg0, u32 arg1,
> >   					   u32 *retval)
> >   {
>