| Message ID | 20231014032844.3823198-1-aichao@kylinos.cn (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v2] platform/x86: support to store/show powermode value for Inspur | expand |
Am 14.10.23 um 05:28 schrieb Ai Chao: > Support to store/show powermode value for Inspur by WMI interface. > This driver provides support for Inspur WMI hotkeys. User used Fn+Q to > change the power mode. If desktop applications receive hotkey(Fn+Q) > event, then it get the currently power mode and change the power mode. > The desktop applications modify brightness and cpufreq based on > power mode. > > change for v2 > - Remove Event GUID, remove inspur_wmi_notify and inspur_wmi_notify. > - Add more explanation. > > Signed-off-by: Ai Chao <aichao@kylinos.cn> > --- > drivers/platform/x86/Kconfig | 14 ++ > drivers/platform/x86/Makefile | 3 + > drivers/platform/x86/inspur-wmi.c | 210 ++++++++++++++++++++++++++++++ > 3 files changed, 227 insertions(+) > create mode 100644 drivers/platform/x86/inspur-wmi.c > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 2a1070543391..fa2a4335c83d 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -988,6 +988,20 @@ config TOUCHSCREEN_DMI > the OS-image for the device. This option supplies the missing info. > Enable this for x86 tablets with Silead or Chipone touchscreens. > > +config INSPUR_WMI > + tristate "Inspur WMI hotkeys driver" > + depends on ACPI_WMI > + depends on INPUT The driver does not provide a input device anymore, please remove the dependency on CONFIG_INPUT. > + help > + This driver provides support for Inspur WMI hotkeys. > + User used Fn+Q to change the power mode. If desktop applications > + receive hotkeys(Fn+Q) event, then it get the currently power mode > + and change the power mode. The desktop applications modify brightness > + and cpufreq based on power mode. > + I assume the whole hotkey stuff uses the other WMI GUID, right? In this case the current driver should not be called "Inspur WMI hotkeys driver", since it does not handle those hotkey notifications. Better call it "Inspur WMI power mode driver", and remove all references to the hotkeys. > + To compile this driver as a module, choose M here: the module > + will be called inspur-wmi. > + > source "drivers/platform/x86/x86-android-tablets/Kconfig" > > config FW_ATTR_CLASS > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index b457de5abf7d..9285c252757e 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -98,6 +98,9 @@ obj-$(CONFIG_TOSHIBA_WMI) += toshiba-wmi.o > # before toshiba_acpi initializes > obj-$(CONFIG_ACPI_TOSHIBA) += toshiba_acpi.o > > +# Inspur > +obj-$(CONFIG_INSPUR_WMI) += inspur-wmi.o > + > # Laptop drivers > obj-$(CONFIG_ACPI_CMPC) += classmate-laptop.o > obj-$(CONFIG_COMPAL_LAPTOP) += compal-laptop.o > diff --git a/drivers/platform/x86/inspur-wmi.c b/drivers/platform/x86/inspur-wmi.c > new file mode 100644 > index 000000000000..ef6cfd87f074 > --- /dev/null > +++ b/drivers/platform/x86/inspur-wmi.c > @@ -0,0 +1,210 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Inspur WMI hotkeys > + * > + * Copyright (C) 2018 Ai Chao <aichao@kylinos.cn> > + */ > + > +#include <linux/acpi.h> > +#include <linux/device.h> > +#include <linux/input.h> > +#include <linux/module.h> > +#include <linux/wmi.h> > + > +#define WMI_INSPUR_POWERMODE_BIOS_GUID "596C31E3-332D-43C9-AEE9-585493284F5D" > + > +enum inspur_wmi_method_ids { > + INSPUR_WMI_GET_POWERMODE = 0x02, > + INSPUR_WMI_SET_POWERMODE = 0x03, > +}; > + > +struct inspur_wmi_priv { > + struct input_dev *idev; > + struct wmi_device *wdev; > +}; > + > +static int inspur_wmi_perform_query(struct wmi_device *wdev, > + enum inspur_wmi_method_ids query_id, > + void *buffer, size_t insize, > + size_t outsize) > +{ > + union acpi_object *obj; > + acpi_status status; > + int ret = 0; > + struct acpi_buffer input = { insize, buffer}; > + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; Please order the variable declarations in a reverse xmas tree order, seeDocumentation/process/maintainer-netdev.rst#Local variable ordering for details. > + > + status = wmidev_evaluate_method(wdev, 0, query_id, &input, &output); > + if (ACPI_FAILURE(status)) { > + dev_err(&wdev->dev, "EC Powermode control failed: %s\n", > + acpi_format_exception(status)); > + return -EIO; > + } > + > + obj = output.pointer; > + if (!obj) > + return -EINVAL; > + > + if (obj->type != ACPI_TYPE_BUFFER) { > + ret = -EINVAL; > + goto out_free; > + } > + > + /* Ignore output data of zero size */ > + if (!outsize) > + goto out_free; Since all users of this functions set outsize to something greater than zero, this check is pointless. Please remove it. > + > + if (obj->buffer.length != outsize) { > + ret = -EINVAL; > + goto out_free; > + } > + > + memcpy(buffer, obj->buffer.pointer, obj->buffer.length); > + > +out_free: > + kfree(obj); > + return ret; > +} > + > +/** > + * Set Power Mode to EC RAM. If Power Mode value greater than 0x3, > + * return error > + * Method ID: 0x3 > + * Arg: 4 Bytes > + * Byte [0]: Power Mode: > + * 0x0: Balance Mode > + * 0x1: Performance Mode > + * 0x2: Power Saver Mode > + * Return Value: 4 Bytes > + * Byte [0]: Return Code > + * 0x0: No Error > + * 0x1: Error > + */ Now i understand what this power mode means. Why not use the platform profile interface for this? All tree modes (Balance, Performance, Power Saver) would perfectly map to standard platform profiles (BALANCED, PERFORMANCE, LOW_POWER), and using the platform profile interface would allow userspace software to control the power mode over a standard interface. I believe drivers/platform/surface/surface_platform_profile.c is a good example for such a driver, _except_ that it ignores the return value of platform_profile_register(). > +static ssize_t powermode_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct inspur_wmi_priv *priv = dev_get_drvdata(dev); > + int ret; > + u32 mode; Since the buffer passed to the WMI method is structured in bytes, maybe you could use a byte array in those cases? This would remove the need to cast "mode" to a "u8 *". > + u8 *ret_code; > + > + ret = kstrtoint(buf, 0, &mode); > + if (ret) > + return ret; > + > + ret = inspur_wmi_perform_query(priv->wdev, > + INSPUR_WMI_SET_POWERMODE, > + &mode, sizeof(mode), sizeof(mode)); > + > + if (ret < 0) > + return ret; > + > + ret_code = (u8 *)(&mode); > + if (ret_code[0]) > + return -EBADRQC; > + > + return count; > +} > + > +/** > + * Get Power Mode from EC RAM, If Power Mode value greater than 0x3, > + * return error > + * Method ID: 0x2 > + * Return Value: 4 Bytes > + * Byte [0]: Return Code > + * 0x0: No Error > + * 0x1: Error > + * Byte [1]: Power Mode > + * 0x0: Balance Mode > + * 0x1: Performance Mode > + * 0x2: Power Saver Mode > + */ > +static ssize_t powermode_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct inspur_wmi_priv *priv = dev_get_drvdata(dev); > + u32 mode = 0; Same as above. > + int ret; > + u8 *ret_code; > + > + ret = inspur_wmi_perform_query(priv->wdev, > + INSPUR_WMI_GET_POWERMODE, > + &mode, sizeof(mode), sizeof(mode)); > + if (ret < 0) > + return ret; > + > + ret_code = (u8 *)(&mode); > + if (ret_code[0]) > + return -EBADRQC; > + > + return sprintf(buf, "%u\n", ret_code[1]); > +} > + > +static DEVICE_ATTR_RW(powermode); > + > +static struct attribute *inspur_wmi_attrs[] = { > + &dev_attr_powermode.attr, > + NULL, > +}; > + > +static const struct attribute_group inspur_wmi_group = { > + .attrs = inspur_wmi_attrs, > +}; > + > +static const struct attribute_group *inspur_wmi_groups[] = { > + &inspur_wmi_group, > + NULL, > +}; > + > +static int inspur_wmi_input_setup(struct wmi_device *wdev) > +{ > + struct inspur_wmi_priv *priv = dev_get_drvdata(&wdev->dev); > + > + priv->idev = devm_input_allocate_device(&wdev->dev); > + if (!priv->idev) > + return -ENOMEM; > + > + priv->idev->name = "Inspur WMI hotkeys"; > + priv->idev->phys = "wmi/input0"; > + priv->idev->id.bustype = BUS_HOST; > + priv->idev->dev.parent = &wdev->dev; > + > + return input_register_device(priv->idev); > +} Please remove all remains of the hotkey handling. > + > +static int inspur_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct inspur_wmi_priv *priv; > + > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->wdev = wdev; > + dev_set_drvdata(&wdev->dev, priv); > + > + return inspur_wmi_input_setup(wdev); > +} > + > +static const struct wmi_device_id inspur_wmi_id_table[] = { > + { .guid_string = WMI_INSPUR_POWERMODE_BIOS_GUID }, > + { } > +}; > + > +static struct wmi_driver inspur_wmi_driver = { > + .driver = { > + .name = "inspur-wmi", > + .dev_groups = inspur_wmi_groups, > + }, > + .id_table = inspur_wmi_id_table, > + .probe = inspur_wmi_probe, > +}; > + > +module_wmi_driver(inspur_wmi_driver); > + > +MODULE_DEVICE_TABLE(wmi, inspur_wmi_id_table); Can you move this right below "inspur_wmi_id_table" please? > +MODULE_AUTHOR("Ai Chao <aichao@kylinos.cn>"); > +MODULE_DESCRIPTION("Inspur WMI hotkeys"); > +MODULE_LICENSE("GPL");
Hi Ai, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on v6.6-rc6 next-20231020] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Ai-Chao/platform-x86-support-to-store-show-powermode-value-for-Inspur/20231017-125537 base: linus/master patch link: https://lore.kernel.org/r/20231014032844.3823198-1-aichao%40kylinos.cn patch subject: [PATCH v2] platform/x86: support to store/show powermode value for Inspur config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231020/202310202354.NjaAKTX2-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231020/202310202354.NjaAKTX2-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310202354.NjaAKTX2-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/platform/x86/inspur-wmi.c:70: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst * Set Power Mode to EC RAM. If Power Mode value greater than 0x3, drivers/platform/x86/inspur-wmi.c:111: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst * Get Power Mode from EC RAM, If Power Mode value greater than 0x3, vim +70 drivers/platform/x86/inspur-wmi.c 68 69 /** > 70 * Set Power Mode to EC RAM. If Power Mode value greater than 0x3, 71 * return error 72 * Method ID: 0x3 73 * Arg: 4 Bytes 74 * Byte [0]: Power Mode: 75 * 0x0: Balance Mode 76 * 0x1: Performance Mode 77 * 0x2: Power Saver Mode 78 * Return Value: 4 Bytes 79 * Byte [0]: Return Code 80 * 0x0: No Error 81 * 0x1: Error 82 */ 83 static ssize_t powermode_store(struct device *dev, 84 struct device_attribute *attr, 85 const char *buf, size_t count) 86 { 87 struct inspur_wmi_priv *priv = dev_get_drvdata(dev); 88 int ret; 89 u32 mode; 90 u8 *ret_code; 91 92 ret = kstrtoint(buf, 0, &mode); 93 if (ret) 94 return ret; 95 96 ret = inspur_wmi_perform_query(priv->wdev, 97 INSPUR_WMI_SET_POWERMODE, 98 &mode, sizeof(mode), sizeof(mode)); 99 100 if (ret < 0) 101 return ret; 102 103 ret_code = (u8 *)(&mode); 104 if (ret_code[0]) 105 return -EBADRQC; 106 107 return count; 108 } 109
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 2a1070543391..fa2a4335c83d 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -988,6 +988,20 @@ config TOUCHSCREEN_DMI the OS-image for the device. This option supplies the missing info. Enable this for x86 tablets with Silead or Chipone touchscreens. +config INSPUR_WMI + tristate "Inspur WMI hotkeys driver" + depends on ACPI_WMI + depends on INPUT + help + This driver provides support for Inspur WMI hotkeys. + User used Fn+Q to change the power mode. If desktop applications + receive hotkeys(Fn+Q) event, then it get the currently power mode + and change the power mode. The desktop applications modify brightness + and cpufreq based on power mode. + + To compile this driver as a module, choose M here: the module + will be called inspur-wmi. + source "drivers/platform/x86/x86-android-tablets/Kconfig" config FW_ATTR_CLASS diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index b457de5abf7d..9285c252757e 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -98,6 +98,9 @@ obj-$(CONFIG_TOSHIBA_WMI) += toshiba-wmi.o # before toshiba_acpi initializes obj-$(CONFIG_ACPI_TOSHIBA) += toshiba_acpi.o +# Inspur +obj-$(CONFIG_INSPUR_WMI) += inspur-wmi.o + # Laptop drivers obj-$(CONFIG_ACPI_CMPC) += classmate-laptop.o obj-$(CONFIG_COMPAL_LAPTOP) += compal-laptop.o diff --git a/drivers/platform/x86/inspur-wmi.c b/drivers/platform/x86/inspur-wmi.c new file mode 100644 index 000000000000..ef6cfd87f074 --- /dev/null +++ b/drivers/platform/x86/inspur-wmi.c @@ -0,0 +1,210 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Inspur WMI hotkeys + * + * Copyright (C) 2018 Ai Chao <aichao@kylinos.cn> + */ + +#include <linux/acpi.h> +#include <linux/device.h> +#include <linux/input.h> +#include <linux/module.h> +#include <linux/wmi.h> + +#define WMI_INSPUR_POWERMODE_BIOS_GUID "596C31E3-332D-43C9-AEE9-585493284F5D" + +enum inspur_wmi_method_ids { + INSPUR_WMI_GET_POWERMODE = 0x02, + INSPUR_WMI_SET_POWERMODE = 0x03, +}; + +struct inspur_wmi_priv { + struct input_dev *idev; + struct wmi_device *wdev; +}; + +static int inspur_wmi_perform_query(struct wmi_device *wdev, + enum inspur_wmi_method_ids query_id, + void *buffer, size_t insize, + size_t outsize) +{ + union acpi_object *obj; + acpi_status status; + int ret = 0; + struct acpi_buffer input = { insize, buffer}; + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; + + status = wmidev_evaluate_method(wdev, 0, query_id, &input, &output); + if (ACPI_FAILURE(status)) { + dev_err(&wdev->dev, "EC Powermode control failed: %s\n", + acpi_format_exception(status)); + return -EIO; + } + + obj = output.pointer; + if (!obj) + return -EINVAL; + + if (obj->type != ACPI_TYPE_BUFFER) { + ret = -EINVAL; + goto out_free; + } + + /* Ignore output data of zero size */ + if (!outsize) + goto out_free; + + if (obj->buffer.length != outsize) { + ret = -EINVAL; + goto out_free; + } + + memcpy(buffer, obj->buffer.pointer, obj->buffer.length); + +out_free: + kfree(obj); + return ret; +} + +/** + * Set Power Mode to EC RAM. If Power Mode value greater than 0x3, + * return error + * Method ID: 0x3 + * Arg: 4 Bytes + * Byte [0]: Power Mode: + * 0x0: Balance Mode + * 0x1: Performance Mode + * 0x2: Power Saver Mode + * Return Value: 4 Bytes + * Byte [0]: Return Code + * 0x0: No Error + * 0x1: Error + */ +static ssize_t powermode_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct inspur_wmi_priv *priv = dev_get_drvdata(dev); + int ret; + u32 mode; + u8 *ret_code; + + ret = kstrtoint(buf, 0, &mode); + if (ret) + return ret; + + ret = inspur_wmi_perform_query(priv->wdev, + INSPUR_WMI_SET_POWERMODE, + &mode, sizeof(mode), sizeof(mode)); + + if (ret < 0) + return ret; + + ret_code = (u8 *)(&mode); + if (ret_code[0]) + return -EBADRQC; + + return count; +} + +/** + * Get Power Mode from EC RAM, If Power Mode value greater than 0x3, + * return error + * Method ID: 0x2 + * Return Value: 4 Bytes + * Byte [0]: Return Code + * 0x0: No Error + * 0x1: Error + * Byte [1]: Power Mode + * 0x0: Balance Mode + * 0x1: Performance Mode + * 0x2: Power Saver Mode + */ +static ssize_t powermode_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct inspur_wmi_priv *priv = dev_get_drvdata(dev); + u32 mode = 0; + int ret; + u8 *ret_code; + + ret = inspur_wmi_perform_query(priv->wdev, + INSPUR_WMI_GET_POWERMODE, + &mode, sizeof(mode), sizeof(mode)); + if (ret < 0) + return ret; + + ret_code = (u8 *)(&mode); + if (ret_code[0]) + return -EBADRQC; + + return sprintf(buf, "%u\n", ret_code[1]); +} + +static DEVICE_ATTR_RW(powermode); + +static struct attribute *inspur_wmi_attrs[] = { + &dev_attr_powermode.attr, + NULL, +}; + +static const struct attribute_group inspur_wmi_group = { + .attrs = inspur_wmi_attrs, +}; + +static const struct attribute_group *inspur_wmi_groups[] = { + &inspur_wmi_group, + NULL, +}; + +static int inspur_wmi_input_setup(struct wmi_device *wdev) +{ + struct inspur_wmi_priv *priv = dev_get_drvdata(&wdev->dev); + + priv->idev = devm_input_allocate_device(&wdev->dev); + if (!priv->idev) + return -ENOMEM; + + priv->idev->name = "Inspur WMI hotkeys"; + priv->idev->phys = "wmi/input0"; + priv->idev->id.bustype = BUS_HOST; + priv->idev->dev.parent = &wdev->dev; + + return input_register_device(priv->idev); +} + +static int inspur_wmi_probe(struct wmi_device *wdev, const void *context) +{ + struct inspur_wmi_priv *priv; + + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->wdev = wdev; + dev_set_drvdata(&wdev->dev, priv); + + return inspur_wmi_input_setup(wdev); +} + +static const struct wmi_device_id inspur_wmi_id_table[] = { + { .guid_string = WMI_INSPUR_POWERMODE_BIOS_GUID }, + { } +}; + +static struct wmi_driver inspur_wmi_driver = { + .driver = { + .name = "inspur-wmi", + .dev_groups = inspur_wmi_groups, + }, + .id_table = inspur_wmi_id_table, + .probe = inspur_wmi_probe, +}; + +module_wmi_driver(inspur_wmi_driver); + +MODULE_DEVICE_TABLE(wmi, inspur_wmi_id_table); +MODULE_AUTHOR("Ai Chao <aichao@kylinos.cn>"); +MODULE_DESCRIPTION("Inspur WMI hotkeys"); +MODULE_LICENSE("GPL");
Support to store/show powermode value for Inspur by WMI interface. This driver provides support for Inspur WMI hotkeys. User used Fn+Q to change the power mode. If desktop applications receive hotkey(Fn+Q) event, then it get the currently power mode and change the power mode. The desktop applications modify brightness and cpufreq based on power mode. change for v2 - Remove Event GUID, remove inspur_wmi_notify and inspur_wmi_notify. - Add more explanation. Signed-off-by: Ai Chao <aichao@kylinos.cn> --- drivers/platform/x86/Kconfig | 14 ++ drivers/platform/x86/Makefile | 3 + drivers/platform/x86/inspur-wmi.c | 210 ++++++++++++++++++++++++++++++ 3 files changed, 227 insertions(+) create mode 100644 drivers/platform/x86/inspur-wmi.c