[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b7d3a60a-7298-41ef-8f4e-7dc56c1ccc8a@oracle.com>
Date: Tue, 29 Apr 2025 02:27:30 +0530
From: ALOK TIWARI <alok.a.tiwari@...cle.com>
To: "Derek J. Clark" <derekjohn.clark@...il.com>,
Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Armin Wolf <W_Armin@....de>, Jonathan Corbet <corbet@....net>,
Mario Limonciello <superm1@...nel.org>, Luke Jones <luke@...nes.dev>,
Xino Ni <nijs1@...ovo.com>, Zhixin Zhang <zhangzx36@...ovo.com>,
Mia Shao <shaohz1@...ovo.com>,
Mark Pearson <mpearson-lenovo@...ebb.ca>,
"Pierre-Loup A . Griffais" <pgriffais@...vesoftware.com>,
"Cody T . -H . Chiu" <codyit@...il.com>,
John Martens <johnfanv2@...il.com>,
platform-driver-x86@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org,
Mario Limonciello <mario.limonciello@....com>
Subject: Re: [External] : [PATCH v6 3/6] platform/x86: Add Lenovo WMI Events
Driver
On 28-04-2025 06:48, Derek J. Clark wrote:
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-wmi-events.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Lenovo WMI Events driver. Lenovo WMI interfaces provide various
> + * hardware triggered events that many drivers need to have propagated.
> + * This driver provides a uniform entrypoint for these events so that
> + * any driver that needs to respond to these events can subscribe to a
> + * notifier chain.
> + *
> + * Copyright(C) 2025 Derek J. Clark <derekjohn.clark@...il.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/types.h>
> +#include <linux/wmi.h>
> +
> +#include "lenovo-wmi-events.h"
> +#include "lenovo-wmi-gamezone.h"
> +
> +#define THERMAL_MODE_EVENT_GUID "D320289E-8FEA-41E0-86F9-911D83151B5F"
> +
> +#define LWMI_EVENT_DEVICE(guid, type) \
> + .guid_string = (guid), .context = &(enum lwmi_events_type) \
> + { \
> + type \
> + }
> +
> +static BLOCKING_NOTIFIER_HEAD(events_chain_head);
> +
> +struct lwmi_events_priv {
> + struct wmi_device *wdev;
> + enum lwmi_events_type type;
> +};
> +
> +/**
> + * lwmi_events_register_notifier() - Add a notifier to the notifier chain.
> + * @nb: The notifier_block struct to register
> + *
> + * Call blocking_notifier_chain_register to register the notifier block to the
> + * lenovo-wmi-events driver blocking notifer chain.
typo notifer -> notifier in all register/unregister
> + *
> + * Return: 0 on success, %-EEXIST on error.
> + */
> +int lwmi_events_register_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&events_chain_head, nb);
> +}
> +EXPORT_SYMBOL_NS_GPL(lwmi_events_register_notifier, "LENOVO_WMI_EVENTS");
> +
> +/**
> + * lwmi_events_unregister_notifier() - Remove a notifier from the notifier
> + * chain.
> + * @nb: The notifier_block struct to register
the @nb parameter is described inconsistently:
@nb: The notifier_block struct to unregister
> + *
> + * Call blocking_notifier_chain_unregister to unregister the notifier block
> + * from the lenovo-wmi-events driver blocking notifer chain.
> + *
> + * Return: 0 on success, %-ENOENT on error.
> + */
> +int lwmi_events_unregister_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&events_chain_head, nb);
> +}
> +EXPORT_SYMBOL_NS_GPL(lwmi_events_unregister_notifier, "LENOVO_WMI_EVENTS");
can be consider lwmi_events_un/register_notifier as static
if they are wrapper func?
> +
> +/**
> + * devm_lwmi_events_unregister_notifier() - Remove a notifier from the notifier
> + * chain.
> + * @data: Void pointer to the notifier_block struct to register.
* @data: Void pointer to the notifier_block struct to unregister.
> + *
> + * Call lwmi_events_unregister_notifier to unregister the notifier block from
> + * the lenovo-wmi-events driver blocking notifer chain.
> + *
> + * Return: 0 on success, %-ENOENT on error.
> + */
> +static void devm_lwmi_events_unregister_notifier(void *data)
> +{
> + struct notifier_block *nb = data;
> +
if (nb) Consider checking NULL pointer
> + lwmi_events_unregister_notifier(nb);
> +}
> +
> +/**
> + * devm_lwmi_events_register_notifier() - Add a notifier to the notifier chain.
> + * @dev: The parent device of the notifier_block struct.
> + * @nb: The notifier_block struct to register
> + *
> + * Call lwmi_events_register_notifier to register the notifier block to the
> + * lenovo-wmi-events driver blocking notifer chain. Then add, as a device
> + * managed action, unregister_notifier to automatically unregister the
> + * notifier block upon its parent device removal.
> + *
> + * Return: 0 on success, or an error code.
> + */
> +int devm_lwmi_events_register_notifier(struct device *dev,
> + struct notifier_block *nb)
> +{
> + int ret;
> +
> + ret = lwmi_events_register_notifier(nb);
> + if (ret < 0)
> + return ret;
> +
> + return devm_add_action_or_reset(dev, devm_lwmi_events_unregister_notifier, nb);
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_lwmi_events_register_notifier, "LENOVO_WMI_EVENTS");
> +
> +/**
> + * lwmi_events_notify() - Call functions for the notifier call chain.
> + * @wdev: The parent WMI device of the driver.
> + * @obj: ACPI object passed by the registered WMI Event.
> + *
> + * Validate WMI event data and notify all registered drivers of the event and
> + * its output.
> + *
> + * Return: 0 on success, or an error code.
> + */
> +static void lwmi_events_notify(struct wmi_device *wdev, union acpi_object *obj)
> +{
> + struct lwmi_events_priv *priv = dev_get_drvdata(&wdev->dev);
> + int sel_prof;
> + int ret;
> +
> + switch (priv->type) {
> + case LWMI_EVENT_THERMAL_MODE:
> + if (obj->type != ACPI_TYPE_INTEGER)
> + return;
> +
> + sel_prof = obj->integer.value;
> +
> + switch (sel_prof) {
> + case LWMI_GZ_THERMAL_MODE_QUIET:
> + case LWMI_GZ_THERMAL_MODE_BALANCED:
> + case LWMI_GZ_THERMAL_MODE_PERFORMANCE:
> + case LWMI_GZ_THERMAL_MODE_EXTREME:
> + case LWMI_GZ_THERMAL_MODE_CUSTOM:
> + ret = blocking_notifier_call_chain(&events_chain_head,
> + LWMI_EVENT_THERMAL_MODE,
> + &sel_prof);
> + if (ret == NOTIFY_BAD)
> + dev_err(&wdev->dev,
> + "Failed to send notification to call chain for WMI Events\n");
> + return;
> + default:
> + dev_err(&wdev->dev, "Got invalid thermal mode: %x",
> + sel_prof);
> + return;
> + }
> + break;
> + default:
> + return;
> + }
> +}
> +
> +static int lwmi_events_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct lwmi_events_priv *priv;
> +
> + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + if (!context)
> + return -EINVAL;
> +
might want to check context before using devm_kzalloc()
to avoid unnecessary memory allocation
> + priv->wdev = wdev;
> + priv->type = *(enum lwmi_events_type *)context;
> +
> + dev_set_drvdata(&wdev->dev, priv);
> + return 0;
> +}
> +
> +static const struct wmi_device_id lwmi_events_id_table[] = {
> + { LWMI_EVENT_DEVICE(THERMAL_MODE_EVENT_GUID, LWMI_EVENT_THERMAL_MODE) },
> + {}
> +};
> +
> +static struct wmi_driver lwmi_events_driver = {
> + .driver = {
> + .name = "lenovo_wmi_events",
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .id_table = lwmi_events_id_table,
> + .probe = lwmi_events_probe,
> + .notify = lwmi_events_notify,
> + .no_singleton = true,
> +};
> +
> +module_wmi_driver(lwmi_events_driver);
> +
> +MODULE_DEVICE_TABLE(wmi, lwmi_events_id_table);
> +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@...il.com>");
> +MODULE_DESCRIPTION("Lenovo WMI Events Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/lenovo-wmi-events.h b/drivers/platform/x86/lenovo-wmi-events.h
> new file mode 100644
> index 000000000000..e4c5459c2f24
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-wmi-events.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +/* Copyright(C) 2025 Derek J. Clark <derekjohn.clark@...il.com> */
> +
> +#ifndef _LENOVO_WMI_EVENTS_H_
> +#define _LENOVO_WMI_EVENTS_H_
> +
> +struct device;
> +struct notifier_block;
> +
> +enum lwmi_events_type {
> + LWMI_EVENT_THERMAL_MODE = 1,
> +};
> +
> +int lwmi_events_register_notifier(struct notifier_block *nb);
> +int lwmi_events_unregister_notifier(struct notifier_block *nb);
> +int devm_lwmi_events_register_notifier(struct device *dev,
> + struct notifier_block *nb);
why devm_lwmi_events_unregister_notifier not here ?
> +
> +#endif /* !_LENOVO_WMI_EVENTS_H_ */
Thanks,
Alok
Powered by blists - more mailing lists