Hi, Added LED subsys people, please include them in future versions automatically. On Thu, 22 Feb 2024, Mustafa Ekşi wrote: > Signed-off-by: Mustafa Ekşi <mustafa.eskieksi@xxxxxxxxx> > --- > MAINTAINERS | 6 + > drivers/platform/x86/Kconfig | 14 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/casper-wmi.c | 344 ++++++++++++++++++++++++++++++ > 4 files changed, 365 insertions(+) > create mode 100644 drivers/platform/x86/casper-wmi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ed4d386853..d0142a75d2c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4723,6 +4723,12 @@ S: Maintained > W: https://wireless.wiki.kernel.org/en/users/Drivers/carl9170 > F: drivers/net/wireless/ath/carl9170/ > > +CASPER EXCALIBUR WMI DRIVER > +M: Mustafa Ekşi <mustafa.eskieksi@xxxxxxxxx> > +L: platform-driver-x86@xxxxxxxxxxxxxxx > +S: Maintained > +F: drivers/platform/x86/casper-wmi.c > + > CAVIUM I2C DRIVER > M: Robert Richter <rric@xxxxxxxxxx> > S: Odd Fixes > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index bdd302274b9..ebef9c9dfb6 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1127,6 +1127,20 @@ config SEL3350_PLATFORM > To compile this driver as a module, choose M here: the module > will be called sel3350-platform. > > +config CASPER_WMI > + tristate "Casper Excalibur Laptop WMI driver" > + depends on ACPI_WMI > + depends on HWMON > + select NEW_LEDS > + select LEDS_CLASS > + help > + Say Y here if you want to support WMI-based fan speed reporting, > + power management and keyboard backlight support on Casper Excalibur > + Laptops. > + > + To compile this driver as a module, choose M here: the module will > + be called casper-wmi. > + > endif # X86_PLATFORM_DEVICES > > config P2SB > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 1de432e8861..4b527dd44ad 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_MXM_WMI) += mxm-wmi.o > obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT) += nvidia-wmi-ec-backlight.o > obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o > obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o > +obj-$(CONFIG_CASPER_WMI) += casper-wmi.o > > # Acer > obj-$(CONFIG_ACERHDF) += acerhdf.o > diff --git a/drivers/platform/x86/casper-wmi.c b/drivers/platform/x86/casper-wmi.c > new file mode 100644 > index 00000000000..aae08202b19 > --- /dev/null > +++ b/drivers/platform/x86/casper-wmi.c > @@ -0,0 +1,344 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > + > +#include <linux/acpi.h> > +#include <linux/leds.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/wmi.h> > +#include <linux/device.h> > +#include <linux/dev_printk.h> > +#include <linux/hwmon.h> > +#include <linux/sysfs.h> > +#include <linux/types.h> > +#include <linux/dmi.h> > +#include <acpi/acexcep.h> > + > +MODULE_AUTHOR("Mustafa Ekşi <mustafa.eskieksi@xxxxxxxxx>"); > +MODULE_DESCRIPTION("Casper Excalibur Laptop WMI driver"); > +MODULE_LICENSE("GPL"); Put these to the end of file. > +#define CASPER_WMI_GUID "644C5791-B7B0-4123-A90B-E93876E0DAAD" > + > +#define CASPER_READ 0xfa00 > +#define CASPER_WRITE 0xfb00 > +#define CASPER_GET_HARDWAREINFO 0x0200 > +#define CASPER_SET_LED 0x0100 > +#define CASPER_POWERPLAN 0x0300 > + > +#define CASPER_KEYBOARD_LED_1 0x03 > +#define CASPER_KEYBOARD_LED_2 0x04 > +#define CASPER_KEYBOARD_LED_3 0x05 > +#define CASPER_ALL_KEYBOARD_LEDS 0x06 > +#define CASPER_CORNER_LEDS 0x07 > + > +struct casper_wmi_args { > + u16 a0, a1; > + u32 a2, a3, a4, a5, a6, a7, a8; > +}; > + > +static u32 casper_last_color; > +static u8 casper_last_led; > + > +static acpi_status casper_set(struct wmi_device *wdev, u16 a1, u8 led_id, > + u32 data) > +{ > + struct casper_wmi_args wmi_args = { > + .a0 = CASPER_WRITE, > + .a1 = a1, > + .a2 = led_id, > + .a3 = data > + }; > + struct acpi_buffer input = { > + (acpi_size) sizeof(struct casper_wmi_args), > + &wmi_args > + }; > + return wmidev_block_set(wdev, 0, &input); > +} > + > +static ssize_t led_control_show(struct device *dev, struct device_attribute > + *attr, char *buf) > +{ > + return sprintf("%u%08x\n", buf, casper_last_led, > + casper_last_color); Fits one line. Use sysfs_emit(). > +} > + > + > +// input is formatted as "IMARRGGBB", I: led_id, M: mode, A: brightness, ... If you do things like this, please add defines for such "fields" and use FIELD_GET/PREP(). Could LED subsystem folks plese check this the correct way to do RGB control? (I suspect it's not). > +static ssize_t led_control_store(struct device *dev, struct device_attribute > + *attr, const char *buf, size_t count) > +{ > + u64 tmp; > + int ret; > + > + ret = kstrtou64(buf, 16, &tmp); > + > + if (ret) > + return ret; Don't place empty line between function and its error handling. Please go through the entire patch and fix all of them (I won't mark them all). > + > + u8 led_id = (tmp >> (8 * 4))&0xFF; FIELD_GET() + add #include for it. > + > + ret = > + casper_set(to_wmi_device(dev->parent), CASPER_SET_LED, led_id, > + (u32) tmp Don't call variable "tmp"! Please create a local variable for these to_wmi_device(dev->parent) to make this fit one line. > + ); > + if (ACPI_FAILURE(ret)) { > + dev_err(dev, "casper-wmi ACPI status: %d\n", ret); > + return ret; > + } > + if (led_id != CASPER_CORNER_LEDS) { > + casper_last_color = (u32) tmp; > + casper_last_led = led_id; > + } > + return count; > +} > + > +static DEVICE_ATTR_RW(led_control); > + > +static struct attribute *casper_kbd_led_attrs[] = { > + &dev_attr_led_control.attr, > + NULL, > +}; > + > +ATTRIBUTE_GROUPS(casper_kbd_led); > + > +static void set_casper_backlight_brightness(struct led_classdev *led_cdev, > + enum led_brightness brightness) > +{ > + // Setting any of the keyboard leds' brightness sets brightness of all > + acpi_status ret = > + casper_set(to_wmi_device(led_cdev->dev->parent), CASPER_SET_LED, > + CASPER_KEYBOARD_LED_1, > + (casper_last_color & 0xF0FFFFFF) | > + (((u32) brightness) << 24) Use FIELD_PREP(). As you need to split lines, please do the calculations in a local variable beforehand and only then call. > + ); > + > + if (ret != 0) > + dev_err(led_cdev->dev, > + "Couldn't set brightness acpi status: %d\n", ret); > +} > + > +static enum led_brightness get_casper_backlight_brightness(struct led_classdev > + *led_cdev) > +{ > + return (casper_last_color&0x0F000000)>>24; FIELD_GET(). > +} > + > +static struct led_classdev casper_kbd_led = { > + .name = "casper::kbd_backlight", > + .brightness = 0, > + .brightness_set = set_casper_backlight_brightness, > + .brightness_get = get_casper_backlight_brightness, > + .max_brightness = 2, > + .groups = casper_kbd_led_groups, > +}; > + > +static acpi_status casper_query(struct wmi_device *wdev, u16 a1, > + struct casper_wmi_args *out) > +{ > + struct casper_wmi_args wmi_args = { > + .a0 = CASPER_READ, > + .a1 = a1 > + }; > + struct acpi_buffer input = { > + (acpi_size) sizeof(struct casper_wmi_args), > + &wmi_args > + }; > + > + acpi_status ret = wmidev_block_set(wdev, 0, &input); > + > + if (ACPI_FAILURE(ret)) { > + dev_err(&wdev->dev, > + "Could not query acpi status: %u", ret); One line. > + return ret; > + } > + > + union acpi_object *obj = wmidev_block_query(wdev, 0); > + > + if (obj == NULL) { > + dev_err(&wdev->dev, > + "Could not query hardware information"); Ditto. > + return AE_ERROR; > + } > + if (obj->type != ACPI_TYPE_BUFFER) { > + dev_err(&wdev->dev, "Return type is not a buffer"); > + return AE_TYPE; > + } > + > + if (obj->buffer.length != 32) { > + dev_err(&wdev->dev, "Return buffer is not long enough"); > + return AE_ERROR; > + } > + memcpy(out, obj->buffer.pointer, 32); 32 appears at least twice here, add define for it. > + kfree(obj); > + return ret; > +} > + > +static umode_t casper_wmi_hwmon_is_visible(const void *drvdata, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + switch (type) { > + case hwmon_fan: > + return 0444; > + case hwmon_pwm: > + return 0644; > + default: > + return 0; > + } > + return 0; > +} > + > +static int casper_wmi_hwmon_read(struct device *dev, > + enum hwmon_sensor_types type, u32 attr, > + int channel, long *val) > +{ > + struct casper_wmi_args out = { 0 }; > + > + switch (type) { > + case hwmon_fan: > + acpi_status ret = casper_query(to_wmi_device(dev->parent), > + CASPER_GET_HARDWAREINFO, &out); > + > + if (ACPI_FAILURE(ret)) > + return ret; Don't put empty line between the call and its error handling. Move the declaration of the ret variable to function level. > + if (channel == 0) { // CPU fan > + u32 cpu_fanspeed = out.a4; > + > + cpu_fanspeed <<= 8; > + cpu_fanspeed += out.a4 >> 8; Is this byteswapping? Use proper endianness helpers/types when dealing with endianness. > + *val = (long) cpu_fanspeed; > + } else if (channel == 1) { // GPU fan > + u32 gpu_fanspeed = out.a5; > + > + gpu_fanspeed <<= 8; > + gpu_fanspeed += out.a5 >> 8; > + *val = (long) gpu_fanspeed; > + } Should the other channel values return -ENODEV or -EINVAL? > + return 0; > + case hwmon_pwm: > + casper_query(to_wmi_device(dev->parent), CASPER_POWERPLAN, > + &out); > + if (channel == 0) > + *val = (long)out.a2; > + else > + return -EOPNOTSUPP; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > + > + return 0; > +} > + > +static int casper_wmi_hwmon_read_string(struct device *dev, > + enum hwmon_sensor_types type, u32 attr, > + int channel, const char **str) > +{ > + switch (type) { > + case hwmon_fan: > + switch (channel) { > + case 0: > + *str = "cpu_fan_speed"; > + break; > + case 1: > + *str = "gpu_fan_speed"; > + break; > + default: > + return -EOPNOTSUPP; > + } > + break; > + default: > + return -EOPNOTSUPP; > + } > + return 0; > +} > + > +static int casper_wmi_hwmon_write(struct device *dev, > + enum hwmon_sensor_types type, u32 attr, > + int channel, long val) > +{ > + acpi_status ret; > + > + switch (type) { > + case hwmon_pwm: > + if (channel != 0) > + return -EOPNOTSUPP; > + ret = > + casper_set(to_wmi_device(dev->parent), CASPER_POWERPLAN, > + val, 0); > + > + if (ACPI_FAILURE(ret)) { > + dev_err(dev, "Couldn't set power plan, acpi_status: %d", > + ret); > + return -EINVAL; > + } > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static const struct hwmon_ops casper_wmi_hwmon_ops = { > + .is_visible = &casper_wmi_hwmon_is_visible, > + .read = &casper_wmi_hwmon_read, > + .read_string = &casper_wmi_hwmon_read_string, > + .write = &casper_wmi_hwmon_write > +}; > + > +static const struct hwmon_channel_info *const casper_wmi_hwmon_info[] = { > + HWMON_CHANNEL_INFO(fan, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL), > + HWMON_CHANNEL_INFO(pwm, HWMON_PWM_MODE), > + NULL > +}; > + > +static const struct hwmon_chip_info casper_wmi_hwmon_chip_info = { > + .ops = &casper_wmi_hwmon_ops, > + .info = casper_wmi_hwmon_info, > +}; > + > +static int casper_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct device *hwmon_dev; > + > + // All Casper Excalibur Laptops use this GUID > + if (!wmi_has_guid(CASPER_WMI_GUID)) > + return -ENODEV; > + > + hwmon_dev = > + devm_hwmon_device_register_with_info(&wdev->dev, "casper_wmi", wdev, > + &casper_wmi_hwmon_chip_info, > + NULL); > + > + acpi_status result = led_classdev_register(&wdev->dev, &casper_kbd_led); > + > + if (result != 0) > + return -ENODEV; > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > + } Misindented brace. > + > +static void casper_wmi_remove(struct wmi_device *wdev) > +{ > + led_classdev_unregister(&casper_kbd_led); > +} > + > +static const struct wmi_device_id casper_wmi_id_table[] = { > + { CASPER_WMI_GUID, NULL }, > + { } > +}; > + > +static struct wmi_driver casper_wmi_driver = { > + .driver = { > + .name = "casper-wmi", > + }, > + .id_table = casper_wmi_id_table, > + .probe = casper_wmi_probe, > + .remove = &casper_wmi_remove Put comma to the end of this line. > +}; > + > +module_wmi_driver(casper_wmi_driver); > + > +MODULE_DEVICE_TABLE(wmi, casper_wmi_id_table); > -- i.
