Re: [PATCH] platform/x86: wmi: change notification handler type

[Mikalai Ramanovich <nikolay.romanovich.00@gmail.com>]

Hi, 
thank you for your reply.

On Tue, Oct 19, 2021 at 05:11:51PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/15/21 21:13, Mikalai Ramanovich wrote:
> > Since AML code on some Xiaomi laptops notifies the WMI hotkey with
> > 0x20 event, we need ACPI_ALL_NOTIFY here to be able to handle it.
> > 
> > Signed-off-by: Mikalai Ramanovich <nikolay.romanovich.00@gmail.com>
> 
> Hmm, this is a rather unusual change and I'm worried that it may have
> some bad side-effects.

I think it can't lead to bad side effects: this driver ignores events 
which are not described in the _WDG section (doesn't have GUID assiciated).

But if it's described it should be handled by this driver even if it
is less than 0x80. But this driver handles only 0x80-0xFF events.

> Can you provide the model-number and an acpidump for the laptop where
> you need this ? And maybe also point out which bit (which lines after
> disassembling) of the DSDT needs this ?

It's Xiaomi Mi Notebook Pro 14 2021. (TIMI A34 by DMI).

Here is a dump of interesting files: 
https://gist.github.com/MikalaiR/eee783cc0b1efdbe2aab158653e84935
(sorry for the link, i don't know it's good to attach files here or not).

The most interesting part is ssdt8.dsl file which contains only one
WMI device. Method \_SB.PC00.LPCB.EC0.XWEV (in ssdt10.dsl, line 2495) 
generates events for this device.

And this is a part of decompiled BMOF from this device:

[WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x40A"), 
 Description("Root/WMI/HID_EVENT20"), 
 guid("{46c93e13-ee9b-4262-8488-563bca757fef}")]
class HID_EVENT20 : WmiEvent {
  [key, read] string InstanceName;
  [read] boolean Active;
  [WmiDataId(1), read, write, Description("Package Data")] uint8 EventDetail[8];
};

ACPI event 0x20 associated with GUID 46c93e13-ee9b-4262-8488-563bca757fef.

> ATM I'm thinking that it might be best to do something like this:
> 
> static u32 acpi_wmi_get_handler_type(void)
> {
> 	if (dmi_name_in_vendors("XIAOMI"))
> 		return ACPI_ALL_NOTIFY;
> 	else
> 		return ACPI_DEVICE_NOTIFY;
> }
> 
> 	status = acpi_install_notify_handler(acpi_device->handle,
> 					     acpi_wmi_get_handler_type(),
> 					     acpi_wmi_notify_handler,
> 					     NULL);
> 
> (and the same for the remove)
> 
> So that we limit this behavior to the Xiaomi case.

In general i don't think it's a good idea, but if it's the only 
acceptable solution, why not.


Regards, 

Mikalai