| Message ID | 20250225220037.16073-4-derekjohn.clark@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86: Add Lenovo Gaming Series WMI Drivers | expand |
On February 25, 2025 1:59:54 PM PST, "Derek J. Clark" <derekjohn.clark@gmail.com> wrote: >Adds lenovo-wmi-capdata01.c which provides a driver for the >LENOVO_CAPABILITY_DATA_01 WMI data block that comes on "Other Mode" >enabled hardware. Provides an interface for querying if a given >attribute is supported by the hardware, as well as its default_value, >max_value, min_value, and step increment. >v3: >- Add as component to lenovo-wmi-other driver. >v2: >- Use devm_kmalloc to ensure driver can be instanced, remove global > reference. >- Ensure reverse Christmas tree for all variable declarations. >- Remove extra whitespace. >- Use guard(mutex) in all mutex instances, global mutex. >- Use pr_fmt instead of adding the driver name to each pr_err. >- Remove noisy pr_info usage. >- Rename capdata_wmi to lenovo_wmi_cd01_priv and cd01_wmi to priv. >- Use list to get the lenovo_wmi_cd01_priv instance in > lenovo_wmi_capdata01_get as none of the data provided by the macros > that will use it can pass a member of the struct for use in > container_of. > >Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> >--- > MAINTAINERS | 1 + > drivers/platform/x86/Kconfig | 5 + > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/lenovo-wmi-capdata01.c | 140 ++++++++++++++++++++ > drivers/platform/x86/lenovo-wmi.h | 19 +++ > 5 files changed, 166 insertions(+) > create mode 100644 drivers/platform/x86/lenovo-wmi-capdata01.c > >diff --git a/MAINTAINERS b/MAINTAINERS >index cf7f4fce1a25..f6d3e79e50ce 100644 >--- a/MAINTAINERS >+++ b/MAINTAINERS >@@ -13157,6 +13157,7 @@ L: platform-driver-x86@vger.kernel.org > S: Maintained > F: Documentation/wmi/devices/lenovo-wmi-gamezone.rst > F: Documentation/wmi/devices/lenovo-wmi-other.rst >+F: drivers/platform/x86/lenovo-wmi-capdata01.c > F: drivers/platform/x86/lenovo-wmi-gamezone.c > F: drivers/platform/x86/lenovo-wmi.c > F: drivers/platform/x86/lenovo-wmi.h >diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >index 875822e6bd65..56336dc3c2d0 100644 >--- a/drivers/platform/x86/Kconfig >+++ b/drivers/platform/x86/Kconfig >@@ -475,6 +475,11 @@ config LENOVO_WMI_GAMEZONE > To compile this driver as a module, choose M here: the module will > be called lenovo-wmi-gamezone. > >+config LENOVO_WMI_DATA01 >+ tristate >+ depends on ACPI_WMI >+ select LENOVO_WMI >+ > config IDEAPAD_LAPTOP > tristate "Lenovo IdeaPad Laptop Extras" > depends on ACPI >diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >index 4a7b2d14eb82..be9031bea090 100644 >--- a/drivers/platform/x86/Makefile >+++ b/drivers/platform/x86/Makefile >@@ -70,6 +70,7 @@ obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > obj-$(CONFIG_LENOVO_WMI) += lenovo-wmi.o > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o >+obj-$(CONFIG_LENOVO_WMI_DATA01) += lenovo-wmi-capdata01.o > > # Intel > obj-y += intel/ >diff --git a/drivers/platform/x86/lenovo-wmi-capdata01.c b/drivers/platform/x86/lenovo-wmi-capdata01.c >new file mode 100644 >index 000000000000..0fe156d5d770 >--- /dev/null >+++ b/drivers/platform/x86/lenovo-wmi-capdata01.c >@@ -0,0 +1,140 @@ >+// SPDX-License-Identifier: GPL-2.0-or-later >+/* >+ * LENOVO_CAPABILITY_DATA_01 WMI data block driver. This interface provides >+ * information on tunable attributes used by the "Other Mode" WMI interface, >+ * including if it is supported by the hardware, the default_value, max_value, >+ * min_value, and step increment. >+ * >+ * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> >+ */ >+ >+#include <linux/cleanup.h> >+#include <linux/component.h> >+#include <linux/container_of.h> >+#include <linux/device.h> >+#include <linux/gfp_types.h> >+#include <linux/types.h> >+#include <linux/wmi.h> >+#include "lenovo-wmi.h" >+ >+/* Interface GUIDs */ >+#define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154" >+ >+static int lenovo_cd01_component_bind(struct device *cd01_dev, >+ struct device *om_dev, void *data) >+{ >+ struct lenovo_wmi_cd01 *cd01 = dev_get_drvdata(cd01_dev); >+ struct lenovo_wmi_om *om = dev_get_drvdata(om_dev); >+ >+ om->cd01 = cd01; >+ return 0; >+} >+ >+static void lenovo_cd01_component_unbind(struct device *cd01_dev, >+ struct device *om_dev, void *data) >+ >+{ >+ struct wmi_device *om_wdev = >+ container_of(om_dev, struct wmi_device, dev); >+ struct lenovo_wmi_om *om = >+ container_of(&om_wdev, struct lenovo_wmi_om, wdev); >+ >+ om->cd01 = NULL; >+} >+ >+static const struct component_ops lenovo_cd01_component_ops = { >+ .bind = lenovo_cd01_component_bind, >+ .unbind = lenovo_cd01_component_unbind, >+}; >+ >+static int lenovo_wmi_cd01_setup(struct lenovo_wmi_cd01 *cd01) >+{ >+ size_t cd_size = sizeof(struct capdata01); >+ int count, idx; >+ >+ count = wmidev_instance_count(cd01->wdev); >+ >+ cd01->capdata = devm_kmalloc_array(&cd01->wdev->dev, (size_t)count, >+ sizeof(*cd01->capdata), GFP_KERNEL); >+ if (!cd01->capdata) >+ return -ENOMEM; >+ >+ cd01->instance_count = count; >+ >+ for (idx = 0; idx < count; idx++) { >+ union acpi_object *ret_obj __free(kfree) = NULL; >+ struct capdata01 *cap_ptr = >+ devm_kmalloc(&cd01->wdev->dev, cd_size, GFP_KERNEL); >+ ret_obj = wmidev_block_query(cd01->wdev, idx); >+ if (!ret_obj) >+ continue; >+ >+ if (ret_obj->type != ACPI_TYPE_BUFFER) >+ continue; >+ >+ if (ret_obj->buffer.length != cd_size) >+ continue; >+ >+ memcpy(cap_ptr, ret_obj->buffer.pointer, >+ ret_obj->buffer.length); >+ cd01->capdata[idx] = cap_ptr; >+ } >+ return 0; >+} >+ >+static int lenovo_wmi_cd01_probe(struct wmi_device *wdev, const void *context) >+ >+{ >+ struct lenovo_wmi_cd01 *cd01; >+ int ret; >+ >+ cd01 = devm_kzalloc(&wdev->dev, sizeof(*cd01), GFP_KERNEL); >+ if (!cd01) >+ return -ENOMEM; >+ >+ cd01->wdev = wdev; >+ >+ ret = lenovo_wmi_cd01_setup(cd01); >+ if (ret) >+ return ret; >+ >+ dev_set_drvdata(&wdev->dev, cd01); >+ >+ ret = component_add(&wdev->dev, &lenovo_cd01_component_ops); >+ >+ return ret; >+} >+ >+static void lenovo_wmi_cd01_remove(struct wmi_device *wdev) >+{ >+ component_del(&wdev->dev, &lenovo_cd01_component_ops); >+} >+ >+static const struct wmi_device_id lenovo_wmi_cd01_id_table[] = { >+ { LENOVO_CAPABILITY_DATA_01_GUID, NULL }, >+ {} >+}; >+ >+static struct wmi_driver lenovo_wmi_cd01_driver = { >+ .driver = { >+ .name = "lenovo_wmi_cd01", >+ .probe_type = PROBE_PREFER_ASYNCHRONOUS, >+ }, >+ .id_table = lenovo_wmi_cd01_id_table, >+ .probe = lenovo_wmi_cd01_probe, >+ .remove = lenovo_wmi_cd01_remove, >+ .no_singleton = true, >+}; >+ >+int lenovo_wmi_cd01_match(struct device *dev, void *data) >+{ >+ return dev->driver == &lenovo_wmi_cd01_driver.driver; >+} >+EXPORT_SYMBOL_GPL(lenovo_wmi_cd01_match); >+ This should have been an EXPORT_SYMBOL_NS_GPL in the CAPDATA_WMI namespace. I thought I changed it but must have dropped it by mistake. I will fix this next time. - Derek >+module_wmi_driver(lenovo_wmi_cd01_driver); >+ >+MODULE_DEVICE_TABLE(wmi, lenovo_wmi_cd01_id_table); >+MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); >+MODULE_DESCRIPTION("Lenovo Capability Data 01 WMI Driver"); >+MODULE_LICENSE("GPL"); >diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h >index 113928b4fc0f..07fa67ed89d6 100644 >--- a/drivers/platform/x86/lenovo-wmi.h >+++ b/drivers/platform/x86/lenovo-wmi.h >@@ -45,6 +45,22 @@ enum lenovo_wmi_action { > THERMAL_MODE_EVENT = 1, > }; > >+/* capdata01 structs */ >+struct lenovo_wmi_cd01 { >+ struct capdata01 **capdata; >+ struct wmi_device *wdev; >+ int instance_count; >+}; >+ >+struct capdata01 { >+ u32 id; >+ u32 supported; >+ u32 default_value; >+ u32 step; >+ u32 min_value; >+ u32 max_value; >+}; >+ > /* wmidev_evaluate_method helper functions */ > int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > u32 method_id, u32 arg0, u32 arg1, >@@ -52,6 +68,9 @@ int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > u32 method_id, u32 arg0, u32 *retval); > >+/* lenovo_wmi_cd01_driver match function */ >+int lenovo_wmi_cd01_match(struct device *dev, void *data); >+ > /* lenovo_wmi_gz_driver notifier functions */ > int lenovo_wmi_gz_notifier_call(struct notifier_block *nb, unsigned long action, > enum platform_profile_option *profile); - Derek
Am 25.02.25 um 22:59 schrieb Derek J. Clark: > Adds lenovo-wmi-capdata01.c which provides a driver for the > LENOVO_CAPABILITY_DATA_01 WMI data block that comes on "Other Mode" > enabled hardware. Provides an interface for querying if a given > attribute is supported by the hardware, as well as its default_value, > max_value, min_value, and step increment. > v3: > - Add as component to lenovo-wmi-other driver. > v2: > - Use devm_kmalloc to ensure driver can be instanced, remove global > reference. > - Ensure reverse Christmas tree for all variable declarations. > - Remove extra whitespace. > - Use guard(mutex) in all mutex instances, global mutex. > - Use pr_fmt instead of adding the driver name to each pr_err. > - Remove noisy pr_info usage. > - Rename capdata_wmi to lenovo_wmi_cd01_priv and cd01_wmi to priv. > - Use list to get the lenovo_wmi_cd01_priv instance in > lenovo_wmi_capdata01_get as none of the data provided by the macros > that will use it can pass a member of the struct for use in > container_of. > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > --- > MAINTAINERS | 1 + > drivers/platform/x86/Kconfig | 5 + > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/lenovo-wmi-capdata01.c | 140 ++++++++++++++++++++ > drivers/platform/x86/lenovo-wmi.h | 19 +++ > 5 files changed, 166 insertions(+) > create mode 100644 drivers/platform/x86/lenovo-wmi-capdata01.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index cf7f4fce1a25..f6d3e79e50ce 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13157,6 +13157,7 @@ L: platform-driver-x86@vger.kernel.org > S: Maintained > F: Documentation/wmi/devices/lenovo-wmi-gamezone.rst > F: Documentation/wmi/devices/lenovo-wmi-other.rst > +F: drivers/platform/x86/lenovo-wmi-capdata01.c > F: drivers/platform/x86/lenovo-wmi-gamezone.c > F: drivers/platform/x86/lenovo-wmi.c > F: drivers/platform/x86/lenovo-wmi.h > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 875822e6bd65..56336dc3c2d0 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -475,6 +475,11 @@ config LENOVO_WMI_GAMEZONE > To compile this driver as a module, choose M here: the module will > be called lenovo-wmi-gamezone. > > +config LENOVO_WMI_DATA01 > + tristate > + depends on ACPI_WMI > + select LENOVO_WMI > + > config IDEAPAD_LAPTOP > tristate "Lenovo IdeaPad Laptop Extras" > depends on ACPI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 4a7b2d14eb82..be9031bea090 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -70,6 +70,7 @@ obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > obj-$(CONFIG_LENOVO_WMI) += lenovo-wmi.o > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > +obj-$(CONFIG_LENOVO_WMI_DATA01) += lenovo-wmi-capdata01.o > > # Intel > obj-y += intel/ > diff --git a/drivers/platform/x86/lenovo-wmi-capdata01.c b/drivers/platform/x86/lenovo-wmi-capdata01.c > new file mode 100644 > index 000000000000..0fe156d5d770 > --- /dev/null > +++ b/drivers/platform/x86/lenovo-wmi-capdata01.c > @@ -0,0 +1,140 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * LENOVO_CAPABILITY_DATA_01 WMI data block driver. This interface provides > + * information on tunable attributes used by the "Other Mode" WMI interface, > + * including if it is supported by the hardware, the default_value, max_value, > + * min_value, and step increment. > + * > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> 2025 > + */ > + > +#include <linux/cleanup.h> > +#include <linux/component.h> > +#include <linux/container_of.h> > +#include <linux/device.h> > +#include <linux/gfp_types.h> > +#include <linux/types.h> > +#include <linux/wmi.h> > +#include "lenovo-wmi.h" > + > +/* Interface GUIDs */ > +#define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154" > + > +static int lenovo_cd01_component_bind(struct device *cd01_dev, > + struct device *om_dev, void *data) > +{ > + struct lenovo_wmi_cd01 *cd01 = dev_get_drvdata(cd01_dev); > + struct lenovo_wmi_om *om = dev_get_drvdata(om_dev); Why not using the *data pointer to pass the cd01 data? This way the capdata driver does not need to know the structure of the private data of the lenovo-wmi-other driver. > + > + om->cd01 = cd01; > + return 0; > +} > + > +static void lenovo_cd01_component_unbind(struct device *cd01_dev, > + struct device *om_dev, void *data) > + > +{ > + struct wmi_device *om_wdev = > + container_of(om_dev, struct wmi_device, dev); > + struct lenovo_wmi_om *om = > + container_of(&om_wdev, struct lenovo_wmi_om, wdev); > + > + om->cd01 = NULL; I think this is unnecessary, please remove the unbind callback. > +} > + > +static const struct component_ops lenovo_cd01_component_ops = { > + .bind = lenovo_cd01_component_bind, > + .unbind = lenovo_cd01_component_unbind, > +}; > + > +static int lenovo_wmi_cd01_setup(struct lenovo_wmi_cd01 *cd01) > +{ > + size_t cd_size = sizeof(struct capdata01); > + int count, idx; > + > + count = wmidev_instance_count(cd01->wdev); > + > + cd01->capdata = devm_kmalloc_array(&cd01->wdev->dev, (size_t)count, > + sizeof(*cd01->capdata), GFP_KERNEL); Cast to size_t is unnecessary here. > + if (!cd01->capdata) > + return -ENOMEM; > + > + cd01->instance_count = count; > + > + for (idx = 0; idx < count; idx++) { > + union acpi_object *ret_obj __free(kfree) = NULL; I am not sure if the compiler frees ret_obj after each loop iteration. Did you test this? > + struct capdata01 *cap_ptr = > + devm_kmalloc(&cd01->wdev->dev, cd_size, GFP_KERNEL); Please call devm_kmalloc() on a separate line. > + ret_obj = wmidev_block_query(cd01->wdev, idx); > + if (!ret_obj) > + continue; > + > + if (ret_obj->type != ACPI_TYPE_BUFFER) > + continue; > + > + if (ret_obj->buffer.length != cd_size) > + continue; > + > + memcpy(cap_ptr, ret_obj->buffer.pointer, > + ret_obj->buffer.length); Using devm_kmemdup() would make sense here. > + cd01->capdata[idx] = cap_ptr; > + } > + return 0; > +} > + > +static int lenovo_wmi_cd01_probe(struct wmi_device *wdev, const void *context) > + > +{ > + struct lenovo_wmi_cd01 *cd01; > + int ret; > + > + cd01 = devm_kzalloc(&wdev->dev, sizeof(*cd01), GFP_KERNEL); > + if (!cd01) > + return -ENOMEM; > + > + cd01->wdev = wdev; > + > + ret = lenovo_wmi_cd01_setup(cd01); > + if (ret) > + return ret; > + > + dev_set_drvdata(&wdev->dev, cd01); > + > + ret = component_add(&wdev->dev, &lenovo_cd01_component_ops); > + > + return ret; > +} > + > +static void lenovo_wmi_cd01_remove(struct wmi_device *wdev) > +{ > + component_del(&wdev->dev, &lenovo_cd01_component_ops); > +} > + > +static const struct wmi_device_id lenovo_wmi_cd01_id_table[] = { > + { LENOVO_CAPABILITY_DATA_01_GUID, NULL }, > + {} > +}; > + > +static struct wmi_driver lenovo_wmi_cd01_driver = { > + .driver = { > + .name = "lenovo_wmi_cd01", > + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > + }, > + .id_table = lenovo_wmi_cd01_id_table, > + .probe = lenovo_wmi_cd01_probe, > + .remove = lenovo_wmi_cd01_remove, > + .no_singleton = true, > +}; > + > +int lenovo_wmi_cd01_match(struct device *dev, void *data) > +{ > + return dev->driver == &lenovo_wmi_cd01_driver.driver; > +} > +EXPORT_SYMBOL_GPL(lenovo_wmi_cd01_match); Please put this symbol into a namespace too. > + > +module_wmi_driver(lenovo_wmi_cd01_driver); > + > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_cd01_id_table); > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > +MODULE_DESCRIPTION("Lenovo Capability Data 01 WMI Driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > index 113928b4fc0f..07fa67ed89d6 100644 > --- a/drivers/platform/x86/lenovo-wmi.h > +++ b/drivers/platform/x86/lenovo-wmi.h > @@ -45,6 +45,22 @@ enum lenovo_wmi_action { > THERMAL_MODE_EVENT = 1, > }; > > +/* capdata01 structs */ > +struct lenovo_wmi_cd01 { > + struct capdata01 **capdata; > + struct wmi_device *wdev; > + int instance_count; > +}; > + > +struct capdata01 { > + u32 id; > + u32 supported; > + u32 default_value; > + u32 step; > + u32 min_value; > + u32 max_value; > +}; Please put those struct definitions into a separate header file. Thanks, Armin Wolf > + > /* wmidev_evaluate_method helper functions */ > int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > u32 method_id, u32 arg0, u32 arg1, > @@ -52,6 +68,9 @@ int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > u32 method_id, u32 arg0, u32 *retval); > > +/* lenovo_wmi_cd01_driver match function */ > +int lenovo_wmi_cd01_match(struct device *dev, void *data); > + > /* lenovo_wmi_gz_driver notifier functions */ > int lenovo_wmi_gz_notifier_call(struct notifier_block *nb, unsigned long action, > enum platform_profile_option *profile);
On Fri, Mar 7, 2025 at 3:05 PM Armin Wolf <W_Armin@gmx.de> wrote: > > Am 25.02.25 um 22:59 schrieb Derek J. Clark: > > > Adds lenovo-wmi-capdata01.c which provides a driver for the > > LENOVO_CAPABILITY_DATA_01 WMI data block that comes on "Other Mode" > > enabled hardware. Provides an interface for querying if a given > > attribute is supported by the hardware, as well as its default_value, > > max_value, min_value, and step increment. > > v3: > > - Add as component to lenovo-wmi-other driver. > > v2: > > - Use devm_kmalloc to ensure driver can be instanced, remove global > > reference. > > - Ensure reverse Christmas tree for all variable declarations. > > - Remove extra whitespace. > > - Use guard(mutex) in all mutex instances, global mutex. > > - Use pr_fmt instead of adding the driver name to each pr_err. > > - Remove noisy pr_info usage. > > - Rename capdata_wmi to lenovo_wmi_cd01_priv and cd01_wmi to priv. > > - Use list to get the lenovo_wmi_cd01_priv instance in > > lenovo_wmi_capdata01_get as none of the data provided by the macros > > that will use it can pass a member of the struct for use in > > container_of. > > > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > > --- > > MAINTAINERS | 1 + > > drivers/platform/x86/Kconfig | 5 + > > drivers/platform/x86/Makefile | 1 + > > drivers/platform/x86/lenovo-wmi-capdata01.c | 140 ++++++++++++++++++++ > > drivers/platform/x86/lenovo-wmi.h | 19 +++ > > 5 files changed, 166 insertions(+) > > create mode 100644 drivers/platform/x86/lenovo-wmi-capdata01.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index cf7f4fce1a25..f6d3e79e50ce 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -13157,6 +13157,7 @@ L: platform-driver-x86@vger.kernel.org > > S: Maintained > > F: Documentation/wmi/devices/lenovo-wmi-gamezone.rst > > F: Documentation/wmi/devices/lenovo-wmi-other.rst > > +F: drivers/platform/x86/lenovo-wmi-capdata01.c > > F: drivers/platform/x86/lenovo-wmi-gamezone.c > > F: drivers/platform/x86/lenovo-wmi.c > > F: drivers/platform/x86/lenovo-wmi.h > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index 875822e6bd65..56336dc3c2d0 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -475,6 +475,11 @@ config LENOVO_WMI_GAMEZONE > > To compile this driver as a module, choose M here: the module will > > be called lenovo-wmi-gamezone. > > > > +config LENOVO_WMI_DATA01 > > + tristate > > + depends on ACPI_WMI > > + select LENOVO_WMI > > + > > config IDEAPAD_LAPTOP > > tristate "Lenovo IdeaPad Laptop Extras" > > depends on ACPI > > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > > index 4a7b2d14eb82..be9031bea090 100644 > > --- a/drivers/platform/x86/Makefile > > +++ b/drivers/platform/x86/Makefile > > @@ -70,6 +70,7 @@ obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > > obj-$(CONFIG_LENOVO_WMI) += lenovo-wmi.o > > obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > > obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > > +obj-$(CONFIG_LENOVO_WMI_DATA01) += lenovo-wmi-capdata01.o > > > > # Intel > > obj-y += intel/ > > diff --git a/drivers/platform/x86/lenovo-wmi-capdata01.c b/drivers/platform/x86/lenovo-wmi-capdata01.c > > new file mode 100644 > > index 000000000000..0fe156d5d770 > > --- /dev/null > > +++ b/drivers/platform/x86/lenovo-wmi-capdata01.c > > @@ -0,0 +1,140 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * LENOVO_CAPABILITY_DATA_01 WMI data block driver. This interface provides > > + * information on tunable attributes used by the "Other Mode" WMI interface, > > + * including if it is supported by the hardware, the default_value, max_value, > > + * min_value, and step increment. > > + * > > + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > > 2025 Acked for all as Mario suggested. > > + */ > > + > > +#include <linux/cleanup.h> > > +#include <linux/component.h> > > +#include <linux/container_of.h> > > +#include <linux/device.h> > > +#include <linux/gfp_types.h> > > +#include <linux/types.h> > > +#include <linux/wmi.h> > > +#include "lenovo-wmi.h" > > + > > +/* Interface GUIDs */ > > +#define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154" > > + > > +static int lenovo_cd01_component_bind(struct device *cd01_dev, > > + struct device *om_dev, void *data) > > +{ > > + struct lenovo_wmi_cd01 *cd01 = dev_get_drvdata(cd01_dev); > > + struct lenovo_wmi_om *om = dev_get_drvdata(om_dev); > > Why not using the *data pointer to pass the cd01 data? This way the capdata driver > does not need to know the structure of the private data of the lenovo-wmi-other driver. > I can do that, sure. Seems preferable TBH as it allows me to call it priv again. > > + > > + om->cd01 = cd01; > > + return 0; > > +} > > + > > +static void lenovo_cd01_component_unbind(struct device *cd01_dev, > > + struct device *om_dev, void *data) > > + > > +{ > > + struct wmi_device *om_wdev = > > + container_of(om_dev, struct wmi_device, dev); > > + struct lenovo_wmi_om *om = > > + container_of(&om_wdev, struct lenovo_wmi_om, wdev); > > + > > + om->cd01 = NULL; > > I think this is unnecessary, please remove the unbind callback. > Acked. > > +} > > + > > +static const struct component_ops lenovo_cd01_component_ops = { > > + .bind = lenovo_cd01_component_bind, > > + .unbind = lenovo_cd01_component_unbind, > > +}; > > + > > +static int lenovo_wmi_cd01_setup(struct lenovo_wmi_cd01 *cd01) > > +{ > > + size_t cd_size = sizeof(struct capdata01); > > + int count, idx; > > + > > + count = wmidev_instance_count(cd01->wdev); > > + > > + cd01->capdata = devm_kmalloc_array(&cd01->wdev->dev, (size_t)count, > > + sizeof(*cd01->capdata), GFP_KERNEL); > > Cast to size_t is unnecessary here. > Acked. > > + if (!cd01->capdata) > > + return -ENOMEM; > > + > > + cd01->instance_count = count; > > + > > + for (idx = 0; idx < count; idx++) { > > + union acpi_object *ret_obj __free(kfree) = NULL; > > I am not sure if the compiler frees ret_obj after each loop iteration. Did you test this? > No, but I'm not sure how I would. I was manually using kfree before but was asked to change to the free macro in an earlier rev. > > + struct capdata01 *cap_ptr = > > + devm_kmalloc(&cd01->wdev->dev, cd_size, GFP_KERNEL); > > Please call devm_kmalloc() on a separate line. > Acked. > > + ret_obj = wmidev_block_query(cd01->wdev, idx); > > + if (!ret_obj) > > + continue; > > + > > + if (ret_obj->type != ACPI_TYPE_BUFFER) > > + continue; > > + > > + if (ret_obj->buffer.length != cd_size) > > + continue; > > + > > + memcpy(cap_ptr, ret_obj->buffer.pointer, > > + ret_obj->buffer.length); > > Using devm_kmemdup() would make sense here. > That's a cool function. Ty, I'll use it > > + cd01->capdata[idx] = cap_ptr; > > + } > > + return 0; > > +} > > + > > +static int lenovo_wmi_cd01_probe(struct wmi_device *wdev, const void *context) > > + > > +{ > > + struct lenovo_wmi_cd01 *cd01; > > + int ret; > > + > > + cd01 = devm_kzalloc(&wdev->dev, sizeof(*cd01), GFP_KERNEL); > > + if (!cd01) > > + return -ENOMEM; > > + > > + cd01->wdev = wdev; > > + > > + ret = lenovo_wmi_cd01_setup(cd01); > > + if (ret) > > + return ret; > > + > > + dev_set_drvdata(&wdev->dev, cd01); > > + > > + ret = component_add(&wdev->dev, &lenovo_cd01_component_ops); > > + > > + return ret; > > +} > > + > > +static void lenovo_wmi_cd01_remove(struct wmi_device *wdev) > > +{ > > + component_del(&wdev->dev, &lenovo_cd01_component_ops); > > +} > > + > > +static const struct wmi_device_id lenovo_wmi_cd01_id_table[] = { > > + { LENOVO_CAPABILITY_DATA_01_GUID, NULL }, > > + {} > > +}; > > + > > +static struct wmi_driver lenovo_wmi_cd01_driver = { > > + .driver = { > > + .name = "lenovo_wmi_cd01", > > + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > > + }, > > + .id_table = lenovo_wmi_cd01_id_table, > > + .probe = lenovo_wmi_cd01_probe, > > + .remove = lenovo_wmi_cd01_remove, > > + .no_singleton = true, > > +}; > > + > > +int lenovo_wmi_cd01_match(struct device *dev, void *data) > > +{ > > + return dev->driver == &lenovo_wmi_cd01_driver.driver; > > +} > > +EXPORT_SYMBOL_GPL(lenovo_wmi_cd01_match); > > Please put this symbol into a namespace too. > Yes, I noticed the mistake right after I sent the patch. > > + > > +module_wmi_driver(lenovo_wmi_cd01_driver); > > + > > +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_cd01_id_table); > > +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > > +MODULE_DESCRIPTION("Lenovo Capability Data 01 WMI Driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > > index 113928b4fc0f..07fa67ed89d6 100644 > > --- a/drivers/platform/x86/lenovo-wmi.h > > +++ b/drivers/platform/x86/lenovo-wmi.h > > @@ -45,6 +45,22 @@ enum lenovo_wmi_action { > > THERMAL_MODE_EVENT = 1, > > }; > > > > +/* capdata01 structs */ > > +struct lenovo_wmi_cd01 { > > + struct capdata01 **capdata; > > + struct wmi_device *wdev; > > + int instance_count; > > +}; > > + > > +struct capdata01 { > > + u32 id; > > + u32 supported; > > + u32 default_value; > > + u32 step; > > + u32 min_value; > > + u32 max_value; > > +}; > > Please put those struct definitions into a separate header file. > Acked. > Thanks, > Armin Wolf > > > + > > /* wmidev_evaluate_method helper functions */ > > int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > u32 method_id, u32 arg0, u32 arg1, > > @@ -52,6 +68,9 @@ int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > > int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > > u32 method_id, u32 arg0, u32 *retval); > > > > +/* lenovo_wmi_cd01_driver match function */ > > +int lenovo_wmi_cd01_match(struct device *dev, void *data); > > + > > /* lenovo_wmi_gz_driver notifier functions */ > > int lenovo_wmi_gz_notifier_call(struct notifier_block *nb, unsigned long action, > > enum platform_profile_option *profile);
Am 10.03.25 um 23:26 schrieb Derek John Clark: > On Fri, Mar 7, 2025 at 3:05 PM Armin Wolf <W_Armin@gmx.de> wrote: >> Am 25.02.25 um 22:59 schrieb Derek J. Clark: >> >>> Adds lenovo-wmi-capdata01.c which provides a driver for the >>> LENOVO_CAPABILITY_DATA_01 WMI data block that comes on "Other Mode" >>> enabled hardware. Provides an interface for querying if a given >>> attribute is supported by the hardware, as well as its default_value, >>> max_value, min_value, and step increment. >>> v3: >>> - Add as component to lenovo-wmi-other driver. >>> v2: >>> - Use devm_kmalloc to ensure driver can be instanced, remove global >>> reference. >>> - Ensure reverse Christmas tree for all variable declarations. >>> - Remove extra whitespace. >>> - Use guard(mutex) in all mutex instances, global mutex. >>> - Use pr_fmt instead of adding the driver name to each pr_err. >>> - Remove noisy pr_info usage. >>> - Rename capdata_wmi to lenovo_wmi_cd01_priv and cd01_wmi to priv. >>> - Use list to get the lenovo_wmi_cd01_priv instance in >>> lenovo_wmi_capdata01_get as none of the data provided by the macros >>> that will use it can pass a member of the struct for use in >>> container_of. >>> >>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> >>> --- >>> MAINTAINERS | 1 + >>> drivers/platform/x86/Kconfig | 5 + >>> drivers/platform/x86/Makefile | 1 + >>> drivers/platform/x86/lenovo-wmi-capdata01.c | 140 ++++++++++++++++++++ >>> drivers/platform/x86/lenovo-wmi.h | 19 +++ >>> 5 files changed, 166 insertions(+) >>> create mode 100644 drivers/platform/x86/lenovo-wmi-capdata01.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index cf7f4fce1a25..f6d3e79e50ce 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -13157,6 +13157,7 @@ L: platform-driver-x86@vger.kernel.org >>> S: Maintained >>> F: Documentation/wmi/devices/lenovo-wmi-gamezone.rst >>> F: Documentation/wmi/devices/lenovo-wmi-other.rst >>> +F: drivers/platform/x86/lenovo-wmi-capdata01.c >>> F: drivers/platform/x86/lenovo-wmi-gamezone.c >>> F: drivers/platform/x86/lenovo-wmi.c >>> F: drivers/platform/x86/lenovo-wmi.h >>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >>> index 875822e6bd65..56336dc3c2d0 100644 >>> --- a/drivers/platform/x86/Kconfig >>> +++ b/drivers/platform/x86/Kconfig >>> @@ -475,6 +475,11 @@ config LENOVO_WMI_GAMEZONE >>> To compile this driver as a module, choose M here: the module will >>> be called lenovo-wmi-gamezone. >>> >>> +config LENOVO_WMI_DATA01 >>> + tristate >>> + depends on ACPI_WMI >>> + select LENOVO_WMI >>> + >>> config IDEAPAD_LAPTOP >>> tristate "Lenovo IdeaPad Laptop Extras" >>> depends on ACPI >>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >>> index 4a7b2d14eb82..be9031bea090 100644 >>> --- a/drivers/platform/x86/Makefile >>> +++ b/drivers/platform/x86/Makefile >>> @@ -70,6 +70,7 @@ obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o >>> obj-$(CONFIG_LENOVO_WMI) += lenovo-wmi.o >>> obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o >>> obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o >>> +obj-$(CONFIG_LENOVO_WMI_DATA01) += lenovo-wmi-capdata01.o >>> >>> # Intel >>> obj-y += intel/ >>> diff --git a/drivers/platform/x86/lenovo-wmi-capdata01.c b/drivers/platform/x86/lenovo-wmi-capdata01.c >>> new file mode 100644 >>> index 000000000000..0fe156d5d770 >>> --- /dev/null >>> +++ b/drivers/platform/x86/lenovo-wmi-capdata01.c >>> @@ -0,0 +1,140 @@ >>> +// SPDX-License-Identifier: GPL-2.0-or-later >>> +/* >>> + * LENOVO_CAPABILITY_DATA_01 WMI data block driver. This interface provides >>> + * information on tunable attributes used by the "Other Mode" WMI interface, >>> + * including if it is supported by the hardware, the default_value, max_value, >>> + * min_value, and step increment. >>> + * >>> + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> >> 2025 > Acked for all as Mario suggested. > >>> + */ >>> + >>> +#include <linux/cleanup.h> >>> +#include <linux/component.h> >>> +#include <linux/container_of.h> >>> +#include <linux/device.h> >>> +#include <linux/gfp_types.h> >>> +#include <linux/types.h> >>> +#include <linux/wmi.h> >>> +#include "lenovo-wmi.h" >>> + >>> +/* Interface GUIDs */ >>> +#define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154" >>> + >>> +static int lenovo_cd01_component_bind(struct device *cd01_dev, >>> + struct device *om_dev, void *data) >>> +{ >>> + struct lenovo_wmi_cd01 *cd01 = dev_get_drvdata(cd01_dev); >>> + struct lenovo_wmi_om *om = dev_get_drvdata(om_dev); >> Why not using the *data pointer to pass the cd01 data? This way the capdata driver >> does not need to know the structure of the private data of the lenovo-wmi-other driver. >> > I can do that, sure. Seems preferable TBH as it allows me to call it priv again. > >>> + >>> + om->cd01 = cd01; >>> + return 0; >>> +} >>> + >>> +static void lenovo_cd01_component_unbind(struct device *cd01_dev, >>> + struct device *om_dev, void *data) >>> + >>> +{ >>> + struct wmi_device *om_wdev = >>> + container_of(om_dev, struct wmi_device, dev); >>> + struct lenovo_wmi_om *om = >>> + container_of(&om_wdev, struct lenovo_wmi_om, wdev); >>> + >>> + om->cd01 = NULL; >> I think this is unnecessary, please remove the unbind callback. >> > Acked. > >>> +} >>> + >>> +static const struct component_ops lenovo_cd01_component_ops = { >>> + .bind = lenovo_cd01_component_bind, >>> + .unbind = lenovo_cd01_component_unbind, >>> +}; >>> + >>> +static int lenovo_wmi_cd01_setup(struct lenovo_wmi_cd01 *cd01) >>> +{ >>> + size_t cd_size = sizeof(struct capdata01); >>> + int count, idx; >>> + >>> + count = wmidev_instance_count(cd01->wdev); >>> + >>> + cd01->capdata = devm_kmalloc_array(&cd01->wdev->dev, (size_t)count, >>> + sizeof(*cd01->capdata), GFP_KERNEL); >> Cast to size_t is unnecessary here. >> > Acked. > >>> + if (!cd01->capdata) >>> + return -ENOMEM; >>> + >>> + cd01->instance_count = count; >>> + >>> + for (idx = 0; idx < count; idx++) { >>> + union acpi_object *ret_obj __free(kfree) = NULL; >> I am not sure if the compiler frees ret_obj after each loop iteration. Did you test this? >> > No, but I'm not sure how I would. I was manually using kfree before > but was asked to change to the free macro in an earlier rev. When loading this driver on you machine you can use kmemleak (https://docs.kernel.org/dev-tools/kmemleak.html) to detect memory leaks. If no leaks are detected then ret_obj is likely freed after each iteration and you can keep using __free(). Thanks, Armin Wolf > >>> + struct capdata01 *cap_ptr = >>> + devm_kmalloc(&cd01->wdev->dev, cd_size, GFP_KERNEL); >> Please call devm_kmalloc() on a separate line. >> > Acked. > >>> + ret_obj = wmidev_block_query(cd01->wdev, idx); >>> + if (!ret_obj) >>> + continue; >>> + >>> + if (ret_obj->type != ACPI_TYPE_BUFFER) >>> + continue; >>> + >>> + if (ret_obj->buffer.length != cd_size) >>> + continue; >>> + >>> + memcpy(cap_ptr, ret_obj->buffer.pointer, >>> + ret_obj->buffer.length); >> Using devm_kmemdup() would make sense here. >> > That's a cool function. Ty, I'll use it > >>> + cd01->capdata[idx] = cap_ptr; >>> + } >>> + return 0; >>> +} >>> + >>> +static int lenovo_wmi_cd01_probe(struct wmi_device *wdev, const void *context) >>> + >>> +{ >>> + struct lenovo_wmi_cd01 *cd01; >>> + int ret; >>> + >>> + cd01 = devm_kzalloc(&wdev->dev, sizeof(*cd01), GFP_KERNEL); >>> + if (!cd01) >>> + return -ENOMEM; >>> + >>> + cd01->wdev = wdev; >>> + >>> + ret = lenovo_wmi_cd01_setup(cd01); >>> + if (ret) >>> + return ret; >>> + >>> + dev_set_drvdata(&wdev->dev, cd01); >>> + >>> + ret = component_add(&wdev->dev, &lenovo_cd01_component_ops); >>> + >>> + return ret; >>> +} >>> + >>> +static void lenovo_wmi_cd01_remove(struct wmi_device *wdev) >>> +{ >>> + component_del(&wdev->dev, &lenovo_cd01_component_ops); >>> +} >>> + >>> +static const struct wmi_device_id lenovo_wmi_cd01_id_table[] = { >>> + { LENOVO_CAPABILITY_DATA_01_GUID, NULL }, >>> + {} >>> +}; >>> + >>> +static struct wmi_driver lenovo_wmi_cd01_driver = { >>> + .driver = { >>> + .name = "lenovo_wmi_cd01", >>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS, >>> + }, >>> + .id_table = lenovo_wmi_cd01_id_table, >>> + .probe = lenovo_wmi_cd01_probe, >>> + .remove = lenovo_wmi_cd01_remove, >>> + .no_singleton = true, >>> +}; >>> + >>> +int lenovo_wmi_cd01_match(struct device *dev, void *data) >>> +{ >>> + return dev->driver == &lenovo_wmi_cd01_driver.driver; >>> +} >>> +EXPORT_SYMBOL_GPL(lenovo_wmi_cd01_match); >> Please put this symbol into a namespace too. >> > Yes, I noticed the mistake right after I sent the patch. > >>> + >>> +module_wmi_driver(lenovo_wmi_cd01_driver); >>> + >>> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_cd01_id_table); >>> +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); >>> +MODULE_DESCRIPTION("Lenovo Capability Data 01 WMI Driver"); >>> +MODULE_LICENSE("GPL"); >>> diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h >>> index 113928b4fc0f..07fa67ed89d6 100644 >>> --- a/drivers/platform/x86/lenovo-wmi.h >>> +++ b/drivers/platform/x86/lenovo-wmi.h >>> @@ -45,6 +45,22 @@ enum lenovo_wmi_action { >>> THERMAL_MODE_EVENT = 1, >>> }; >>> >>> +/* capdata01 structs */ >>> +struct lenovo_wmi_cd01 { >>> + struct capdata01 **capdata; >>> + struct wmi_device *wdev; >>> + int instance_count; >>> +}; >>> + >>> +struct capdata01 { >>> + u32 id; >>> + u32 supported; >>> + u32 default_value; >>> + u32 step; >>> + u32 min_value; >>> + u32 max_value; >>> +}; >> Please put those struct definitions into a separate header file. >> > Acked. > >> Thanks, >> Armin Wolf >> >>> + >>> /* wmidev_evaluate_method helper functions */ >>> int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, >>> u32 method_id, u32 arg0, u32 arg1, >>> @@ -52,6 +68,9 @@ int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, >>> int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, >>> u32 method_id, u32 arg0, u32 *retval); >>> >>> +/* lenovo_wmi_cd01_driver match function */ >>> +int lenovo_wmi_cd01_match(struct device *dev, void *data); >>> + >>> /* lenovo_wmi_gz_driver notifier functions */ >>> int lenovo_wmi_gz_notifier_call(struct notifier_block *nb, unsigned long action, >>> enum platform_profile_option *profile);
On Tue, Mar 11, 2025 at 1:34 PM Armin Wolf <W_Armin@gmx.de> wrote: > > Am 10.03.25 um 23:26 schrieb Derek John Clark: > > > On Fri, Mar 7, 2025 at 3:05 PM Armin Wolf <W_Armin@gmx.de> wrote: > >> Am 25.02.25 um 22:59 schrieb Derek J. Clark: > >> > >>> Adds lenovo-wmi-capdata01.c which provides a driver for the > >>> LENOVO_CAPABILITY_DATA_01 WMI data block that comes on "Other Mode" > >>> enabled hardware. Provides an interface for querying if a given > >>> attribute is supported by the hardware, as well as its default_value, > >>> max_value, min_value, and step increment. > >>> v3: > >>> - Add as component to lenovo-wmi-other driver. > >>> v2: > >>> - Use devm_kmalloc to ensure driver can be instanced, remove global > >>> reference. > >>> - Ensure reverse Christmas tree for all variable declarations. > >>> - Remove extra whitespace. > >>> - Use guard(mutex) in all mutex instances, global mutex. > >>> - Use pr_fmt instead of adding the driver name to each pr_err. > >>> - Remove noisy pr_info usage. > >>> - Rename capdata_wmi to lenovo_wmi_cd01_priv and cd01_wmi to priv. > >>> - Use list to get the lenovo_wmi_cd01_priv instance in > >>> lenovo_wmi_capdata01_get as none of the data provided by the macros > >>> that will use it can pass a member of the struct for use in > >>> container_of. > >>> > >>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > >>> --- > >>> MAINTAINERS | 1 + > >>> drivers/platform/x86/Kconfig | 5 + > >>> drivers/platform/x86/Makefile | 1 + > >>> drivers/platform/x86/lenovo-wmi-capdata01.c | 140 ++++++++++++++++++++ > >>> drivers/platform/x86/lenovo-wmi.h | 19 +++ > >>> 5 files changed, 166 insertions(+) > >>> create mode 100644 drivers/platform/x86/lenovo-wmi-capdata01.c > >>> > >>> diff --git a/MAINTAINERS b/MAINTAINERS > >>> index cf7f4fce1a25..f6d3e79e50ce 100644 > >>> --- a/MAINTAINERS > >>> +++ b/MAINTAINERS > >>> @@ -13157,6 +13157,7 @@ L: platform-driver-x86@vger.kernel.org > >>> S: Maintained > >>> F: Documentation/wmi/devices/lenovo-wmi-gamezone.rst > >>> F: Documentation/wmi/devices/lenovo-wmi-other.rst > >>> +F: drivers/platform/x86/lenovo-wmi-capdata01.c > >>> F: drivers/platform/x86/lenovo-wmi-gamezone.c > >>> F: drivers/platform/x86/lenovo-wmi.c > >>> F: drivers/platform/x86/lenovo-wmi.h > >>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > >>> index 875822e6bd65..56336dc3c2d0 100644 > >>> --- a/drivers/platform/x86/Kconfig > >>> +++ b/drivers/platform/x86/Kconfig > >>> @@ -475,6 +475,11 @@ config LENOVO_WMI_GAMEZONE > >>> To compile this driver as a module, choose M here: the module will > >>> be called lenovo-wmi-gamezone. > >>> > >>> +config LENOVO_WMI_DATA01 > >>> + tristate > >>> + depends on ACPI_WMI > >>> + select LENOVO_WMI > >>> + > >>> config IDEAPAD_LAPTOP > >>> tristate "Lenovo IdeaPad Laptop Extras" > >>> depends on ACPI > >>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > >>> index 4a7b2d14eb82..be9031bea090 100644 > >>> --- a/drivers/platform/x86/Makefile > >>> +++ b/drivers/platform/x86/Makefile > >>> @@ -70,6 +70,7 @@ obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o > >>> obj-$(CONFIG_LENOVO_WMI) += lenovo-wmi.o > >>> obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o > >>> obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o > >>> +obj-$(CONFIG_LENOVO_WMI_DATA01) += lenovo-wmi-capdata01.o > >>> > >>> # Intel > >>> obj-y += intel/ > >>> diff --git a/drivers/platform/x86/lenovo-wmi-capdata01.c b/drivers/platform/x86/lenovo-wmi-capdata01.c > >>> new file mode 100644 > >>> index 000000000000..0fe156d5d770 > >>> --- /dev/null > >>> +++ b/drivers/platform/x86/lenovo-wmi-capdata01.c > >>> @@ -0,0 +1,140 @@ > >>> +// SPDX-License-Identifier: GPL-2.0-or-later > >>> +/* > >>> + * LENOVO_CAPABILITY_DATA_01 WMI data block driver. This interface provides > >>> + * information on tunable attributes used by the "Other Mode" WMI interface, > >>> + * including if it is supported by the hardware, the default_value, max_value, > >>> + * min_value, and step increment. > >>> + * > >>> + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> > >> 2025 > > Acked for all as Mario suggested. > > > >>> + */ > >>> + > >>> +#include <linux/cleanup.h> > >>> +#include <linux/component.h> > >>> +#include <linux/container_of.h> > >>> +#include <linux/device.h> > >>> +#include <linux/gfp_types.h> > >>> +#include <linux/types.h> > >>> +#include <linux/wmi.h> > >>> +#include "lenovo-wmi.h" > >>> + > >>> +/* Interface GUIDs */ > >>> +#define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154" > >>> + > >>> +static int lenovo_cd01_component_bind(struct device *cd01_dev, > >>> + struct device *om_dev, void *data) > >>> +{ > >>> + struct lenovo_wmi_cd01 *cd01 = dev_get_drvdata(cd01_dev); > >>> + struct lenovo_wmi_om *om = dev_get_drvdata(om_dev); > >> Why not using the *data pointer to pass the cd01 data? This way the capdata driver > >> does not need to know the structure of the private data of the lenovo-wmi-other driver. > >> > > I can do that, sure. Seems preferable TBH as it allows me to call it priv again. > > > >>> + > >>> + om->cd01 = cd01; > >>> + return 0; > >>> +} > >>> + > >>> +static void lenovo_cd01_component_unbind(struct device *cd01_dev, > >>> + struct device *om_dev, void *data) > >>> + > >>> +{ > >>> + struct wmi_device *om_wdev = > >>> + container_of(om_dev, struct wmi_device, dev); > >>> + struct lenovo_wmi_om *om = > >>> + container_of(&om_wdev, struct lenovo_wmi_om, wdev); > >>> + > >>> + om->cd01 = NULL; > >> I think this is unnecessary, please remove the unbind callback. > >> > > Acked. > > > >>> +} > >>> + > >>> +static const struct component_ops lenovo_cd01_component_ops = { > >>> + .bind = lenovo_cd01_component_bind, > >>> + .unbind = lenovo_cd01_component_unbind, > >>> +}; > >>> + > >>> +static int lenovo_wmi_cd01_setup(struct lenovo_wmi_cd01 *cd01) > >>> +{ > >>> + size_t cd_size = sizeof(struct capdata01); > >>> + int count, idx; > >>> + > >>> + count = wmidev_instance_count(cd01->wdev); > >>> + > >>> + cd01->capdata = devm_kmalloc_array(&cd01->wdev->dev, (size_t)count, > >>> + sizeof(*cd01->capdata), GFP_KERNEL); > >> Cast to size_t is unnecessary here. > >> > > Acked. > > > >>> + if (!cd01->capdata) > >>> + return -ENOMEM; > >>> + > >>> + cd01->instance_count = count; > >>> + > >>> + for (idx = 0; idx < count; idx++) { > >>> + union acpi_object *ret_obj __free(kfree) = NULL; > >> I am not sure if the compiler frees ret_obj after each loop iteration. Did you test this? > >> > > No, but I'm not sure how I would. I was manually using kfree before > > but was asked to change to the free macro in an earlier rev. > > When loading this driver on you machine you can use kmemleak (https://docs.kernel.org/dev-tools/kmemleak.html) > to detect memory leaks. If no leaks are detected then ret_obj is likely freed after each iteration and you > can keep using __free(). > > Thanks, > Armin Wolf > No leaks were detected. I'll leave this in for v4. Thanks, Derek > > > >>> + struct capdata01 *cap_ptr = > >>> + devm_kmalloc(&cd01->wdev->dev, cd_size, GFP_KERNEL); > >> Please call devm_kmalloc() on a separate line. > >> > > Acked. > > > >>> + ret_obj = wmidev_block_query(cd01->wdev, idx); > >>> + if (!ret_obj) > >>> + continue; > >>> + > >>> + if (ret_obj->type != ACPI_TYPE_BUFFER) > >>> + continue; > >>> + > >>> + if (ret_obj->buffer.length != cd_size) > >>> + continue; > >>> + > >>> + memcpy(cap_ptr, ret_obj->buffer.pointer, > >>> + ret_obj->buffer.length); > >> Using devm_kmemdup() would make sense here. > >> > > That's a cool function. Ty, I'll use it > > > >>> + cd01->capdata[idx] = cap_ptr; > >>> + } > >>> + return 0; > >>> +} > >>> + > >>> +static int lenovo_wmi_cd01_probe(struct wmi_device *wdev, const void *context) > >>> + > >>> +{ > >>> + struct lenovo_wmi_cd01 *cd01; > >>> + int ret; > >>> + > >>> + cd01 = devm_kzalloc(&wdev->dev, sizeof(*cd01), GFP_KERNEL); > >>> + if (!cd01) > >>> + return -ENOMEM; > >>> + > >>> + cd01->wdev = wdev; > >>> + > >>> + ret = lenovo_wmi_cd01_setup(cd01); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + dev_set_drvdata(&wdev->dev, cd01); > >>> + > >>> + ret = component_add(&wdev->dev, &lenovo_cd01_component_ops); > >>> + > >>> + return ret; > >>> +} > >>> + > >>> +static void lenovo_wmi_cd01_remove(struct wmi_device *wdev) > >>> +{ > >>> + component_del(&wdev->dev, &lenovo_cd01_component_ops); > >>> +} > >>> + > >>> +static const struct wmi_device_id lenovo_wmi_cd01_id_table[] = { > >>> + { LENOVO_CAPABILITY_DATA_01_GUID, NULL }, > >>> + {} > >>> +}; > >>> + > >>> +static struct wmi_driver lenovo_wmi_cd01_driver = { > >>> + .driver = { > >>> + .name = "lenovo_wmi_cd01", > >>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > >>> + }, > >>> + .id_table = lenovo_wmi_cd01_id_table, > >>> + .probe = lenovo_wmi_cd01_probe, > >>> + .remove = lenovo_wmi_cd01_remove, > >>> + .no_singleton = true, > >>> +}; > >>> + > >>> +int lenovo_wmi_cd01_match(struct device *dev, void *data) > >>> +{ > >>> + return dev->driver == &lenovo_wmi_cd01_driver.driver; > >>> +} > >>> +EXPORT_SYMBOL_GPL(lenovo_wmi_cd01_match); > >> Please put this symbol into a namespace too. > >> > > Yes, I noticed the mistake right after I sent the patch. > > > >>> + > >>> +module_wmi_driver(lenovo_wmi_cd01_driver); > >>> + > >>> +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_cd01_id_table); > >>> +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); > >>> +MODULE_DESCRIPTION("Lenovo Capability Data 01 WMI Driver"); > >>> +MODULE_LICENSE("GPL"); > >>> diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h > >>> index 113928b4fc0f..07fa67ed89d6 100644 > >>> --- a/drivers/platform/x86/lenovo-wmi.h > >>> +++ b/drivers/platform/x86/lenovo-wmi.h > >>> @@ -45,6 +45,22 @@ enum lenovo_wmi_action { > >>> THERMAL_MODE_EVENT = 1, > >>> }; > >>> > >>> +/* capdata01 structs */ > >>> +struct lenovo_wmi_cd01 { > >>> + struct capdata01 **capdata; > >>> + struct wmi_device *wdev; > >>> + int instance_count; > >>> +}; > >>> + > >>> +struct capdata01 { > >>> + u32 id; > >>> + u32 supported; > >>> + u32 default_value; > >>> + u32 step; > >>> + u32 min_value; > >>> + u32 max_value; > >>> +}; > >> Please put those struct definitions into a separate header file. > >> > > Acked. > > > >> Thanks, > >> Armin Wolf > >> > >>> + > >>> /* wmidev_evaluate_method helper functions */ > >>> int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > >>> u32 method_id, u32 arg0, u32 arg1, > >>> @@ -52,6 +68,9 @@ int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, > >>> int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, > >>> u32 method_id, u32 arg0, u32 *retval); > >>> > >>> +/* lenovo_wmi_cd01_driver match function */ > >>> +int lenovo_wmi_cd01_match(struct device *dev, void *data); > >>> + > >>> /* lenovo_wmi_gz_driver notifier functions */ > >>> int lenovo_wmi_gz_notifier_call(struct notifier_block *nb, unsigned long action, > >>> enum platform_profile_option *profile);
diff --git a/MAINTAINERS b/MAINTAINERS index cf7f4fce1a25..f6d3e79e50ce 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13157,6 +13157,7 @@ L: platform-driver-x86@vger.kernel.org S: Maintained F: Documentation/wmi/devices/lenovo-wmi-gamezone.rst F: Documentation/wmi/devices/lenovo-wmi-other.rst +F: drivers/platform/x86/lenovo-wmi-capdata01.c F: drivers/platform/x86/lenovo-wmi-gamezone.c F: drivers/platform/x86/lenovo-wmi.c F: drivers/platform/x86/lenovo-wmi.h diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 875822e6bd65..56336dc3c2d0 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -475,6 +475,11 @@ config LENOVO_WMI_GAMEZONE To compile this driver as a module, choose M here: the module will be called lenovo-wmi-gamezone. +config LENOVO_WMI_DATA01 + tristate + depends on ACPI_WMI + select LENOVO_WMI + config IDEAPAD_LAPTOP tristate "Lenovo IdeaPad Laptop Extras" depends on ACPI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 4a7b2d14eb82..be9031bea090 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_YT2_1380) += lenovo-yoga-tab2-pro-1380-fastcharger.o obj-$(CONFIG_LENOVO_WMI) += lenovo-wmi.o obj-$(CONFIG_LENOVO_WMI_CAMERA) += lenovo-wmi-camera.o obj-$(CONFIG_LENOVO_WMI_GAMEZONE) += lenovo-wmi-gamezone.o +obj-$(CONFIG_LENOVO_WMI_DATA01) += lenovo-wmi-capdata01.o # Intel obj-y += intel/ diff --git a/drivers/platform/x86/lenovo-wmi-capdata01.c b/drivers/platform/x86/lenovo-wmi-capdata01.c new file mode 100644 index 000000000000..0fe156d5d770 --- /dev/null +++ b/drivers/platform/x86/lenovo-wmi-capdata01.c @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * LENOVO_CAPABILITY_DATA_01 WMI data block driver. This interface provides + * information on tunable attributes used by the "Other Mode" WMI interface, + * including if it is supported by the hardware, the default_value, max_value, + * min_value, and step increment. + * + * Copyright(C) 2024 Derek J. Clark <derekjohn.clark@gmail.com> + */ + +#include <linux/cleanup.h> +#include <linux/component.h> +#include <linux/container_of.h> +#include <linux/device.h> +#include <linux/gfp_types.h> +#include <linux/types.h> +#include <linux/wmi.h> +#include "lenovo-wmi.h" + +/* Interface GUIDs */ +#define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154" + +static int lenovo_cd01_component_bind(struct device *cd01_dev, + struct device *om_dev, void *data) +{ + struct lenovo_wmi_cd01 *cd01 = dev_get_drvdata(cd01_dev); + struct lenovo_wmi_om *om = dev_get_drvdata(om_dev); + + om->cd01 = cd01; + return 0; +} + +static void lenovo_cd01_component_unbind(struct device *cd01_dev, + struct device *om_dev, void *data) + +{ + struct wmi_device *om_wdev = + container_of(om_dev, struct wmi_device, dev); + struct lenovo_wmi_om *om = + container_of(&om_wdev, struct lenovo_wmi_om, wdev); + + om->cd01 = NULL; +} + +static const struct component_ops lenovo_cd01_component_ops = { + .bind = lenovo_cd01_component_bind, + .unbind = lenovo_cd01_component_unbind, +}; + +static int lenovo_wmi_cd01_setup(struct lenovo_wmi_cd01 *cd01) +{ + size_t cd_size = sizeof(struct capdata01); + int count, idx; + + count = wmidev_instance_count(cd01->wdev); + + cd01->capdata = devm_kmalloc_array(&cd01->wdev->dev, (size_t)count, + sizeof(*cd01->capdata), GFP_KERNEL); + if (!cd01->capdata) + return -ENOMEM; + + cd01->instance_count = count; + + for (idx = 0; idx < count; idx++) { + union acpi_object *ret_obj __free(kfree) = NULL; + struct capdata01 *cap_ptr = + devm_kmalloc(&cd01->wdev->dev, cd_size, GFP_KERNEL); + ret_obj = wmidev_block_query(cd01->wdev, idx); + if (!ret_obj) + continue; + + if (ret_obj->type != ACPI_TYPE_BUFFER) + continue; + + if (ret_obj->buffer.length != cd_size) + continue; + + memcpy(cap_ptr, ret_obj->buffer.pointer, + ret_obj->buffer.length); + cd01->capdata[idx] = cap_ptr; + } + return 0; +} + +static int lenovo_wmi_cd01_probe(struct wmi_device *wdev, const void *context) + +{ + struct lenovo_wmi_cd01 *cd01; + int ret; + + cd01 = devm_kzalloc(&wdev->dev, sizeof(*cd01), GFP_KERNEL); + if (!cd01) + return -ENOMEM; + + cd01->wdev = wdev; + + ret = lenovo_wmi_cd01_setup(cd01); + if (ret) + return ret; + + dev_set_drvdata(&wdev->dev, cd01); + + ret = component_add(&wdev->dev, &lenovo_cd01_component_ops); + + return ret; +} + +static void lenovo_wmi_cd01_remove(struct wmi_device *wdev) +{ + component_del(&wdev->dev, &lenovo_cd01_component_ops); +} + +static const struct wmi_device_id lenovo_wmi_cd01_id_table[] = { + { LENOVO_CAPABILITY_DATA_01_GUID, NULL }, + {} +}; + +static struct wmi_driver lenovo_wmi_cd01_driver = { + .driver = { + .name = "lenovo_wmi_cd01", + .probe_type = PROBE_PREFER_ASYNCHRONOUS, + }, + .id_table = lenovo_wmi_cd01_id_table, + .probe = lenovo_wmi_cd01_probe, + .remove = lenovo_wmi_cd01_remove, + .no_singleton = true, +}; + +int lenovo_wmi_cd01_match(struct device *dev, void *data) +{ + return dev->driver == &lenovo_wmi_cd01_driver.driver; +} +EXPORT_SYMBOL_GPL(lenovo_wmi_cd01_match); + +module_wmi_driver(lenovo_wmi_cd01_driver); + +MODULE_DEVICE_TABLE(wmi, lenovo_wmi_cd01_id_table); +MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>"); +MODULE_DESCRIPTION("Lenovo Capability Data 01 WMI Driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/platform/x86/lenovo-wmi.h b/drivers/platform/x86/lenovo-wmi.h index 113928b4fc0f..07fa67ed89d6 100644 --- a/drivers/platform/x86/lenovo-wmi.h +++ b/drivers/platform/x86/lenovo-wmi.h @@ -45,6 +45,22 @@ enum lenovo_wmi_action { THERMAL_MODE_EVENT = 1, }; +/* capdata01 structs */ +struct lenovo_wmi_cd01 { + struct capdata01 **capdata; + struct wmi_device *wdev; + int instance_count; +}; + +struct capdata01 { + u32 id; + u32 supported; + u32 default_value; + u32 step; + u32 min_value; + u32 max_value; +}; + /* wmidev_evaluate_method helper functions */ int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, u32 method_id, u32 arg0, u32 arg1, @@ -52,6 +68,9 @@ int lenovo_wmidev_evaluate_method_2(struct wmi_device *wdev, u8 instance, int lenovo_wmidev_evaluate_method_1(struct wmi_device *wdev, u8 instance, u32 method_id, u32 arg0, u32 *retval); +/* lenovo_wmi_cd01_driver match function */ +int lenovo_wmi_cd01_match(struct device *dev, void *data); + /* lenovo_wmi_gz_driver notifier functions */ int lenovo_wmi_gz_notifier_call(struct notifier_block *nb, unsigned long action, enum platform_profile_option *profile);
Adds lenovo-wmi-capdata01.c which provides a driver for the LENOVO_CAPABILITY_DATA_01 WMI data block that comes on "Other Mode" enabled hardware. Provides an interface for querying if a given attribute is supported by the hardware, as well as its default_value, max_value, min_value, and step increment. v3: - Add as component to lenovo-wmi-other driver. v2: - Use devm_kmalloc to ensure driver can be instanced, remove global reference. - Ensure reverse Christmas tree for all variable declarations. - Remove extra whitespace. - Use guard(mutex) in all mutex instances, global mutex. - Use pr_fmt instead of adding the driver name to each pr_err. - Remove noisy pr_info usage. - Rename capdata_wmi to lenovo_wmi_cd01_priv and cd01_wmi to priv. - Use list to get the lenovo_wmi_cd01_priv instance in lenovo_wmi_capdata01_get as none of the data provided by the macros that will use it can pass a member of the struct for use in container_of. Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> --- MAINTAINERS | 1 + drivers/platform/x86/Kconfig | 5 + drivers/platform/x86/Makefile | 1 + drivers/platform/x86/lenovo-wmi-capdata01.c | 140 ++++++++++++++++++++ drivers/platform/x86/lenovo-wmi.h | 19 +++ 5 files changed, 166 insertions(+) create mode 100644 drivers/platform/x86/lenovo-wmi-capdata01.c