| Message ID | 20250321035106.26752-1-luke@ljones.dev |
|---|---|
| Headers | show |
| Series | hid-asus: asus-wmi: refactor Ally suspend/resume | expand |
On 21/03/25 16:51, Luke Jones wrote: > 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. It is *strongly* preferred to remove the hack completely, but this may take some time after this series lands for it to be viable without regressing users. I thought I had added to above to cover-letter but did not, sorry all. The aim is to remove the hack at some stage this year. > - Changelog: > + V2: > - Adjust warning message to explicitly mention suspend issues > - Use switch/case block to set min_version > - Set min_version to 0 by default and toggle hacks off > + V3: > - Fix errors picked up by test bot: incorrect return in the else block > of `#if IS_REACHABLE(CONFIG_ASUS_WMI)`: > - set_ally_mcu_hack() > - set_ally_mcu_powersave() > > 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 | 111 +++++++++++++++++- > drivers/platform/x86/asus-wmi.c | 130 ++++++++++++++------- > include/linux/platform_data/x86/asus-wmi.h | 13 +++ > 3 files changed, 213 insertions(+), 41 deletions(-) >
On 3/20/2025 22:51, 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> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/hid/hid-asus.c | 107 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 105 insertions(+), 2 deletions(-) > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c > index 46e3e42f9eb5..599c836507ff 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,99 @@ 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 > + * The bytes "5a 05 03 31 00 1a 13" and possibly more come before the version > + * string, and there may be additional bytes after the version string such as > + * "75 00 74 00 65 00" or a postfix such as "_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, version; > + char buf[4]; > + > + dots = 0; > + while (p < end && dots < 2) { > + if (*p++ == '.') > + dots++; > + } > + > + if (dots != 2 || p >= end || (p + 3) >= end) > + return -EINVAL; > + > + memcpy(buf, p, 3); > + buf[3] = '\0'; > + > + err = kstrtoint(buf, 10, &version); > + if (err || version < 0) > + return -EINVAL; > + > + return version; > +} > + > +static int mcu_request_version(struct hid_device *hdev) > +{ > + u8 *response __free(kfree) = kzalloc(ROG_ALLY_REPORT_SIZE, GFP_KERNEL); > + const u8 request[] = { 0x5a, 0x05, 0x03, 0x31, 0x00, 0x20 }; > + int ret; > + > + if (!response) > + return -ENOMEM; > + > + ret = asus_kbd_set_report(hdev, request, sizeof(request)); > + if (ret < 0) > + return ret; > + > + ret = hid_hw_raw_request(hdev, FEATURE_REPORT_ID, response, > + ROG_ALLY_REPORT_SIZE, HID_FEATURE_REPORT, > + HID_REQ_GET_REPORT); > + if (ret < 0) > + return ret; > + > + ret = mcu_parse_version_string(response, ROG_ALLY_REPORT_SIZE); > + if (ret < 0) { > + pr_err("Failed to parse MCU version: %d\n", ret); > + print_hex_dump(KERN_ERR, "MCU: ", DUMP_PREFIX_NONE, > + 16, 1, response, ROG_ALLY_REPORT_SIZE, false); > + } > + > + return ret; > +} > + > +static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct) > +{ > + int min_version, version; > + > + version = mcu_request_version(hdev); > + if (version < 0) > + return; > + > + switch (idProduct) { > + case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY: > + min_version = ROG_ALLY_MIN_MCU; > + break; > + case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X: > + min_version = ROG_ALLY_X_MIN_MCU; > + break; > + default: > + min_version = 0; > + } > + > + if (version < min_version) { > + hid_warn(hdev, > + "The MCU firmware version must be %d or greater to avoid issues with suspend.\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 +655,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 +1383,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 },
On Fri, 21 Mar 2025 at 04:51, Luke Jones <luke@ljones.dev> wrote: > > 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. > > - Changelog: > + V2: > - Adjust warning message to explicitly mention suspend issues > - Use switch/case block to set min_version > - Set min_version to 0 by default and toggle hacks off > + V3: > - Fix errors picked up by test bot: incorrect return in the else block > of `#if IS_REACHABLE(CONFIG_ASUS_WMI)`: > - set_ally_mcu_hack() > - set_ally_mcu_powersave() > > 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 | 111 +++++++++++++++++- > drivers/platform/x86/asus-wmi.c | 130 ++++++++++++++------- > include/linux/platform_data/x86/asus-wmi.h | 13 +++ > 3 files changed, 213 insertions(+), 41 deletions(-) > > -- > 2.49.0 > Since I have to also test my series on my ally and booted a dev environment, I am also giving this a go. I'll post some results in a bit Antheas
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. - Changelog: + V2: - Adjust warning message to explicitly mention suspend issues - Use switch/case block to set min_version - Set min_version to 0 by default and toggle hacks off + V3: - Fix errors picked up by test bot: incorrect return in the else block of `#if IS_REACHABLE(CONFIG_ASUS_WMI)`: - set_ally_mcu_hack() - set_ally_mcu_powersave() 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 | 111 +++++++++++++++++- drivers/platform/x86/asus-wmi.c | 130 ++++++++++++++------- include/linux/platform_data/x86/asus-wmi.h | 13 +++ 3 files changed, 213 insertions(+), 41 deletions(-)