| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <ZOTS4S.OUOP1DLTNVXP3@ljones.dev>
Date: Tue, 28 Nov 2023 09:17:23 +1300
From: Luke Jones <luke@...nes.dev>
To: Hans de Goede <hdegoede@...hat.com>
Cc: ilpo.jarvinen@...ux.intel.com, corentin.chary@...il.com,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] platform/x86: asus-wmi: disable USB0 hub on ROG
Ally before suspend
On Mon, Nov 27 2023 at 09:53:13 AM +01:00:00, Hans de Goede
<hdegoede@...hat.com> 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@...nes.dev>
>
> 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?
>
> With that fixed you can add my:
>
> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
>
> 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 */
>
Powered by blists - more mailing lists