Hi 2023. február 14., kedd 14:25 keltezéssel, Nikita Kravets <teackot@xxxxxxxxx> írta: > Add a new driver to allow various MSI laptops' functionalities to be > controlled from userspace. This includes such features as power > profiles (aka shift modes), fan speed, charge thresholds, LEDs, etc. > > This driver contains EC memory configurations for different firmware > versions and exports battery charge thresholds to userspace (note, > that start and end thresholds control the same EC parameter > and are always 10% apart). > > Link: https://github.com/BeardOverflow/msi-ec/ > Discussion: https://github.com/BeardOverflow/msi-ec/pull/13 > Signed-off-by: Nikita Kravets <teackot@xxxxxxxxx> > --- > drivers/platform/x86/Kconfig | 7 + > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/msi-ec.c | 528 ++++++++++++++++++++++++++++++++++ > drivers/platform/x86/msi-ec.h | 119 ++++++++ > 4 files changed, 655 insertions(+) > create mode 100644 drivers/platform/x86/msi-ec.c > create mode 100644 drivers/platform/x86/msi-ec.h > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 5692385e2d26..4534d11f9ca5 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -644,6 +644,13 @@ config THINKPAD_LMI > > source "drivers/platform/x86/intel/Kconfig" > > +config MSI_EC > + tristate "MSI EC Extras" > + depends on ACPI > + help > + This driver allows various MSI laptops' functionalities to be > + controlled from userspace, including battery charge threshold. > + > config MSI_LAPTOP > tristate "MSI Laptop Extras" > depends on ACPI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 1d3d1b02541b..7cc2beca8208 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -71,6 +71,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o > obj-y += intel/ > > # MSI > +obj-$(CONFIG_MSI_EC) += msi-ec.o > obj-$(CONFIG_MSI_LAPTOP) += msi-laptop.o > obj-$(CONFIG_MSI_WMI) += msi-wmi.o > > diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c > new file mode 100644 > index 000000000000..b32106445bf6 > --- /dev/null > +++ b/drivers/platform/x86/msi-ec.c > @@ -0,0 +1,528 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > + > +/* > + * msi-ec: MSI laptops' embedded controller driver. > + * > + * This driver allows various MSI laptops' functionalities to be > + * controlled from userspace. > + * > + * It contains EC memory configurations for different firmware versions > + * and exports battery charge thresholds to userspace. > + * > + * Copyright (C) 2023 Jose Angel Pastrana <japp0005@xxxxxxxxxxxx> > + * Copyright (C) 2023 Aakash Singh <mail@xxxxxxxxxxxxxxx> > + * Copyright (C) 2023 Nikita Kravets <teackot@xxxxxxxxx> > + */ > + > +#include "msi-ec.h" > + > +#include <acpi/battery.h> > +#include <linux/acpi.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/proc_fs.h> > +#include <linux/seq_file.h> > + > +static const char *const SM_ECO_NAME = "eco"; > +static const char *const SM_COMFORT_NAME = "comfort"; > +static const char *const SM_SPORT_NAME = "sport"; > +static const char *const SM_TURBO_NAME = "turbo"; > + > +static const char *ALLOWED_FW_0[] __initdata = { > + "14C1EMS1.101", > + NULL, Usually commas are omitted after sentinel entries. > +}; > + > +static struct msi_ec_conf CONF0 __initdata = { > + .allowed_fw = ALLOWED_FW_0, Alternatively: .allowed_fw = (const char * const []) { "...", "...", NULL }, (although this won't inherit the __initdata attribute as far as I can see) > + .charge_control = { > + .address = 0xef, > + .offset_start = 0x8a, > + .offset_end = 0x80, > + .range_min = 0x8a, > + .range_max = 0xe4, > + }, > + .webcam = { > + .address = 0x2e, > + .block_address = 0x2f, > + .bit = 1, > + }, > + .fn_super_swap = { > + .address = 0xbf, > + .bit = 4, > + }, > + .cooler_boost = { > + .address = 0x98, > + .bit = 7, > + }, > + .shift_mode = { > + .address = 0xf2, > + .modes = { > + { SM_ECO_NAME, 0xc2 }, > + { SM_COMFORT_NAME, 0xc1 }, > + { SM_SPORT_NAME, 0xc0 }, > + }, > + .modes_count = 3, > + }, > + .super_battery = { > + .supported = false, > + }, > + .fan_mode = { > + .address = 0xf4, > + }, > + .cpu = { > + .rt_temp_address = 0x68, > + .rt_fan_speed_address = 0x71, > + .rt_fan_speed_base_min = 0x19, > + .rt_fan_speed_base_max = 0x37, > + .bs_fan_speed_address = 0x89, > + .bs_fan_speed_base_min = 0x00, > + .bs_fan_speed_base_max = 0x0f, > + }, > + .gpu = { > + .rt_temp_address = 0x80, > + .rt_fan_speed_address = 0x89, > + }, > + .leds = { > + .micmute_led_address = 0x2b, > + .mute_led_address = 0x2c, > + .bit = 2, > + }, > + .kbd_bl = { > + .bl_mode_address = 0x2c, // ? > + .bl_modes = { 0x00, 0x08 }, // ? > + .max_mode = 1, // ? > + .bl_state_address = 0xf3, > + .state_base_value = 0x80, > + .max_state = 3, > + }, > +}; > + > +static const char *ALLOWED_FW_1[] __initdata = { > + "17F2EMS1.106", > + NULL, > +}; > + > +static struct msi_ec_conf CONF1 __initdata = { > + .allowed_fw = ALLOWED_FW_1, > + .charge_control = { > + .address = 0xef, > + .offset_start = 0x8a, > + .offset_end = 0x80, > + .range_min = 0x8a, > + .range_max = 0xe4, > + }, > + .webcam = { > + .address = 0x2e, > + .block_address = 0x2f, > + .bit = 1, > + }, > + .fn_super_swap = { > + .address = 0xbf, > + .bit = 4, > + }, > + .cooler_boost = { > + .address = 0x98, > + .bit = 7, > + }, > + .shift_mode = { > + .address = 0xf2, > + .modes = { > + { SM_ECO_NAME, 0xc2 }, > + { SM_COMFORT_NAME, 0xc1 }, > + { SM_SPORT_NAME, 0xc0 }, > + { SM_TURBO_NAME, 0xc4 }, > + }, > + .modes_count = 4, > + }, > + .super_battery = { > + .supported = false, > + }, > + .fan_mode = { > + .address = 0xf4, > + }, > + .cpu = { > + .rt_temp_address = 0x68, > + .rt_fan_speed_address = 0x71, > + .rt_fan_speed_base_min = 0x19, > + .rt_fan_speed_base_max = 0x37, > + .bs_fan_speed_address = 0x89, > + .bs_fan_speed_base_min = 0x00, > + .bs_fan_speed_base_max = 0x0f, > + }, > + .gpu = { > + .rt_temp_address = 0x80, > + .rt_fan_speed_address = 0x89, > + }, > + .leds = { > + .micmute_led_address = 0x2b, > + .mute_led_address = 0x2c, > + .bit = 2, > + }, > + .kbd_bl = { > + .bl_mode_address = 0x2c, // ? > + .bl_modes = { 0x00, 0x08 }, // ? > + .max_mode = 1, // ? > + .bl_state_address = 0xf3, > + .state_base_value = 0x80, > + .max_state = 3, > + }, > +}; > + > +static const char *ALLOWED_FW_2[] __initdata = { > + "1552EMS1.118", > + NULL, > +}; > + > +static struct msi_ec_conf CONF2 __initdata = { > + .allowed_fw = ALLOWED_FW_2, > + .charge_control = { > + .address = 0xd7, > + .offset_start = 0x8a, > + .offset_end = 0x80, > + .range_min = 0x8a, > + .range_max = 0xe4, > + }, > + .webcam = { > + .address = 0x2e, > + .block_address = 0x2f, > + .bit = 1, > + }, > + .fn_super_swap = { > + .address = 0xe8, > + .bit = 4, > + }, > + .cooler_boost = { > + .address = 0x98, > + .bit = 7, > + }, > + .shift_mode = { > + .address = 0xf2, > + .modes = { > + { SM_ECO_NAME, 0xc2 }, > + { SM_COMFORT_NAME, 0xc1 }, > + { SM_SPORT_NAME, 0xc0 }, > + }, > + .modes_count = 3, > + }, > + .super_battery = { > + .supported = true, > + .address = 0xeb, > + .mask = 0x0f, > + }, > + .fan_mode = { > + .address = 0xd4, > + }, > + .cpu = { > + .rt_temp_address = 0x68, > + .rt_fan_speed_address = 0x71, > + .rt_fan_speed_base_min = 0x19, > + .rt_fan_speed_base_max = 0x37, > + .bs_fan_speed_address = 0x89, > + .bs_fan_speed_base_min = 0x00, > + .bs_fan_speed_base_max = 0x0f, > + }, > + .gpu = { > + .rt_temp_address = 0x80, > + .rt_fan_speed_address = 0x89, > + }, > + .leds = { > + .micmute_led_address = 0x2c, > + .mute_led_address = 0x2d, > + .bit = 1, > + }, > + .kbd_bl = { > + .bl_mode_address = 0x2c, // ? > + .bl_modes = { 0x00, 0x08 }, // ? > + .max_mode = 1, // ? > + .bl_state_address = 0xd3, > + .state_base_value = 0x80, > + .max_state = 3, > + }, > +}; > + > +static const char *ALLOWED_FW_3[] __initdata = { > + "1592EMS1.111", > + "E1592IMS.10C", > + NULL, > +}; > + > +static struct msi_ec_conf CONF3 __initdata = { > + .allowed_fw = ALLOWED_FW_3, > + .charge_control = { > + .address = 0xef, > + .offset_start = 0x8a, > + .offset_end = 0x80, > + .range_min = 0x8a, > + .range_max = 0xe4, > + }, > + .webcam = { > + .address = 0x2e, > + .block_address = 0x2f, > + .bit = 1, > + }, > + .fn_super_swap = { > + .address = 0xe8, > + .bit = 4, > + }, > + .cooler_boost = { > + .address = 0x98, > + .bit = 7, > + }, > + .shift_mode = { > + .address = 0xd2, > + .modes = { > + { SM_ECO_NAME, 0xc2 }, > + { SM_COMFORT_NAME, 0xc1 }, > + { SM_SPORT_NAME, 0xc0 }, > + }, > + .modes_count = 3, > + }, > + .super_battery = { > + .supported = true, > + .address = 0xeb, > + .mask = 0x0f, > + }, > + .fan_mode = { > + .address = 0xd4, > + }, > + .cpu = { > + .rt_temp_address = 0x68, > + .rt_fan_speed_address = 0xc9, > + .rt_fan_speed_base_min = 0x19, > + .rt_fan_speed_base_max = 0x37, > + .bs_fan_speed_address = 0x89, // ? > + .bs_fan_speed_base_min = 0x00, > + .bs_fan_speed_base_max = 0x0f, > + }, > + .gpu = { > + .rt_temp_address = 0x80, > + .rt_fan_speed_address = 0x89, > + }, > + .leds = { > + .micmute_led_address = 0x2b, > + .mute_led_address = 0x2c, > + .bit = 1, > + }, > + .kbd_bl = { > + .bl_mode_address = 0x2c, // ? > + .bl_modes = { 0x00, 0x08 }, // ? > + .max_mode = 1, // ? > + .bl_state_address = 0xd3, > + .state_base_value = 0x80, > + .max_state = 3, > + }, > +}; > + > +static struct msi_ec_conf *CONFIGURATIONS[] __initdata = { > + &CONF0, > + &CONF1, > + &CONF2, > + &CONF3, > + NULL, > +}; > + > +static struct msi_ec_conf conf; // current configuration > + > +// ============================================================ // > +// Helper functions > +// ============================================================ // > + > +static int ec_read_seq(u8 addr, u8 *buf, int len) > +{ > + int result; > + u8 i; > + for (i = 0; i < len; i++) { It's a small thing, but I would make `i` and `len` be the same type. > + result = ec_read(addr + i, buf + i); > + if (result < 0) > + return result; > + } > + return 0; > +} > + > +static int ec_get_firmware_version(u8 buf[MSI_EC_FW_VERSION_LENGTH + 1]) > +{ > + int result; > + > + memset(buf, 0, MSI_EC_FW_VERSION_LENGTH + 1); > + result = ec_read_seq(MSI_EC_FW_VERSION_ADDRESS, > + buf, > + MSI_EC_FW_VERSION_LENGTH); > + if (result < 0) > + return result; > + > + return MSI_EC_FW_VERSION_LENGTH + 1; > +} > + > +// ============================================================ // > +// Sysfs power_supply subsystem > +// ============================================================ // > + > +static ssize_t charge_control_threshold_show(u8 offset, struct device *device, > + struct device_attribute *attr, > + char *buf) > +{ > + u8 rdata; > + int result; > + > + result = ec_read(conf.charge_control.address, &rdata); > + if (result < 0) > + return result; > + > + return sprintf(buf, "%i\n", rdata - offset); Please prefer `sysfs_emit()`. > +} > + > +static ssize_t charge_control_threshold_store(u8 offset, struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + u8 wdata; > + int result; > + > + result = kstrtou8(buf, 10, &wdata); > + if (result < 0) > + return result; > + > + wdata += offset; > + if (wdata < conf.charge_control.range_min || > + wdata > conf.charge_control.range_max) > + return -EINVAL; > + > + result = ec_write(conf.charge_control.address, wdata); > + if (result < 0) > + return result; > + > + return count; > +} > + > +static ssize_t > +charge_control_start_threshold_show(struct device *device, > + struct device_attribute *attr, char *buf) > +{ > + return charge_control_threshold_show(conf.charge_control.offset_start, > + device, attr, buf); > +} > + > +static ssize_t > +charge_control_start_threshold_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + return charge_control_threshold_store(conf.charge_control.offset_start, > + dev, attr, buf, count); > +} > + > +static ssize_t charge_control_end_threshold_show(struct device *device, > + struct device_attribute *attr, > + char *buf) > +{ > + return charge_control_threshold_show(conf.charge_control.offset_end, > + device, attr, buf); > +} > + > +static ssize_t charge_control_end_threshold_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + return charge_control_threshold_store(conf.charge_control.offset_end, > + dev, attr, buf, count); > +} > + > +static DEVICE_ATTR_RW(charge_control_start_threshold); > +static DEVICE_ATTR_RW(charge_control_end_threshold); > + > +static struct attribute *msi_battery_attrs[] = { > + &dev_attr_charge_control_start_threshold.attr, > + &dev_attr_charge_control_end_threshold.attr, > + NULL, > +}; > + > +ATTRIBUTE_GROUPS(msi_battery); > + > +static int msi_battery_add(struct power_supply *battery, > + struct acpi_battery_hook *hook) > +{ > + if (device_add_groups(&battery->dev, msi_battery_groups)) > + return -ENODEV; > + return 0; Why not return device_add_groups(...); ? Furthermore, is it possible that there are two or more batteries? > +} > + > +static int msi_battery_remove(struct power_supply *battery, > + struct acpi_battery_hook *hook) > +{ > + device_remove_groups(&battery->dev, msi_battery_groups); > + return 0; > +} > + > +static struct acpi_battery_hook battery_hook = { > + .add_battery = msi_battery_add, > + .remove_battery = msi_battery_remove, > + .name = MSI_EC_DRIVER_NAME, > +}; > + > +// ============================================================ // > +// Module load/unload > +// ============================================================ // > + > +static int __init load_configuration(void) > +{ > + int result; > + > + // get firmware version > + u8 fw_version[MSI_EC_FW_VERSION_LENGTH + 1]; > + result = ec_get_firmware_version(fw_version); > + if (result < 0) { > + return result; > + } > + > + // load the suitable configuration, if exists > + for (int i = 0; CONFIGURATIONS[i]; i++) { > + for (int j = 0; CONFIGURATIONS[i]->allowed_fw[j]; j++) { > + if (strcmp(CONFIGURATIONS[i]->allowed_fw[j], fw_version) == 0) { > + memcpy(&conf, CONFIGURATIONS[i], sizeof(struct msi_ec_conf)); > + conf.allowed_fw = NULL; > + return 0; > + } > + } > + } Have you checked if `match_string()` from string.h works here? > + > + pr_err("Your firmware version is not supported!\n"); > + return -EOPNOTSUPP; > +} > + > +static int __init msi_ec_init(void) > +{ > + int result; > + > + if (acpi_disabled) { I am wondering how useful this check really is. > + pr_err("Unable to init because ACPI needs to be enabled first!\n"); > + return -ENODEV; > + } > + > + result = load_configuration(); This will start poking the embedded controller when the module is loaded, regardless of the platform. I am not sure that is desirable. > + if (result < 0) > + return result; > + > + battery_hook_register(&battery_hook); > + > + pr_info("msi-ec: module_init\n"); Instead of manually prefixing the messages, I suggest you do #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt before including any headers and then you can drop the "msi-ec: " from the strings. > + return 0; > +} > + > +static void __exit msi_ec_exit(void) > +{ > + battery_hook_unregister(&battery_hook); > + > + pr_info("msi-ec: module_exit\n"); > +} > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Jose Angel Pastrana <japp0005@xxxxxxxxxxxx>"); > +MODULE_AUTHOR("Aakash Singh <mail@xxxxxxxxxxxxxxx>"); > +MODULE_AUTHOR("Nikita Kravets <teackot@xxxxxxxxx>"); > +MODULE_DESCRIPTION("MSI Embedded Controller"); > + > +module_init(msi_ec_init); > +module_exit(msi_ec_exit); > diff --git a/drivers/platform/x86/msi-ec.h b/drivers/platform/x86/msi-ec.h > new file mode 100644 > index 000000000000..4de6bba363ff > --- /dev/null > +++ b/drivers/platform/x86/msi-ec.h > @@ -0,0 +1,119 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > + > +/* > + * msi-ec: MSI laptops' embedded controller driver. > + * > + * Copyright (C) 2023 Jose Angel Pastrana <japp0005@xxxxxxxxxxxx> > + * Copyright (C) 2023 Aakash Singh <mail@xxxxxxxxxxxxxxx> > + * Copyright (C) 2023 Nikita Kravets <teackot@xxxxxxxxx> > + */ > + > +#ifndef _MSI_EC_H_ > +#define _MSI_EC_H_ > + > +#include <linux/types.h> > + > +#define MSI_EC_DRIVER_NAME "msi-ec" > + > +// Firmware info addresses are universal > +#define MSI_EC_FW_VERSION_ADDRESS 0xa0 > +#define MSI_EC_FW_DATE_ADDRESS 0xac > +#define MSI_EC_FW_TIME_ADDRESS 0xb4 > +#define MSI_EC_FW_VERSION_LENGTH 12 > +#define MSI_EC_FW_DATE_LENGTH 8 > +#define MSI_EC_FW_TIME_LENGTH 8 > + > +struct msi_ec_charge_control_conf { > + int address; > + int offset_start; > + int offset_end; > + int range_min; > + int range_max; > +}; > + > +struct msi_ec_webcam_conf { > + int address; > + int block_address; > + int bit; > +}; > + > +struct msi_ec_fn_super_swap_conf { > + int address; > + int bit; > +}; > + > +struct msi_ec_cooler_boost_conf { > + int address; > + int bit; > +}; > + > +struct msi_ec_mode { > + const char *name; > + int value; > +}; > + > +struct msi_ec_shift_mode_conf { > + int address; > + struct msi_ec_mode modes[5]; // fixed size for easier hard coding > + int modes_count; > +}; > + > +struct msi_ec_super_battery_conf { > + bool supported; > + int address; > + int mask; > +}; > + > +struct msi_ec_fan_mode_conf { > + int address; > +}; > + > +struct msi_ec_cpu_conf { > + int rt_temp_address; > + int rt_fan_speed_address; // realtime > + int rt_fan_speed_base_min; > + int rt_fan_speed_base_max; > + int bs_fan_speed_address; // basic > + int bs_fan_speed_base_min; > + int bs_fan_speed_base_max; > +}; > + > +struct msi_ec_gpu_conf { > + int rt_temp_address; > + int rt_fan_speed_address; // realtime > +}; > + > +struct msi_ec_led_conf { > + int micmute_led_address; > + int mute_led_address; > + int bit; > +}; > + > +#define MSI_EC_KBD_BL_STATE_MASK 0x3 > +struct msi_ec_kbd_bl_conf { > + int bl_mode_address; > + int bl_modes[2]; > + int max_mode; > + > + int bl_state_address; > + int state_base_value; > + int max_state; > +}; > + > +struct msi_ec_conf { > + const char **allowed_fw; > + > + struct msi_ec_charge_control_conf charge_control; > + struct msi_ec_webcam_conf webcam; > + struct msi_ec_fn_super_swap_conf fn_super_swap; > + struct msi_ec_cooler_boost_conf cooler_boost; > + struct msi_ec_shift_mode_conf shift_mode; > + struct msi_ec_super_battery_conf super_battery; > + struct msi_ec_fan_mode_conf fan_mode; > + struct msi_ec_cpu_conf cpu; > + struct msi_ec_gpu_conf gpu; > + struct msi_ec_led_conf leds; > + struct msi_ec_kbd_bl_conf kbd_bl; > +}; > + > +#endif // _MSI_EC_H_ > -- > 2.39.1 > Regards, Barnabás Pőcze
