mbox series

[v2,0/1] platform/x86: asus-wmi: disable USB0 hub on ROG Ally before suspend

Message ID 20231126230521.125708-1-luke@ljones.dev
Headers
Series platform/x86: asus-wmi: disable USB0 hub on ROG Ally before suspend |

Message

Luke Jones Nov. 26, 2023, 11:05 p.m. UTC 
  This is a fix for the ROG Ally not being able to use the N-Key device after a suspend/resume cycle.

The root of the issue is that ASUS changed the MCU firmware to dfisconnect the USB0 hub when the
screen is switched off during the s2idle suspend path. I tried many many different tactics to try
and get this s2idle part to work but it seems there are races between this and other subsystems.

What has so far been reliable and consistent is a manual call to the CSEE method that is called in
ACPI by the Microsoft DSM screen off path followed by a short sleep in asus-wmi. The PM prepare
hook looks to be the earliest possible place. A sleep that is too long ends up with USB subsystem
registering a disconnect, and thus on resume the device paths change. Too short and it is unreliable.

Some discussion regarding this mess is at https://gitlab.freedesktop.org/drm/amd/-/issues/2719#note_2181402

Changelog:
- v2:
  - Emit dev_err(), but don't return error codes
  - Add check for MCU power save mode being enabled and disable if active
  - General cleanup and rename some vars/funcs
- v1: https://lore.kernel.org/all/20231124082749.23353-1-luke@ljones.dev/

Luke D. Jones (1):
  platform/x86: asus-wmi: disable USB0 hub on ROG Ally before suspend

 drivers/platform/x86/asus-wmi.c            | 50 ++++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |  3 ++
 2 files changed, 53 insertions(+)

Comments

Luke Jones Nov. 26, 2023, 11:10 p.m. UTC | #1 
On Mon, Nov 27 2023 at 12:05:20 PM +13:00:00, Luke D. Jones 
<luke@ljones.dev> wrote:
> This is a fix for the ROG Ally not being able to use the N-Key device 
> after a suspend/resume cycle.
> 
> The root of the issue is that ASUS changed the MCU firmware to 
> dfisconnect the USB0 hub when the
> screen is switched off during the s2idle suspend path. I tried many 
> many different tactics to try
> and get this s2idle part to work but it seems there are races between 
> this and other subsystems.
> 
> What has so far been reliable and consistent is a manual call to the 
> CSEE method that is called in
> ACPI by the Microsoft DSM screen off path followed by a short sleep 
> in asus-wmi. The PM prepare
> hook looks to be the earliest possible place. A sleep that is too 
> long ends up with USB subsystem
> registering a disconnect, and thus on resume the device paths change. 
> Too short and it is unreliable.
> 
> Some discussion regarding this mess is at 
> https://gitlab.freedesktop.org/drm/amd/-/issues/2719#note_2181402
> 
> Changelog:
> - v2:
>   - Emit dev_err(), but don't return error codes
>   - Add check for MCU power save mode being enabled and disable if 
> active
>   - General cleanup and rename some vars/funcs
> - v1: 
> https://lore.kernel.org/all/20231124082749.23353-1-luke@ljones.dev/
> 
> Luke D. Jones (1):
>   platform/x86: asus-wmi: disable USB0 hub on ROG Ally before suspend
> 
>  drivers/platform/x86/asus-wmi.c            | 50 
> ++++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |  3 ++
>  2 files changed, 53 insertions(+)
> 
> --
> 2.43.0
> 

Hans I omitted your review tag as I made more changes than requested.
Ilpo Järvinen Nov. 28, 2023, 1:21 p.m. UTC | #2 
On Mon, 27 Nov 2023 12:05:20 +1300, Luke D. Jones wrote:

> This is a fix for the ROG Ally not being able to use the N-Key device after a suspend/resume cycle.
> 
> The root of the issue is that ASUS changed the MCU firmware to dfisconnect the USB0 hub when the
> screen is switched off during the s2idle suspend path. I tried many many different tactics to try
> and get this s2idle part to work but it seems there are races between this and other subsystems.
> 
> What has so far been reliable and consistent is a manual call to the CSEE method that is called in
> ACPI by the Microsoft DSM screen off path followed by a short sleep in asus-wmi. The PM prepare
> hook looks to be the earliest possible place. A sleep that is too long ends up with USB subsystem
> registering a disconnect, and thus on resume the device paths change. Too short and it is unreliable.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo branch. Note it will show up in the public
platform-drivers-x86/review-ilpo branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/1] platform/x86: asus-wmi: disable USB0 hub on ROG Ally before suspend
      commit: e0894ff038d86f30614ec16ec26dacb88c8d2bd4

--
 i.