Re: [PATCH 0/3] platfom/x86: asus-wmi: revert ROG Ally quirks

From: Hans de Goede
Date: Sat Oct 05 2024 - 10:37:24 EST


Hi Luke,

On 26-Sep-24 11:53 AM, Luke D. Jones wrote:
> The ASUS ROG Ally (and Ally X) quirks that I added over the last year
> are not required. I worked with ASUS to pinpoint the exact cause of
> the original issue (MCU USB dev missing every second resume) and the
> result is a new MCU firmware which will be released on approx 16/10/24.

First of all let me say that it is great that you have gotten Asus
to come up with a fixed firmware, thank you.

With that said I believe that it is way too early to revert these quirks,
users are usually not great at installing BIOS updates and that assumes
this will be handled as part of a BIOS update, if it requires running
a separate tool then the chances of users not installing the update
will likely be even worse.

So IMHO for now we should keep these quirks around to avoid regressions
for users who don't have the MCU update.

Related, have you seen this series:

https://lore.kernel.org/platform-driver-x86/20240922172258.48435-1-lkml@xxxxxxxxxxx/

that seems to fix the same issue ?

And it does so in another, arguably better way.

Although unfortunately as patch 3/5 shows just calling the global
"display off" callback before suspending devices is not enough
fixing things still requires inserting a sleep using a DMI quirk :|

Still that series including the DMI quirk might be a cleaner way
to deal with this and if that is merged then dropping the quirks
from asus-wmi makes sense.

Regards,

Hans




> All users should update to MCU FW as soon as released to:
> - Ally 1: v319
> - Ally X: v313
>
> Luke D. Jones (3):
> Revert "platform/x86: asus-wmi: ROG Ally increase wait time, allow MCU
> powersave"
> Revert "platform/x86: asus-wmi: disable USB0 hub on ROG Ally before
> suspend"
> platfom/x86: asus-wmi: cleanup after Ally quirk reverts
>
> drivers/platform/x86/asus-wmi.c | 39 +--------------------------------
> 1 file changed, 1 insertion(+), 38 deletions(-)
>