mbox series

[v2,0/5] platform/x86: wmi: Pass event data directly to legacy notify handlers

Message ID 20240901031055.3030-1-W_Armin@gmx.de
Headers show
Series platform/x86: wmi: Pass event data directly to legacy notify handlers | expand

Message

Armin Wolf Sept. 1, 2024, 3:10 a.m. UTC
The current legacy WMI handlers are susceptible to picking up wrong
WMI event data on systems where different WMI devices share some
notification IDs.

Prevent this by letting the WMI driver core taking care of retrieving
the event data. This also simplifies the legacy WMI handlers and their
implementation inside the WMI driver core.

The fisr patch fixes a minor issue discovered inside the hp-wmi-sensors
driver.
The second patch converts all legacy WMI notify handlers to stop using
wmi_get_event_data() and instead use the new event data provided by
the WMI driver core.
The remaining patches finally perform some cleanups.

The patch series was tested on a Dell Inspiron 3505 and a Acer Aspire
E1-731 and appears to work.

Changes since v1:
- Rework the hp-wmi-sensors patch
- add Reviewed-by tags

Armin Wolf (5):
  hwmon: (hp-wmi-sensors) Check if WMI event data exists
  platform/x86: wmi: Pass event data directly to legacy notify handlers
  platform/x86: wmi: Remove wmi_get_event_data()
  platform/x86: wmi: Merge get_event_data() with wmi_get_notify_data()
  platform/x86: wmi: Call both legacy and WMI driver notify handlers

 drivers/hwmon/hp-wmi-sensors.c           |  20 +---
 drivers/platform/x86/acer-wmi.c          |  16 +--
 drivers/platform/x86/asus-wmi.c          |  19 +--
 drivers/platform/x86/dell/dell-wmi-aio.c |  13 +--
 drivers/platform/x86/hp/hp-wmi.c         |  16 +--
 drivers/platform/x86/huawei-wmi.c        |  14 +--
 drivers/platform/x86/lg-laptop.c         |  13 +--
 drivers/platform/x86/msi-wmi.c           |  20 +---
 drivers/platform/x86/toshiba-wmi.c       |  15 +--
 drivers/platform/x86/wmi.c               | 143 ++++++-----------------
 include/linux/acpi.h                     |   3 +-
 11 files changed, 53 insertions(+), 239 deletions(-)

--
2.39.2

Comments

Ilpo Järvinen Sept. 2, 2024, 2:09 p.m. UTC | #1
On Sun, 1 Sep 2024, Armin Wolf wrote:

> The BIOS can choose to return no event data in response to a
> WMI event, so the ACPI object passed to the WMI notify handler
> can be NULL.
> 
> Check for such a situation and ignore the event in such a case.
> 
> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Hans de Goede Sept. 4, 2024, 5:55 p.m. UTC | #2
Hi All,

On 9/2/24 4:28 PM, Guenter Roeck wrote:
> On Sun, Sep 01, 2024 at 05:10:51AM +0200, Armin Wolf wrote:
>> The BIOS can choose to return no event data in response to a
>> WMI event, so the ACPI object passed to the WMI notify handler
>> can be NULL.
>>
>> Check for such a situation and ignore the event in such a case.
>>
>> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> Applied.

Thank you.

Unfortunately patch 2/5 touches the same part of the file,
so I cannot apply the rest of the series without first
bringing this patch into platform-drivers-x86/for-next .

Guenter, can you provide an immutable branch/tag with
this patch on it; or drop this patch that I merge
the entire series through platform-drivers-x86/for-next ?

Regards,

Hans
Guenter Roeck Sept. 4, 2024, 6:38 p.m. UTC | #3
On 9/4/24 10:55, Hans de Goede wrote:
> Hi All,
> 
> On 9/2/24 4:28 PM, Guenter Roeck wrote:
>> On Sun, Sep 01, 2024 at 05:10:51AM +0200, Armin Wolf wrote:
>>> The BIOS can choose to return no event data in response to a
>>> WMI event, so the ACPI object passed to the WMI notify handler
>>> can be NULL.
>>>
>>> Check for such a situation and ignore the event in such a case.
>>>
>>> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>
>> Applied.
> 
> Thank you.
> 
> Unfortunately patch 2/5 touches the same part of the file,
> so I cannot apply the rest of the series without first
> bringing this patch into platform-drivers-x86/for-next .
> 
> Guenter, can you provide an immutable branch/tag with
> this patch on it; or drop this patch that I merge
> the entire series through platform-drivers-x86/for-next ?
> 

Can you wait a couple of days ? Since this is a bug fix, I had
planned to send a pull request either later today or; with that,
the patch would be upstream.

Guenter
Hans de Goede Sept. 4, 2024, 6:42 p.m. UTC | #4
Hi Guenter,

On 9/4/24 8:38 PM, Guenter Roeck wrote:
> On 9/4/24 10:55, Hans de Goede wrote:
>> Hi All,
>>
>> On 9/2/24 4:28 PM, Guenter Roeck wrote:
>>> On Sun, Sep 01, 2024 at 05:10:51AM +0200, Armin Wolf wrote:
>>>> The BIOS can choose to return no event data in response to a
>>>> WMI event, so the ACPI object passed to the WMI notify handler
>>>> can be NULL.
>>>>
>>>> Check for such a situation and ignore the event in such a case.
>>>>
>>>> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>>
>>> Applied.
>>
>> Thank you.
>>
>> Unfortunately patch 2/5 touches the same part of the file,
>> so I cannot apply the rest of the series without first
>> bringing this patch into platform-drivers-x86/for-next .
>>
>> Guenter, can you provide an immutable branch/tag with
>> this patch on it; or drop this patch that I merge
>> the entire series through platform-drivers-x86/for-next ?
>>
> 
> Can you wait a couple of days ? Since this is a bug fix, I had
> planned to send a pull request either later today or; with that,
> the patch would be upstream.

Yes I can wait a couple of days, thank you.

Regards,

Hans
Guenter Roeck Sept. 4, 2024, 7:40 p.m. UTC | #5
On 9/4/24 11:42, Hans de Goede wrote:
> Hi Guenter,
> 
> On 9/4/24 8:38 PM, Guenter Roeck wrote:
>> On 9/4/24 10:55, Hans de Goede wrote:
>>> Hi All,
>>>
>>> On 9/2/24 4:28 PM, Guenter Roeck wrote:
>>>> On Sun, Sep 01, 2024 at 05:10:51AM +0200, Armin Wolf wrote:
>>>>> The BIOS can choose to return no event data in response to a
>>>>> WMI event, so the ACPI object passed to the WMI notify handler
>>>>> can be NULL.
>>>>>
>>>>> Check for such a situation and ignore the event in such a case.
>>>>>
>>>>> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
>>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>>>
>>>> Applied.
>>>
>>> Thank you.
>>>
>>> Unfortunately patch 2/5 touches the same part of the file,
>>> so I cannot apply the rest of the series without first
>>> bringing this patch into platform-drivers-x86/for-next .
>>>
>>> Guenter, can you provide an immutable branch/tag with
>>> this patch on it; or drop this patch that I merge
>>> the entire series through platform-drivers-x86/for-next ?
>>>
>>
>> Can you wait a couple of days ? Since this is a bug fix, I had
>> planned to send a pull request either later today or; with that,
>> the patch would be upstream.
> 
> Yes I can wait a couple of days, thank you.
> 

I sent a pull request, and the patch is already upstream.

Guenter
Hans de Goede Sept. 5, 2024, 3:31 p.m. UTC | #6
Hi,

On 9/1/24 5:10 AM, Armin Wolf wrote:
> The current legacy WMI handlers are susceptible to picking up wrong
> WMI event data on systems where different WMI devices share some
> notification IDs.
> 
> Prevent this by letting the WMI driver core taking care of retrieving
> the event data. This also simplifies the legacy WMI handlers and their
> implementation inside the WMI driver core.
> 
> The fisr patch fixes a minor issue discovered inside the hp-wmi-sensors
> driver.
> The second patch converts all legacy WMI notify handlers to stop using
> wmi_get_event_data() and instead use the new event data provided by
> the WMI driver core.
> The remaining patches finally perform some cleanups.
> 
> The patch series was tested on a Dell Inspiron 3505 and a Acer Aspire
> E1-731 and appears to work.
> 
> Changes since v1:
> - Rework the hp-wmi-sensors patch
> - add Reviewed-by tags
> 
> Armin Wolf (5):
>   hwmon: (hp-wmi-sensors) Check if WMI event data exists
>   platform/x86: wmi: Pass event data directly to legacy notify handlers
>   platform/x86: wmi: Remove wmi_get_event_data()
>   platform/x86: wmi: Merge get_event_data() with wmi_get_notify_data()
>   platform/x86: wmi: Call both legacy and WMI driver notify handlers

Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans




> 
>  drivers/hwmon/hp-wmi-sensors.c           |  20 +---
>  drivers/platform/x86/acer-wmi.c          |  16 +--
>  drivers/platform/x86/asus-wmi.c          |  19 +--
>  drivers/platform/x86/dell/dell-wmi-aio.c |  13 +--
>  drivers/platform/x86/hp/hp-wmi.c         |  16 +--
>  drivers/platform/x86/huawei-wmi.c        |  14 +--
>  drivers/platform/x86/lg-laptop.c         |  13 +--
>  drivers/platform/x86/msi-wmi.c           |  20 +---
>  drivers/platform/x86/toshiba-wmi.c       |  15 +--
>  drivers/platform/x86/wmi.c               | 143 ++++++-----------------
>  include/linux/acpi.h                     |   3 +-
>  11 files changed, 53 insertions(+), 239 deletions(-)
> 
> --
> 2.39.2
>