The fw_attributes_class provides a much cleaner interface to all of the
attributes introduced to asus-wmi. This patch moves all of these extra
attributes over to fw_attributes_class, and shifts the bulk of these
definitions to a new kernel module to reduce the clutter of asus-wmi
with the intention of deprecating the asus-wmi attributes in future.
The work applies only to WMI methods which don't have a clearly defined
place within the sysfs and as a result ended up lumped together in
/sys/devices/platform/asus-nb-wmi/ with no standard API.
Where possible the fw attrs now implement defaults, min, max, scalar,
choices, etc. As en example dgpu_disable becomes:
/sys/class/firmware-attributes/asus-armoury/attributes/dgpu_disable/
├── current_value
├── display_name
├── possible_values
└── type
as do other attributes.
The ppt_* based attributes are removed in this initial patch as the
implementation is somewhat broken due to the WMI methods requiring a
set of limits on the values accepted (which is not provided by WMI).
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/platform/x86/Kconfig | 12 +
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/asus-armoury.c | 593 +++++++++++++++++++++
drivers/platform/x86/asus-armoury.h | 146 +++++
drivers/platform/x86/asus-wmi.c | 4 -
include/linux/platform_data/x86/asus-wmi.h | 3 +
6 files changed, 755 insertions(+), 4 deletions(-)
create mode 100644 drivers/platform/x86/asus-armoury.c
create mode 100644 drivers/platform/x86/asus-armoury.h
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 3875abba5a79..80ec8b45022d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -265,6 +265,18 @@ config ASUS_WIRELESS
If you choose to compile this driver as a module the module will be
called asus-wireless.
+config ASUS_ARMOURY
+ tristate "ASUS Armoury driver"
+ depends on ASUS_WMI
+ select FW_ATTR_CLASS
+ help
+ Say Y here if you have a WMI aware Asus machine and would like to use the
+ firmware_attributes API to control various settings typically exposed in
+ the ASUS Armoury Crate application available on Windows.
+
+ To compile this driver as a module, choose M here: the module will
+ be called asus-armoury.
+
config ASUS_WMI
tristate "ASUS WMI Driver"
depends on ACPI_WMI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index e1b142947067..fe3e7e7dede8 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_APPLE_GMUX) += apple-gmux.o
# ASUS
obj-$(CONFIG_ASUS_LAPTOP) += asus-laptop.o
obj-$(CONFIG_ASUS_WIRELESS) += asus-wireless.o
+obj-$(CONFIG_ASUS_ARMOURY) += asus-armoury.o
obj-$(CONFIG_ASUS_WMI) += asus-wmi.o
obj-$(CONFIG_ASUS_NB_WMI) += asus-nb-wmi.o
obj-$(CONFIG_ASUS_TF103C_DOCK) += asus-tf103c-dock.o
diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
new file mode 100644
index 000000000000..0085f1b112ab
--- /dev/null
+++ b/drivers/platform/x86/asus-armoury.c
@@ -0,0 +1,593 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Asus Armoury (WMI) attributes driver. This driver uses the fw_attributes
+ * class to expose the various WMI functions that many gaming and some
+ * non-gaming ASUS laptops have available.
+ * These typically don't fit anywhere else in the sysfs such as under LED class,
+ * hwmon or other, and are set in Windows using the ASUS Armoury Crate tool.
+ *
+ * Copyright(C) 2010 Intel Corporation.
+ * Copyright(C) 2024-2024 Luke Jones <luke@ljones.dev>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/dmi.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/kmod.h>
+#include <linux/kobject.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_data/x86/asus-wmi.h>
+#include <linux/types.h>
+#include <linux/acpi.h>
+
+#include "asus-armoury.h"
+#include "firmware_attributes_class.h"
+
+#define ASUS_NB_WMI_EVENT_GUID "0B3CBB35-E3C2-45ED-91C2-4C5A6D195D1C"
+
+#define ASUS_MINI_LED_MODE_MASK 0x03
+/* Standard modes for devices with only on/off */
+#define ASUS_MINI_LED_OFF 0x00
+#define ASUS_MINI_LED_ON 0x01
+/* Like "on" but the effect is more vibrant or brighter */
+#define ASUS_MINI_LED_STRONG_MODE 0x02
+/* New modes for devices with 3 mini-led mode types */
+#define ASUS_MINI_LED_2024_WEAK 0x00
+#define ASUS_MINI_LED_2024_STRONG 0x01
+#define ASUS_MINI_LED_2024_OFF 0x02
+
+/* Default limits for tunables available on ASUS ROG laptops */
+#define NVIDIA_BOOST_MIN 5
+#define NVIDIA_BOOST_MAX 25
+#define NVIDIA_TEMP_MIN 75
+#define NVIDIA_TEMP_MAX 87
+#define PPT_CPU_LIMIT_MIN 5
+#define PPT_CPU_LIMIT_MAX 150
+#define PPT_CPU_LIMIT_DEFAULT 80
+#define PPT_PLATFORM_MIN 5
+#define PPT_PLATFORM_MAX 100
+#define PPT_PLATFORM_DEFAULT 80
+
+static const struct class *fw_attr_class;
+
+struct asus_armoury_priv {
+ struct device *fw_attr_dev;
+ struct kset *fw_attr_kset;
+
+ u32 mini_led_dev_id;
+ u32 gpu_mux_dev_id;
+
+ struct mutex mutex;
+};
+
+static struct asus_armoury_priv asus_armoury = { .mutex = __MUTEX_INITIALIZER(
+ asus_armoury.mutex) };
+
+struct fw_attrs_group {
+ bool pending_reboot;
+};
+
+static struct fw_attrs_group fw_attrs = {
+ .pending_reboot = false,
+};
+
+struct asus_attr_group {
+ const struct attribute_group *attr_group;
+ u32 wmi_devid;
+};
+
+static bool asus_wmi_is_present(u32 dev_id)
+{
+ u32 retval;
+ int status;
+
+ status = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, dev_id, 0, &retval);
+ pr_debug("%s called (0x%08x), retval: 0x%08x\n", __func__, dev_id, retval);
+
+ return status == 0 && (retval & ASUS_WMI_DSTS_PRESENCE_BIT);
+}
+
+static void asus_set_reboot_and_signal_event(void)
+{
+ fw_attrs.pending_reboot = true;
+ kobject_uevent(&asus_armoury.fw_attr_dev->kobj, KOBJ_CHANGE);
+}
+
+static ssize_t pending_reboot_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%d\n", fw_attrs.pending_reboot);
+}
+
+static struct kobj_attribute pending_reboot = __ATTR_RO(pending_reboot);
+
+static bool asus_bios_requires_reboot(struct kobj_attribute *attr)
+{
+ return !strcmp(attr->attr.name, "gpu_mux_mode");
+}
+
+/**
+ * attr_int_store() - Generic store function for use with most WMI functions.
+ * @kobj: Pointer to the driver object.
+ * @kobj_attribute: Pointer to the attribute calling this function.
+ * @buf: The buffer to read from, this is parsed to `int` type.
+ * @count: Required by sysfs attribute macros, pass in from the callee attr.
+ * @min: Minimum accepted value. Below this returns -EINVAL.
+ * @max: Maximum accepted value. Above this returns -EINVAL.
+ * @store_value: Pointer to where the parsed value should be stored.
+ * @wmi_dev: The WMI function ID to use.
+ *
+ * The WMI functions available on most ASUS laptops return a 1 as "success", and
+ * a 0 as failed. However some functions can return n > 1 for additional errors.
+ * The function currently treats all values which are not 1 as errors, ignoring
+ * the possible differences in WMI error returns.
+ *
+ * Returns: Either count, or an error.
+ */
+static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf,
+ size_t count, u32 min, u32 max, u32 *store_value, u32 wmi_dev)
+{
+ u32 result, value;
+ int err;
+
+ err = kstrtouint(buf, 10, &value);
+ if (err)
+ return err;
+
+ if (value < min || value > max)
+ return -EINVAL;
+
+ mutex_lock(&asus_armoury.mutex);
+ err = asus_wmi_set_devstate(wmi_dev, value, &result);
+ mutex_unlock(&asus_armoury.mutex);
+ if (err) {
+ pr_err("Failed to set %s: %d\n", attr->attr.name, err);
+ return err;
+ }
+
+ if (result != 1) {
+ pr_err("Failed to set %s (result): 0x%x\n", attr->attr.name, result);
+ return -EIO;
+ }
+
+ if (store_value != NULL)
+ *store_value = value;
+ sysfs_notify(kobj, NULL, attr->attr.name);
+
+ if (asus_bios_requires_reboot(attr))
+ asus_set_reboot_and_signal_event();
+
+ return count;
+}
+
+/* Mini-LED mode **************************************************************/
+static ssize_t mini_led_mode_current_value_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ u32 value;
+ int err;
+
+ err = asus_wmi_get_devstate_dsts(asus_armoury.mini_led_dev_id, &value);
+ if (err)
+ return err;
+
+ value &= ASUS_MINI_LED_MODE_MASK;
+
+ /*
+ * Remap the mode values to match previous generation mini-LED. The last gen
+ * WMI 0 == off, while on this version WMI 2 == off (flipped).
+ */
+ if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
+ switch (value) {
+ case ASUS_MINI_LED_2024_WEAK:
+ value = ASUS_MINI_LED_ON;
+ break;
+ case ASUS_MINI_LED_2024_STRONG:
+ value = ASUS_MINI_LED_STRONG_MODE;
+ break;
+ case ASUS_MINI_LED_2024_OFF:
+ value = ASUS_MINI_LED_OFF;
+ break;
+ }
+ }
+
+ return sysfs_emit(buf, "%u\n", value);
+}
+
+static ssize_t mini_led_mode_current_value_store(struct kobject *kobj,
+ struct kobj_attribute *attr, const char *buf,
+ size_t count)
+{
+ int result, err;
+ u32 mode;
+
+ err = kstrtou32(buf, 10, &mode);
+ if (err)
+ return err;
+
+ if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE &&
+ mode > ASUS_MINI_LED_ON)
+ return -EINVAL;
+ if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2 &&
+ mode > ASUS_MINI_LED_STRONG_MODE)
+ return -EINVAL;
+
+ /*
+ * Remap the mode values so expected behaviour is the same as the last
+ * generation of mini-LED with 0 == off, 1 == on.
+ */
+ if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
+ switch (mode) {
+ case ASUS_MINI_LED_OFF:
+ mode = ASUS_MINI_LED_2024_OFF;
+ break;
+ case ASUS_MINI_LED_ON:
+ mode = ASUS_MINI_LED_2024_WEAK;
+ break;
+ case ASUS_MINI_LED_STRONG_MODE:
+ mode = ASUS_MINI_LED_2024_STRONG;
+ break;
+ }
+ }
+
+ mutex_lock(&asus_armoury.mutex);
+ err = asus_wmi_set_devstate(asus_armoury.mini_led_dev_id, mode, &result);
+ mutex_unlock(&asus_armoury.mutex);
+ if (err) {
+ pr_warn("Failed to set mini-LED: %d\n", err);
+ return err;
+ }
+
+ if (result != 1) {
+ pr_warn("Failed to set mini-LED mode (result): 0x%x\n", result);
+ return -EIO;
+ }
+
+ sysfs_notify(kobj, NULL, attr->attr.name);
+
+ return count;
+}
+
+static ssize_t mini_led_mode_possible_values_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ switch (asus_armoury.mini_led_dev_id) {
+ case ASUS_WMI_DEVID_MINI_LED_MODE:
+ return sysfs_emit(buf, "0;1\n");
+ case ASUS_WMI_DEVID_MINI_LED_MODE2:
+ return sysfs_emit(buf, "0;1;2\n");
+ }
+
+ return sysfs_emit(buf, "0\n");
+}
+
+ATTR_GROUP_ENUM_CUSTOM(mini_led_mode, "mini_led_mode", "Set the mini-LED backlight mode");
+
+static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
+ struct kobj_attribute *attr, const char *buf,
+ size_t count)
+{
+ int result, err;
+ u32 optimus;
+
+ err = kstrtou32(buf, 10, &optimus);
+ if (err)
+ return err;
+
+ if (optimus > 1)
+ return -EINVAL;
+
+ if (asus_wmi_is_present(ASUS_WMI_DEVID_DGPU)) {
+ err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_DGPU, &result);
+ if (err)
+ return err;
+ if (result && !optimus) {
+ err = -ENODEV;
+ pr_warn("Can not switch MUX to dGPU mode when dGPU is disabled: %02X %02X %d\n",
+ result, optimus, err);
+ return err;
+ }
+ }
+
+ if (asus_wmi_is_present(ASUS_WMI_DEVID_EGPU)) {
+ err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU, &result);
+ if (err)
+ return err;
+ if (result && !optimus) {
+ err = -ENODEV;
+ pr_warn("Can not switch MUX to dGPU mode when eGPU is enabled: %d\n",
+ err);
+ return err;
+ }
+ }
+
+ mutex_lock(&asus_armoury.mutex);
+ err = asus_wmi_set_devstate(asus_armoury.gpu_mux_dev_id, optimus, &result);
+ mutex_unlock(&asus_armoury.mutex);
+ if (err) {
+ pr_err("Failed to set GPU MUX mode: %d\n", err);
+ return err;
+ }
+ /* !1 is considered a fail by ASUS */
+ if (result != 1) {
+ pr_warn("Failed to set GPU MUX mode (result): 0x%x\n", result);
+ return -EIO;
+ }
+
+ sysfs_notify(kobj, NULL, attr->attr.name);
+ asus_set_reboot_and_signal_event();
+
+ return count;
+}
+WMI_SHOW_INT(gpu_mux_mode_current_value, "%d\n", asus_armoury.gpu_mux_dev_id);
+ATTR_GROUP_BOOL_CUSTOM(gpu_mux_mode, "gpu_mux_mode", "Set the GPU display MUX mode");
+
+/*
+ * A user may be required to store the value twice, typical store first, then
+ * rescan PCI bus to activate power, then store a second time to save correctly.
+ * The reason for this is that an extra code path in the ACPI is enabled when
+ * the device and bus are powered.
+ */
+static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
+ struct kobj_attribute *attr, const char *buf,
+ size_t count)
+{
+ int result, err;
+ u32 disable;
+
+ err = kstrtou32(buf, 10, &disable);
+ if (err)
+ return err;
+
+ if (disable > 1)
+ return -EINVAL;
+
+ if (asus_armoury.gpu_mux_dev_id) {
+ err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
+ if (err)
+ return err;
+ if (!result && disable) {
+ err = -ENODEV;
+ pr_warn("Can not disable dGPU when the MUX is in dGPU mode: %d\n", err);
+ return err;
+ }
+ }
+
+ mutex_lock(&asus_armoury.mutex);
+ err = asus_wmi_set_devstate(ASUS_WMI_DEVID_DGPU, disable, &result);
+ mutex_unlock(&asus_armoury.mutex);
+ if (err) {
+ pr_warn("Failed to set dGPU disable: %d\n", err);
+ return err;
+ }
+
+ if (result != 1) {
+ pr_warn("Failed to set dGPU disable (result): 0x%x\n", result);
+ return -EIO;
+ }
+
+ sysfs_notify(kobj, NULL, attr->attr.name);
+
+ return count;
+}
+WMI_SHOW_INT(dgpu_disable_current_value, "%d\n", ASUS_WMI_DEVID_DGPU);
+ATTR_GROUP_BOOL_CUSTOM(dgpu_disable, "dgpu_disable", "Disable the dGPU");
+
+/* The ACPI call to enable the eGPU also disables the internal dGPU */
+static ssize_t egpu_enable_current_value_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ int result, err;
+ u32 enable;
+
+ err = kstrtou32(buf, 10, &enable);
+ if (err)
+ return err;
+
+ if (enable > 1)
+ return -EINVAL;
+
+ err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU_CONNECTED, &result);
+ if (err) {
+ pr_warn("Failed to get eGPU connection status: %d\n", err);
+ return err;
+ }
+
+ if (asus_armoury.gpu_mux_dev_id) {
+ err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
+ if (err) {
+ pr_warn("Failed to get GPU MUX status: %d\n", result);
+ return result;
+ }
+ if (!result && enable) {
+ err = -ENODEV;
+ pr_warn("Can not enable eGPU when the MUX is in dGPU mode: %d\n", err);
+ return err;
+ }
+ }
+
+ mutex_lock(&asus_armoury.mutex);
+ err = asus_wmi_set_devstate(ASUS_WMI_DEVID_EGPU, enable, &result);
+ mutex_unlock(&asus_armoury.mutex);
+ if (err) {
+ pr_warn("Failed to set eGPU state: %d\n", err);
+ return err;
+ }
+
+ if (result != 1) {
+ pr_warn("Failed to set eGPU state (retval): 0x%x\n", result);
+ return -EIO;
+ }
+
+ sysfs_notify(kobj, NULL, attr->attr.name);
+
+ return count;
+}
+WMI_SHOW_INT(egpu_enable_current_value, "%d\n", ASUS_WMI_DEVID_EGPU);
+ATTR_GROUP_BOOL_CUSTOM(egpu_enable, "egpu_enable", "Enable the eGPU (also disables dGPU)");
+
+/* Simple attribute creation */
+ATTR_GROUP_ENUM_INT_RO(charge_mode, "charge_mode", ASUS_WMI_DEVID_CHARGE_MODE, "0;1;2",
+ "Show the current mode of charging");
+
+ATTR_GROUP_BOOL_RW(boot_sound, "boot_sound", ASUS_WMI_DEVID_BOOT_SOUND,
+ "Set the boot POST sound");
+ATTR_GROUP_BOOL_RW(mcu_powersave, "mcu_powersave", ASUS_WMI_DEVID_MCU_POWERSAVE,
+ "Set MCU powersaving mode");
+ATTR_GROUP_BOOL_RW(panel_od, "panel_overdrive", ASUS_WMI_DEVID_PANEL_OD,
+ "Set the panel refresh overdrive");
+ATTR_GROUP_BOOL_RO(egpu_connected, "egpu_connected", ASUS_WMI_DEVID_EGPU_CONNECTED,
+ "Show the eGPU connection status");
+
+/* If an attribute does not require any special case handling add it here */
+static const struct asus_attr_group armoury_attr_groups[] = {
+ { &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
+ { &egpu_enable_attr_group, ASUS_WMI_DEVID_EGPU },
+ { &dgpu_disable_attr_group, ASUS_WMI_DEVID_DGPU },
+
+ { &charge_mode_attr_group, ASUS_WMI_DEVID_CHARGE_MODE },
+ { &boot_sound_attr_group, ASUS_WMI_DEVID_BOOT_SOUND },
+ { &mcu_powersave_attr_group, ASUS_WMI_DEVID_MCU_POWERSAVE },
+ { &panel_od_attr_group, ASUS_WMI_DEVID_PANEL_OD },
+};
+
+static int asus_fw_attr_add(void)
+{
+ int err, i;
+
+ err = fw_attributes_class_get(&fw_attr_class);
+ if (err)
+ return err;
+
+ asus_armoury.fw_attr_dev =
+ device_create(fw_attr_class, NULL, MKDEV(0, 0), NULL, "%s", DRIVER_NAME);
+
+ if (IS_ERR(asus_armoury.fw_attr_dev)) {
+ err = PTR_ERR(asus_armoury.fw_attr_dev);
+ goto fail_class_get;
+ }
+
+ asus_armoury.fw_attr_kset =
+ kset_create_and_add("attributes", NULL, &asus_armoury.fw_attr_dev->kobj);
+ if (!asus_armoury.fw_attr_kset) {
+ err = -ENOMEM;
+ goto err_destroy_classdev;
+ }
+
+ err = sysfs_create_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
+ if (err) {
+ pr_err("Failed to create sysfs level attributes\n");
+ goto err_destroy_kset;
+ }
+
+ asus_armoury.mini_led_dev_id = 0;
+ if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE)) {
+ asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
+ err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
+ } else if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE2)) {
+ asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE2;
+ err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
+ }
+ if (err) {
+ pr_err("Failed to create sysfs-group for mini_led\n");
+ goto err_remove_file;
+ }
+
+ asus_armoury.gpu_mux_dev_id = 0;
+ if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX)) {
+ asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX;
+ err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
+ } else if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX_VIVO)) {
+ asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX_VIVO;
+ err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
+ }
+ if (err) {
+ pr_err("Failed to create sysfs-group for gpu_mux\n");
+ goto err_remove_mini_led_group;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
+ if (!asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
+ continue;
+
+ err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
+ armoury_attr_groups[i].attr_group);
+ if (err) {
+ pr_err("Failed to create sysfs-group for %s\n",
+ armoury_attr_groups[i].attr_group->name);
+ goto err_remove_groups;
+ } else {
+ pr_debug("Created sysfs-group for %s\n",
+ armoury_attr_groups[i].attr_group->name);
+ }
+ }
+
+ return 0;
+
+err_remove_groups:
+ while (--i >= 0) {
+ if (asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
+ sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, armoury_attr_groups[i].attr_group);
+ }
+ sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
+err_remove_mini_led_group:
+ sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
+err_remove_file:
+ sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
+err_destroy_kset:
+ kset_unregister(asus_armoury.fw_attr_kset);
+err_destroy_classdev:
+ device_destroy(fw_attr_class, MKDEV(0, 0));
+fail_class_get:
+ fw_attributes_class_put();
+ return err;
+}
+
+/* Init / exit ****************************************************************/
+
+static int __init asus_fw_init(void)
+{
+ char *wmi_uid;
+ int err;
+
+ wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
+ if (!wmi_uid)
+ return -ENODEV;
+
+ /*
+ * if equal to "ASUSWMI" then it's DCTS that can't be used for this
+ * driver, DSTS is required.
+ */
+ if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI))
+ return -ENODEV;
+
+ err = asus_fw_attr_add();
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static void __exit asus_fw_exit(void)
+{
+ mutex_lock(&asus_armoury.mutex);
+
+ sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
+ kset_unregister(asus_armoury.fw_attr_kset);
+ device_destroy(fw_attr_class, MKDEV(0, 0));
+ fw_attributes_class_put();
+
+ mutex_unlock(&asus_armoury.mutex);
+}
+
+module_init(asus_fw_init);
+module_exit(asus_fw_exit);
+
+MODULE_IMPORT_NS(ASUS_WMI);
+MODULE_AUTHOR("Luke Jones <luke@ljones.dev>");
+MODULE_DESCRIPTION("ASUS BIOS Configuration Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("wmi:" ASUS_NB_WMI_EVENT_GUID);
diff --git a/drivers/platform/x86/asus-armoury.h b/drivers/platform/x86/asus-armoury.h
new file mode 100644
index 000000000000..4d0dd34c52aa
--- /dev/null
+++ b/drivers/platform/x86/asus-armoury.h
@@ -0,0 +1,146 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Definitions for kernel modules using asus-armoury driver
+ *
+ * Copyright (c) 2024 Luke Jones <luke@ljones.dev>
+ */
+
+#ifndef _ASUS_ARMOURY_H_
+#define _ASUS_ARMOURY_H_
+
+#include <linux/types.h>
+#include <linux/platform_device.h>
+
+#define DRIVER_NAME "asus-armoury"
+
+static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count, u32 min, u32 max,
+ u32 *store_value, u32 wmi_dev);
+
+static ssize_t enum_type_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "enumeration\n");
+}
+
+#define __ASUS_ATTR_RO(_func, _name) \
+ { \
+ .attr = { .name = __stringify(_name), .mode = 0444 }, \
+ .show = _func##_##_name##_show, \
+ }
+
+#define __ASUS_ATTR_RO_AS(_name, _show) \
+ { \
+ .attr = { .name = __stringify(_name), .mode = 0444 }, \
+ .show = _show, \
+ }
+
+#define __ASUS_ATTR_RW(_func, _name) \
+ __ATTR(_name, 0644, _func##_##_name##_show, _func##_##_name##_store)
+
+#define __WMI_STORE_INT(_attr, _min, _max, _wmi) \
+ static ssize_t _attr##_store(struct kobject *kobj, \
+ struct kobj_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ return attr_int_store(kobj, attr, buf, count, _min, _max, \
+ NULL, _wmi); \
+ }
+
+#define WMI_SHOW_INT(_attr, _fmt, _wmi) \
+ static ssize_t _attr##_show(struct kobject *kobj, \
+ struct kobj_attribute *attr, char *buf) \
+ { \
+ u32 result; \
+ int err; \
+ err = asus_wmi_get_devstate_dsts(_wmi, &result); \
+ if (err) \
+ return err; \
+ return sysfs_emit(buf, _fmt, \
+ result & ~ASUS_WMI_DSTS_PRESENCE_BIT); \
+ }
+
+/* Create functions and attributes for use in other macros or on their own */
+
+#define __ATTR_CURRENT_INT_RO(_attr, _wmi) \
+ WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
+ static struct kobj_attribute attr_##_attr##_current_value = \
+ __ASUS_ATTR_RO(_attr, current_value)
+
+#define __ATTR_CURRENT_INT_RW(_attr, _minv, _maxv, _wmi) \
+ __WMI_STORE_INT(_attr##_current_value, _minv, _maxv, _wmi); \
+ WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
+ static struct kobj_attribute attr_##_attr##_current_value = \
+ __ASUS_ATTR_RW(_attr, current_value)
+
+/* Shows a formatted static variable */
+#define __ATTR_SHOW_FMT(_prop, _attrname, _fmt, _val) \
+ static ssize_t _attrname##_##_prop##_show( \
+ struct kobject *kobj, struct kobj_attribute *attr, char *buf) \
+ { \
+ return sysfs_emit(buf, _fmt, _val); \
+ } \
+ static struct kobj_attribute attr_##_attrname##_##_prop = \
+ __ASUS_ATTR_RO(_attrname, _prop)
+
+/* Boolean style enumeration, base macro. Requires adding show/store */
+#define __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname) \
+ __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
+ __ATTR_SHOW_FMT(possible_values, _attrname, "%s\n", _possible); \
+ static struct kobj_attribute attr_##_attrname##_type = \
+ __ASUS_ATTR_RO_AS(type, enum_type_show); \
+ static struct attribute *_attrname##_attrs[] = { \
+ &attr_##_attrname##_current_value.attr, \
+ &attr_##_attrname##_display_name.attr, \
+ &attr_##_attrname##_possible_values.attr, \
+ &attr_##_attrname##_type.attr, \
+ NULL \
+ }; \
+ static const struct attribute_group _attrname##_attr_group = { \
+ .name = _fsname, .attrs = _attrname##_attrs \
+ }
+
+#define ATTR_GROUP_BOOL_RO(_attrname, _fsname, _wmi, _dispname) \
+ __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
+ __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
+
+#define ATTR_GROUP_BOOL_RW(_attrname, _fsname, _wmi, _dispname) \
+ __ATTR_CURRENT_INT_RW(_attrname, 0, 1, _wmi); \
+ __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
+
+/*
+ * Requires <name>_current_value_show(), <name>_current_value_show()
+ */
+#define ATTR_GROUP_BOOL_CUSTOM(_attrname, _fsname, _dispname) \
+ static struct kobj_attribute attr_##_attrname##_current_value = \
+ __ASUS_ATTR_RW(_attrname, current_value); \
+ __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
+
+#define ATTR_GROUP_ENUM_INT_RO(_attrname, _fsname, _wmi, _possible, _dispname) \
+ __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
+ __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname)
+
+/*
+ * Requires <name>_current_value_show(), <name>_current_value_show()
+ * and <name>_possible_values_show()
+ */
+#define ATTR_GROUP_ENUM_CUSTOM(_attrname, _fsname, _dispname) \
+ __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
+ static struct kobj_attribute attr_##_attrname##_current_value = \
+ __ASUS_ATTR_RW(_attrname, current_value); \
+ static struct kobj_attribute attr_##_attrname##_possible_values = \
+ __ASUS_ATTR_RO(_attrname, possible_values); \
+ static struct kobj_attribute attr_##_attrname##_type = \
+ __ASUS_ATTR_RO_AS(type, enum_type_show); \
+ static struct attribute *_attrname##_attrs[] = { \
+ &attr_##_attrname##_current_value.attr, \
+ &attr_##_attrname##_display_name.attr, \
+ &attr_##_attrname##_possible_values.attr, \
+ &attr_##_attrname##_type.attr, \
+ NULL \
+ }; \
+ static const struct attribute_group _attrname##_attr_group = { \
+ .name = _fsname, .attrs = _attrname##_attrs \
+ }
+
+#endif /* _ASUS_BIOSCFG_H_ */
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 0a5221d65130..3c6d774f4453 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -55,8 +55,6 @@ module_param(fnlock_default, bool, 0444);
#define to_asus_wmi_driver(pdrv) \
(container_of((pdrv), struct asus_wmi_driver, platform_driver))
-#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
-
#define NOTIFY_BRNUP_MIN 0x11
#define NOTIFY_BRNUP_MAX 0x1f
#define NOTIFY_BRNDOWN_MIN 0x20
@@ -105,8 +103,6 @@ module_param(fnlock_default, bool, 0444);
#define USB_INTEL_XUSB2PR 0xD0
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
-#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
-
#define WMI_EVENT_MASK 0xFFFF
#define FAN_CURVE_POINTS 8
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 6ea4dedfb85e..21313e1eb6c9 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -6,6 +6,9 @@
#include <linux/types.h>
#include <linux/dmi.h>
+#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
+#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
+
/* WMI Methods */
#define ASUS_WMI_METHODID_SPEC 0x43455053 /* BIOS SPECification */
#define ASUS_WMI_METHODID_SFBD 0x44424653 /* Set First Boot Device */
--
2.46.1
On Mon, 30 Sep 2024, Luke D. Jones wrote:
> The fw_attributes_class provides a much cleaner interface to all of the
> attributes introduced to asus-wmi. This patch moves all of these extra
> attributes over to fw_attributes_class, and shifts the bulk of these
> definitions to a new kernel module to reduce the clutter of asus-wmi
> with the intention of deprecating the asus-wmi attributes in future.
>
> The work applies only to WMI methods which don't have a clearly defined
> place within the sysfs and as a result ended up lumped together in
> /sys/devices/platform/asus-nb-wmi/ with no standard API.
>
> Where possible the fw attrs now implement defaults, min, max, scalar,
> choices, etc. As en example dgpu_disable becomes:
>
> /sys/class/firmware-attributes/asus-armoury/attributes/dgpu_disable/
> ├── current_value
> ├── display_name
> ├── possible_values
> └── type
>
> as do other attributes.
>
> The ppt_* based attributes are removed in this initial patch as the
> implementation is somewhat broken due to the WMI methods requiring a
> set of limits on the values accepted (which is not provided by WMI).
>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
> drivers/platform/x86/Kconfig | 12 +
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/asus-armoury.c | 593 +++++++++++++++++++++
> drivers/platform/x86/asus-armoury.h | 146 +++++
> drivers/platform/x86/asus-wmi.c | 4 -
> include/linux/platform_data/x86/asus-wmi.h | 3 +
> 6 files changed, 755 insertions(+), 4 deletions(-)
> create mode 100644 drivers/platform/x86/asus-armoury.c
> create mode 100644 drivers/platform/x86/asus-armoury.h
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 3875abba5a79..80ec8b45022d 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -265,6 +265,18 @@ config ASUS_WIRELESS
> If you choose to compile this driver as a module the module will be
> called asus-wireless.
>
> +config ASUS_ARMOURY
> + tristate "ASUS Armoury driver"
> + depends on ASUS_WMI
> + select FW_ATTR_CLASS
> + help
> + Say Y here if you have a WMI aware Asus machine and would like to use the
> + firmware_attributes API to control various settings typically exposed in
> + the ASUS Armoury Crate application available on Windows.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called asus-armoury.
> +
> config ASUS_WMI
> tristate "ASUS WMI Driver"
> depends on ACPI_WMI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index e1b142947067..fe3e7e7dede8 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_APPLE_GMUX) += apple-gmux.o
> # ASUS
> obj-$(CONFIG_ASUS_LAPTOP) += asus-laptop.o
> obj-$(CONFIG_ASUS_WIRELESS) += asus-wireless.o
> +obj-$(CONFIG_ASUS_ARMOURY) += asus-armoury.o
> obj-$(CONFIG_ASUS_WMI) += asus-wmi.o
> obj-$(CONFIG_ASUS_NB_WMI) += asus-nb-wmi.o
> obj-$(CONFIG_ASUS_TF103C_DOCK) += asus-tf103c-dock.o
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> new file mode 100644
> index 000000000000..0085f1b112ab
> --- /dev/null
> +++ b/drivers/platform/x86/asus-armoury.c
> @@ -0,0 +1,593 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Asus Armoury (WMI) attributes driver. This driver uses the fw_attributes
> + * class to expose the various WMI functions that many gaming and some
> + * non-gaming ASUS laptops have available.
> + * These typically don't fit anywhere else in the sysfs such as under LED class,
> + * hwmon or other, and are set in Windows using the ASUS Armoury Crate tool.
> + *
> + * Copyright(C) 2010 Intel Corporation.
Does something from 2010 really have relevance to this? I won't oppose if
you feel you want to keep it but looking through the early commits into
drivers/platform/x86/eeepc-wmi.c and what's left of them makes me quite
skeptical.
> + * Copyright(C) 2024-2024 Luke Jones <luke@ljones.dev>
2024 seems enough.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/dmi.h>
> +#include <linux/errno.h>
> +#include <linux/fs.h>
> +#include <linux/kernel.h>
> +#include <linux/kmod.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_data/x86/asus-wmi.h>
> +#include <linux/types.h>
> +#include <linux/acpi.h>
> +
> +#include "asus-armoury.h"
> +#include "firmware_attributes_class.h"
> +
> +#define ASUS_NB_WMI_EVENT_GUID "0B3CBB35-E3C2-45ED-91C2-4C5A6D195D1C"
> +
> +#define ASUS_MINI_LED_MODE_MASK 0x03
> +/* Standard modes for devices with only on/off */
> +#define ASUS_MINI_LED_OFF 0x00
> +#define ASUS_MINI_LED_ON 0x01
> +/* Like "on" but the effect is more vibrant or brighter */
> +#define ASUS_MINI_LED_STRONG_MODE 0x02
> +/* New modes for devices with 3 mini-led mode types */
> +#define ASUS_MINI_LED_2024_WEAK 0x00
> +#define ASUS_MINI_LED_2024_STRONG 0x01
> +#define ASUS_MINI_LED_2024_OFF 0x02
> +
> +/* Default limits for tunables available on ASUS ROG laptops */
> +#define NVIDIA_BOOST_MIN 5
> +#define NVIDIA_BOOST_MAX 25
> +#define NVIDIA_TEMP_MIN 75
> +#define NVIDIA_TEMP_MAX 87
> +#define PPT_CPU_LIMIT_MIN 5
> +#define PPT_CPU_LIMIT_MAX 150
> +#define PPT_CPU_LIMIT_DEFAULT 80
> +#define PPT_PLATFORM_MIN 5
> +#define PPT_PLATFORM_MAX 100
> +#define PPT_PLATFORM_DEFAULT 80
Please try to align the defined values better above.
> +static const struct class *fw_attr_class;
> +
> +struct asus_armoury_priv {
> + struct device *fw_attr_dev;
> + struct kset *fw_attr_kset;
> +
> + u32 mini_led_dev_id;
> + u32 gpu_mux_dev_id;
> +
> + struct mutex mutex;
Please document what this protects.
> +};
> +
> +static struct asus_armoury_priv asus_armoury = { .mutex = __MUTEX_INITIALIZER(
> + asus_armoury.mutex) };
If you need to break lines, put the members on own lines as usual:
static struct asus_armoury_priv asus_armoury = {
.mutex = ...,
};
> +
> +struct fw_attrs_group {
> + bool pending_reboot;
> +};
> +
> +static struct fw_attrs_group fw_attrs = {
> + .pending_reboot = false,
> +};
> +
> +struct asus_attr_group {
> + const struct attribute_group *attr_group;
> + u32 wmi_devid;
> +};
> +
> +static bool asus_wmi_is_present(u32 dev_id)
> +{
> + u32 retval;
> + int status;
> +
> + status = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, dev_id, 0, &retval);
> + pr_debug("%s called (0x%08x), retval: 0x%08x\n", __func__, dev_id, retval);
> +
> + return status == 0 && (retval & ASUS_WMI_DSTS_PRESENCE_BIT);
> +}
> +
> +static void asus_set_reboot_and_signal_event(void)
> +{
> + fw_attrs.pending_reboot = true;
> + kobject_uevent(&asus_armoury.fw_attr_dev->kobj, KOBJ_CHANGE);
> +}
> +
> +static ssize_t pending_reboot_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "%d\n", fw_attrs.pending_reboot);
> +}
> +
> +static struct kobj_attribute pending_reboot = __ATTR_RO(pending_reboot);
> +
> +static bool asus_bios_requires_reboot(struct kobj_attribute *attr)
> +{
> + return !strcmp(attr->attr.name, "gpu_mux_mode");
> +}
> +
> +/**
> + * attr_int_store() - Generic store function for use with most WMI functions.
> + * @kobj: Pointer to the driver object.
> + * @kobj_attribute: Pointer to the attribute calling this function.
> + * @buf: The buffer to read from, this is parsed to `int` type.
> + * @count: Required by sysfs attribute macros, pass in from the callee attr.
> + * @min: Minimum accepted value. Below this returns -EINVAL.
> + * @max: Maximum accepted value. Above this returns -EINVAL.
> + * @store_value: Pointer to where the parsed value should be stored.
> + * @wmi_dev: The WMI function ID to use.
> + *
Lacks description of what it does (enven a single sentence would make
this much more useful as a whole). The WMI error value discussion below is
fine but it feels out-of-place if that's only thing there is.
> + * The WMI functions available on most ASUS laptops return a 1 as "success", and
> + * a 0 as failed. However some functions can return n > 1 for additional errors.
> + * The function currently treats all values which are not 1 as errors, ignoring
> + * the possible differences in WMI error returns.
> + *
> + * Returns: Either count, or an error.
> + */
> +static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf,
> + size_t count, u32 min, u32 max, u32 *store_value, u32 wmi_dev)
> +{
> + u32 result, value;
> + int err;
> +
> + err = kstrtouint(buf, 10, &value);
I'd prefer the function be named "..._uint_..." instead of "int" to not
lose this part of the behavior of the wrapper.
> + if (err)
> + return err;
> +
> + if (value < min || value > max)
> + return -EINVAL;
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(wmi_dev, value, &result);
> + mutex_unlock(&asus_armoury.mutex);
Please wrap this so you don't have to do the lock in every single
callsite. You can use guard() in the wrapper.
> + if (err) {
> + pr_err("Failed to set %s: %d\n", attr->attr.name, err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_err("Failed to set %s (result): 0x%x\n", attr->attr.name, result);
> + return -EIO;
> + }
> +
> + if (store_value != NULL)
> + *store_value = value;
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + if (asus_bios_requires_reboot(attr))
> + asus_set_reboot_and_signal_event();
> +
> + return count;
> +}
> +
> +/* Mini-LED mode **************************************************************/
> +static ssize_t mini_led_mode_current_value_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + u32 value;
> + int err;
> +
> + err = asus_wmi_get_devstate_dsts(asus_armoury.mini_led_dev_id, &value);
> + if (err)
> + return err;
> +
> + value &= ASUS_MINI_LED_MODE_MASK;
> +
> + /*
> + * Remap the mode values to match previous generation mini-LED. The last gen
> + * WMI 0 == off, while on this version WMI 2 == off (flipped).
> + */
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
> + switch (value) {
> + case ASUS_MINI_LED_2024_WEAK:
> + value = ASUS_MINI_LED_ON;
> + break;
> + case ASUS_MINI_LED_2024_STRONG:
> + value = ASUS_MINI_LED_STRONG_MODE;
> + break;
> + case ASUS_MINI_LED_2024_OFF:
> + value = ASUS_MINI_LED_OFF;
> + break;
> + }
> + }
> +
> + return sysfs_emit(buf, "%u\n", value);
> +}
> +
> +static ssize_t mini_led_mode_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf,
> + size_t count)
I'd put the buf & count on the same line. They kind of belong together and
the lines would be more equal in length too.
> +{
> + int result, err;
> + u32 mode;
> +
> + err = kstrtou32(buf, 10, &mode);
> + if (err)
> + return err;
> +
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE &&
> + mode > ASUS_MINI_LED_ON)
> + return -EINVAL;
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2 &&
> + mode > ASUS_MINI_LED_STRONG_MODE)
> + return -EINVAL;
> +
> + /*
> + * Remap the mode values so expected behaviour is the same as the last
> + * generation of mini-LED with 0 == off, 1 == on.
> + */
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
> + switch (mode) {
> + case ASUS_MINI_LED_OFF:
> + mode = ASUS_MINI_LED_2024_OFF;
> + break;
> + case ASUS_MINI_LED_ON:
> + mode = ASUS_MINI_LED_2024_WEAK;
> + break;
> + case ASUS_MINI_LED_STRONG_MODE:
> + mode = ASUS_MINI_LED_2024_STRONG;
> + break;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(asus_armoury.mini_led_dev_id, mode, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_warn("Failed to set mini-LED: %d\n", err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_warn("Failed to set mini-LED mode (result): 0x%x\n", result);
> + return -EIO;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + return count;
> +}
> +
> +static ssize_t mini_led_mode_possible_values_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + switch (asus_armoury.mini_led_dev_id) {
> + case ASUS_WMI_DEVID_MINI_LED_MODE:
> + return sysfs_emit(buf, "0;1\n");
> + case ASUS_WMI_DEVID_MINI_LED_MODE2:
> + return sysfs_emit(buf, "0;1;2\n");
> + }
> +
> + return sysfs_emit(buf, "0\n");
> +}
> +
> +ATTR_GROUP_ENUM_CUSTOM(mini_led_mode, "mini_led_mode", "Set the mini-LED backlight mode");
> +
> +static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int result, err;
> + u32 optimus;
> +
> + err = kstrtou32(buf, 10, &optimus);
> + if (err)
> + return err;
> +
> + if (optimus > 1)
> + return -EINVAL;
> +
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_DGPU)) {
> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_DGPU, &result);
> + if (err)
> + return err;
> + if (result && !optimus) {
> + err = -ENODEV;
> + pr_warn("Can not switch MUX to dGPU mode when dGPU is disabled: %02X %02X %d\n",
> + result, optimus, err);
Printing an error code you synthetized right above the print doesn't feel
very useful. With that removed, you can just use return -ENODEV;
> + return err;
> + }
> + }
> +
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_EGPU)) {
> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU, &result);
> + if (err)
> + return err;
> + if (result && !optimus) {
> + err = -ENODEV;
> + pr_warn("Can not switch MUX to dGPU mode when eGPU is enabled: %d\n",
> + err);
Ditto.
> + return err;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(asus_armoury.gpu_mux_dev_id, optimus, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_err("Failed to set GPU MUX mode: %d\n", err);
> + return err;
> + }
> + /* !1 is considered a fail by ASUS */
> + if (result != 1) {
> + pr_warn("Failed to set GPU MUX mode (result): 0x%x\n", result);
> + return -EIO;
> + }
I wonder if the asus_wmi_set_devstate wrapper should also handle these
error checks + prints. They seem pretty much duplicated except for the
error string that could be provided as an argument.
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> + asus_set_reboot_and_signal_event();
> +
> + return count;
> +}
> +WMI_SHOW_INT(gpu_mux_mode_current_value, "%d\n", asus_armoury.gpu_mux_dev_id);
> +ATTR_GROUP_BOOL_CUSTOM(gpu_mux_mode, "gpu_mux_mode", "Set the GPU display MUX mode");
> +
> +/*
> + * A user may be required to store the value twice, typical store first, then
> + * rescan PCI bus to activate power, then store a second time to save correctly.
> + * The reason for this is that an extra code path in the ACPI is enabled when
> + * the device and bus are powered.
> + */
> +static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int result, err;
> + u32 disable;
> +
> + err = kstrtou32(buf, 10, &disable);
> + if (err)
> + return err;
> +
> + if (disable > 1)
> + return -EINVAL;
> +
> + if (asus_armoury.gpu_mux_dev_id) {
> + err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
> + if (err)
> + return err;
> + if (!result && disable) {
> + err = -ENODEV;
> + pr_warn("Can not disable dGPU when the MUX is in dGPU mode: %d\n", err);
> + return err;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_DGPU, disable, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_warn("Failed to set dGPU disable: %d\n", err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_warn("Failed to set dGPU disable (result): 0x%x\n", result);
> + return -EIO;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + return count;
> +}
> +WMI_SHOW_INT(dgpu_disable_current_value, "%d\n", ASUS_WMI_DEVID_DGPU);
> +ATTR_GROUP_BOOL_CUSTOM(dgpu_disable, "dgpu_disable", "Disable the dGPU");
> +
> +/* The ACPI call to enable the eGPU also disables the internal dGPU */
> +static ssize_t egpu_enable_current_value_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int result, err;
> + u32 enable;
> +
> + err = kstrtou32(buf, 10, &enable);
> + if (err)
> + return err;
> +
> + if (enable > 1)
> + return -EINVAL;
> +
> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU_CONNECTED, &result);
> + if (err) {
> + pr_warn("Failed to get eGPU connection status: %d\n", err);
> + return err;
> + }
> +
> + if (asus_armoury.gpu_mux_dev_id) {
> + err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
> + if (err) {
> + pr_warn("Failed to get GPU MUX status: %d\n", result);
> + return result;
> + }
> + if (!result && enable) {
> + err = -ENODEV;
> + pr_warn("Can not enable eGPU when the MUX is in dGPU mode: %d\n", err);
> + return err;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_EGPU, enable, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_warn("Failed to set eGPU state: %d\n", err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_warn("Failed to set eGPU state (retval): 0x%x\n", result);
> + return -EIO;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + return count;
> +}
> +WMI_SHOW_INT(egpu_enable_current_value, "%d\n", ASUS_WMI_DEVID_EGPU);
> +ATTR_GROUP_BOOL_CUSTOM(egpu_enable, "egpu_enable", "Enable the eGPU (also disables dGPU)");
> +
> +/* Simple attribute creation */
> +ATTR_GROUP_ENUM_INT_RO(charge_mode, "charge_mode", ASUS_WMI_DEVID_CHARGE_MODE, "0;1;2",
> + "Show the current mode of charging");
> +
> +ATTR_GROUP_BOOL_RW(boot_sound, "boot_sound", ASUS_WMI_DEVID_BOOT_SOUND,
> + "Set the boot POST sound");
> +ATTR_GROUP_BOOL_RW(mcu_powersave, "mcu_powersave", ASUS_WMI_DEVID_MCU_POWERSAVE,
> + "Set MCU powersaving mode");
> +ATTR_GROUP_BOOL_RW(panel_od, "panel_overdrive", ASUS_WMI_DEVID_PANEL_OD,
> + "Set the panel refresh overdrive");
> +ATTR_GROUP_BOOL_RO(egpu_connected, "egpu_connected", ASUS_WMI_DEVID_EGPU_CONNECTED,
> + "Show the eGPU connection status");
> +
> +/* If an attribute does not require any special case handling add it here */
> +static const struct asus_attr_group armoury_attr_groups[] = {
> + { &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
> + { &egpu_enable_attr_group, ASUS_WMI_DEVID_EGPU },
> + { &dgpu_disable_attr_group, ASUS_WMI_DEVID_DGPU },
> +
> + { &charge_mode_attr_group, ASUS_WMI_DEVID_CHARGE_MODE },
> + { &boot_sound_attr_group, ASUS_WMI_DEVID_BOOT_SOUND },
> + { &mcu_powersave_attr_group, ASUS_WMI_DEVID_MCU_POWERSAVE },
> + { &panel_od_attr_group, ASUS_WMI_DEVID_PANEL_OD },
> +};
> +
> +static int asus_fw_attr_add(void)
> +{
> + int err, i;
> +
> + err = fw_attributes_class_get(&fw_attr_class);
> + if (err)
> + return err;
> +
> + asus_armoury.fw_attr_dev =
> + device_create(fw_attr_class, NULL, MKDEV(0, 0), NULL, "%s", DRIVER_NAME);
Don't split at = but after MKDEV() arg.
> +
> + if (IS_ERR(asus_armoury.fw_attr_dev)) {
Don't leave empty line between call and it's error handling.
> + err = PTR_ERR(asus_armoury.fw_attr_dev);
> + goto fail_class_get;
> + }
> +
> + asus_armoury.fw_attr_kset =
> + kset_create_and_add("attributes", NULL, &asus_armoury.fw_attr_dev->kobj);
Don't split at =
> + if (!asus_armoury.fw_attr_kset) {
> + err = -ENOMEM;
> + goto err_destroy_classdev;
> + }
> +
> + err = sysfs_create_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> + if (err) {
> + pr_err("Failed to create sysfs level attributes\n");
> + goto err_destroy_kset;
> + }
> +
> + asus_armoury.mini_led_dev_id = 0;
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE)) {
> + asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> + } else if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE2)) {
> + asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE2;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> + }
> + if (err) {
> + pr_err("Failed to create sysfs-group for mini_led\n");
> + goto err_remove_file;
> + }
asus_armoury.mini_led_dev_id = 0;
if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE))
asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
else if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE2))
asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE2;
if (asus_armoury.mini_led_dev_id) {
err = sysfs_create_group(...);
if (err) {
...
}
}
> +
> + asus_armoury.gpu_mux_dev_id = 0;
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX)) {
> + asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> + } else if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX_VIVO)) {
> + asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX_VIVO;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> + }
> + if (err) {
> + pr_err("Failed to create sysfs-group for gpu_mux\n");
> + goto err_remove_mini_led_group;
> + }
Same here.
> +
> + for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
> + if (!asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
> + continue;
> +
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> + armoury_attr_groups[i].attr_group);
> + if (err) {
> + pr_err("Failed to create sysfs-group for %s\n",
> + armoury_attr_groups[i].attr_group->name);
> + goto err_remove_groups;
> + } else {
No need to use else since you goto away from the error block.
> + pr_debug("Created sysfs-group for %s\n",
> + armoury_attr_groups[i].attr_group->name);
> + }
> + }
> +
> + return 0;
> +
> +err_remove_groups:
> + while (--i >= 0) {
> + if (asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, armoury_attr_groups[i].attr_group);
> + }
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> +err_remove_mini_led_group:
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
Do you need to check if these two got created before calling remove? IIRC,
some sysfs remove function required that.
> +err_remove_file:
> + sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> +err_destroy_kset:
> + kset_unregister(asus_armoury.fw_attr_kset);
> +err_destroy_classdev:
> + device_destroy(fw_attr_class, MKDEV(0, 0));
> +fail_class_get:
> + fw_attributes_class_put();
> + return err;
> +}
> +
> +/* Init / exit ****************************************************************/
> +
> +static int __init asus_fw_init(void)
> +{
> + char *wmi_uid;
> + int err;
> +
> + wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
> + if (!wmi_uid)
> + return -ENODEV;
> +
> + /*
> + * if equal to "ASUSWMI" then it's DCTS that can't be used for this
> + * driver, DSTS is required.
> + */
> + if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI))
> + return -ENODEV;
> +
> + err = asus_fw_attr_add();
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static void __exit asus_fw_exit(void)
> +{
> + mutex_lock(&asus_armoury.mutex);
I'm not sure if this really helps anything. What are you trying to protect
against here with it?
> + sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> + kset_unregister(asus_armoury.fw_attr_kset);
> + device_destroy(fw_attr_class, MKDEV(0, 0));
> + fw_attributes_class_put();
> +
> + mutex_unlock(&asus_armoury.mutex);
> +}
> +
> +module_init(asus_fw_init);
> +module_exit(asus_fw_exit);
> +
> +MODULE_IMPORT_NS(ASUS_WMI);
> +MODULE_AUTHOR("Luke Jones <luke@ljones.dev>");
> +MODULE_DESCRIPTION("ASUS BIOS Configuration Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("wmi:" ASUS_NB_WMI_EVENT_GUID);
> diff --git a/drivers/platform/x86/asus-armoury.h b/drivers/platform/x86/asus-armoury.h
> new file mode 100644
> index 000000000000..4d0dd34c52aa
> --- /dev/null
> +++ b/drivers/platform/x86/asus-armoury.h
> @@ -0,0 +1,146 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Definitions for kernel modules using asus-armoury driver
> + *
> + * Copyright (c) 2024 Luke Jones <luke@ljones.dev>
> + */
> +
> +#ifndef _ASUS_ARMOURY_H_
> +#define _ASUS_ARMOURY_H_
> +
> +#include <linux/types.h>
> +#include <linux/platform_device.h>
> +
> +#define DRIVER_NAME "asus-armoury"
> +
> +static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count, u32 min, u32 max,
> + u32 *store_value, u32 wmi_dev);
> +
> +static ssize_t enum_type_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "enumeration\n");
> +}
> +
> +#define __ASUS_ATTR_RO(_func, _name) \
> + { \
> + .attr = { .name = __stringify(_name), .mode = 0444 }, \
> + .show = _func##_##_name##_show, \
> + }
Align all the continuation \ with tabs.
> +#define __ASUS_ATTR_RO_AS(_name, _show) \
> + { \
> + .attr = { .name = __stringify(_name), .mode = 0444 }, \
> + .show = _show, \
> + }
> +
> +#define __ASUS_ATTR_RW(_func, _name) \
> + __ATTR(_name, 0644, _func##_##_name##_show, _func##_##_name##_store)
> +
> +#define __WMI_STORE_INT(_attr, _min, _max, _wmi) \
> + static ssize_t _attr##_store(struct kobject *kobj, \
> + struct kobj_attribute *attr, \
> + const char *buf, size_t count) \
> + { \
> + return attr_int_store(kobj, attr, buf, count, _min, _max, \
> + NULL, _wmi); \
> + }
> +
> +#define WMI_SHOW_INT(_attr, _fmt, _wmi) \
> + static ssize_t _attr##_show(struct kobject *kobj, \
> + struct kobj_attribute *attr, char *buf) \
> + { \
> + u32 result; \
> + int err; \
Empty line here as per the usual style, with the continuation \ obviously.
> + err = asus_wmi_get_devstate_dsts(_wmi, &result); \
> + if (err) \
> + return err; \
> + return sysfs_emit(buf, _fmt, \
> + result & ~ASUS_WMI_DSTS_PRESENCE_BIT); \
> + }
I'll stop reviewing on this patch as I'm out of ability to focus. I'll
take a look at the remaining patches later.
--
i.
> +
> +/* Create functions and attributes for use in other macros or on their own */
> +
> +#define __ATTR_CURRENT_INT_RO(_attr, _wmi) \
> + WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
> + static struct kobj_attribute attr_##_attr##_current_value = \
> + __ASUS_ATTR_RO(_attr, current_value)
> +
> +#define __ATTR_CURRENT_INT_RW(_attr, _minv, _maxv, _wmi) \
> + __WMI_STORE_INT(_attr##_current_value, _minv, _maxv, _wmi); \
> + WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
> + static struct kobj_attribute attr_##_attr##_current_value = \
> + __ASUS_ATTR_RW(_attr, current_value)
> +
> +/* Shows a formatted static variable */
> +#define __ATTR_SHOW_FMT(_prop, _attrname, _fmt, _val) \
> + static ssize_t _attrname##_##_prop##_show( \
> + struct kobject *kobj, struct kobj_attribute *attr, char *buf) \
> + { \
> + return sysfs_emit(buf, _fmt, _val); \
> + } \
> + static struct kobj_attribute attr_##_attrname##_##_prop = \
> + __ASUS_ATTR_RO(_attrname, _prop)
> +
> +/* Boolean style enumeration, base macro. Requires adding show/store */
> +#define __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname) \
> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
> + __ATTR_SHOW_FMT(possible_values, _attrname, "%s\n", _possible); \
> + static struct kobj_attribute attr_##_attrname##_type = \
> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
> + static struct attribute *_attrname##_attrs[] = { \
> + &attr_##_attrname##_current_value.attr, \
> + &attr_##_attrname##_display_name.attr, \
> + &attr_##_attrname##_possible_values.attr, \
> + &attr_##_attrname##_type.attr, \
> + NULL \
> + }; \
> + static const struct attribute_group _attrname##_attr_group = { \
> + .name = _fsname, .attrs = _attrname##_attrs \
> + }
> +
> +#define ATTR_GROUP_BOOL_RO(_attrname, _fsname, _wmi, _dispname) \
> + __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
> +
> +#define ATTR_GROUP_BOOL_RW(_attrname, _fsname, _wmi, _dispname) \
> + __ATTR_CURRENT_INT_RW(_attrname, 0, 1, _wmi); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
> +
> +/*
> + * Requires <name>_current_value_show(), <name>_current_value_show()
> + */
> +#define ATTR_GROUP_BOOL_CUSTOM(_attrname, _fsname, _dispname) \
> + static struct kobj_attribute attr_##_attrname##_current_value = \
> + __ASUS_ATTR_RW(_attrname, current_value); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
> +
> +#define ATTR_GROUP_ENUM_INT_RO(_attrname, _fsname, _wmi, _possible, _dispname) \
> + __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname)
> +
> +/*
> + * Requires <name>_current_value_show(), <name>_current_value_show()
> + * and <name>_possible_values_show()
> + */
> +#define ATTR_GROUP_ENUM_CUSTOM(_attrname, _fsname, _dispname) \
> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
> + static struct kobj_attribute attr_##_attrname##_current_value = \
> + __ASUS_ATTR_RW(_attrname, current_value); \
> + static struct kobj_attribute attr_##_attrname##_possible_values = \
> + __ASUS_ATTR_RO(_attrname, possible_values); \
> + static struct kobj_attribute attr_##_attrname##_type = \
> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
> + static struct attribute *_attrname##_attrs[] = { \
> + &attr_##_attrname##_current_value.attr, \
> + &attr_##_attrname##_display_name.attr, \
> + &attr_##_attrname##_possible_values.attr, \
> + &attr_##_attrname##_type.attr, \
> + NULL \
> + }; \
> + static const struct attribute_group _attrname##_attr_group = { \
> + .name = _fsname, .attrs = _attrname##_attrs \
> + }
> +
> +#endif /* _ASUS_BIOSCFG_H_ */
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 0a5221d65130..3c6d774f4453 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -55,8 +55,6 @@ module_param(fnlock_default, bool, 0444);
> #define to_asus_wmi_driver(pdrv) \
> (container_of((pdrv), struct asus_wmi_driver, platform_driver))
>
> -#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
> -
> #define NOTIFY_BRNUP_MIN 0x11
> #define NOTIFY_BRNUP_MAX 0x1f
> #define NOTIFY_BRNDOWN_MIN 0x20
> @@ -105,8 +103,6 @@ module_param(fnlock_default, bool, 0444);
> #define USB_INTEL_XUSB2PR 0xD0
> #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
>
> -#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
> -
> #define WMI_EVENT_MASK 0xFFFF
>
> #define FAN_CURVE_POINTS 8
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 6ea4dedfb85e..21313e1eb6c9 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -6,6 +6,9 @@
> #include <linux/types.h>
> #include <linux/dmi.h>
>
> +#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
> +#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
> +
> /* WMI Methods */
> #define ASUS_WMI_METHODID_SPEC 0x43455053 /* BIOS SPECification */
> #define ASUS_WMI_METHODID_SFBD 0x44424653 /* Set First Boot Device */
>
Hello,
On Wed, 16 Oct 2024, at 4:54 PM, Ilpo Järvinen wrote:
> On Mon, 30 Sep 2024, Luke D. Jones wrote:
>
>> The fw_attributes_class provides a much cleaner interface to all of the
>> attributes introduced to asus-wmi. This patch moves all of these extra
>> attributes over to fw_attributes_class, and shifts the bulk of these
>> definitions to a new kernel module to reduce the clutter of asus-wmi
>> with the intention of deprecating the asus-wmi attributes in future.
>>
>> The work applies only to WMI methods which don't have a clearly defined
>> place within the sysfs and as a result ended up lumped together in
>> /sys/devices/platform/asus-nb-wmi/ with no standard API.
>>
>> Where possible the fw attrs now implement defaults, min, max, scalar,
>> choices, etc. As en example dgpu_disable becomes:
>>
>> /sys/class/firmware-attributes/asus-armoury/attributes/dgpu_disable/
>> ├── current_value
>> ├── display_name
>> ├── possible_values
>> └── type
>>
>> as do other attributes.
>>
>> The ppt_* based attributes are removed in this initial patch as the
>> implementation is somewhat broken due to the WMI methods requiring a
>> set of limits on the values accepted (which is not provided by WMI).
>>
>> Signed-off-by: Luke D. Jones <luke@ljones.dev>
>> ---
>> drivers/platform/x86/Kconfig | 12 +
>> drivers/platform/x86/Makefile | 1 +
>> drivers/platform/x86/asus-armoury.c | 593 +++++++++++++++++++++
>> drivers/platform/x86/asus-armoury.h | 146 +++++
>> drivers/platform/x86/asus-wmi.c | 4 -
>> include/linux/platform_data/x86/asus-wmi.h | 3 +
>> 6 files changed, 755 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/platform/x86/asus-armoury.c
>> create mode 100644 drivers/platform/x86/asus-armoury.h
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 3875abba5a79..80ec8b45022d 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -265,6 +265,18 @@ config ASUS_WIRELESS
>> If you choose to compile this driver as a module the module will be
>> called asus-wireless.
>>
>> +config ASUS_ARMOURY
>> + tristate "ASUS Armoury driver"
>> + depends on ASUS_WMI
>> + select FW_ATTR_CLASS
>> + help
>> + Say Y here if you have a WMI aware Asus machine and would like to use the
>> + firmware_attributes API to control various settings typically exposed in
>> + the ASUS Armoury Crate application available on Windows.
>> +
>> + To compile this driver as a module, choose M here: the module will
>> + be called asus-armoury.
>> +
>> config ASUS_WMI
>> tristate "ASUS WMI Driver"
>> depends on ACPI_WMI
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index e1b142947067..fe3e7e7dede8 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -32,6 +32,7 @@ obj-$(CONFIG_APPLE_GMUX) += apple-gmux.o
>> # ASUS
>> obj-$(CONFIG_ASUS_LAPTOP) += asus-laptop.o
>> obj-$(CONFIG_ASUS_WIRELESS) += asus-wireless.o
>> +obj-$(CONFIG_ASUS_ARMOURY) += asus-armoury.o
>> obj-$(CONFIG_ASUS_WMI) += asus-wmi.o
>> obj-$(CONFIG_ASUS_NB_WMI) += asus-nb-wmi.o
>> obj-$(CONFIG_ASUS_TF103C_DOCK) += asus-tf103c-dock.o
>> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
>> new file mode 100644
>> index 000000000000..0085f1b112ab
>> --- /dev/null
>> +++ b/drivers/platform/x86/asus-armoury.c
>> @@ -0,0 +1,593 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Asus Armoury (WMI) attributes driver. This driver uses the fw_attributes
>> + * class to expose the various WMI functions that many gaming and some
>> + * non-gaming ASUS laptops have available.
>> + * These typically don't fit anywhere else in the sysfs such as under LED class,
>> + * hwmon or other, and are set in Windows using the ASUS Armoury Crate tool.
>> + *
>> + * Copyright(C) 2010 Intel Corporation.
>
> Does something from 2010 really have relevance to this? I won't oppose if
> you feel you want to keep it but looking through the early commits into
> drivers/platform/x86/eeepc-wmi.c and what's left of them makes me quite
> skeptical.
>
>> + * Copyright(C) 2024-2024 Luke Jones <luke@ljones.dev>
>
> 2024 seems enough.
Ack both. Was copy/paste error
>
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/device.h>
>> +#include <linux/dmi.h>
>> +#include <linux/errno.h>
>> +#include <linux/fs.h>
>> +#include <linux/kernel.h>
>> +#include <linux/kmod.h>
>> +#include <linux/kobject.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_data/x86/asus-wmi.h>
>> +#include <linux/types.h>
>> +#include <linux/acpi.h>
>> +
>> +#include "asus-armoury.h"
>> +#include "firmware_attributes_class.h"
>> +
>> +#define ASUS_NB_WMI_EVENT_GUID "0B3CBB35-E3C2-45ED-91C2-4C5A6D195D1C"
>> +
>> +#define ASUS_MINI_LED_MODE_MASK 0x03
>> +/* Standard modes for devices with only on/off */
>> +#define ASUS_MINI_LED_OFF 0x00
>> +#define ASUS_MINI_LED_ON 0x01
>> +/* Like "on" but the effect is more vibrant or brighter */
>> +#define ASUS_MINI_LED_STRONG_MODE 0x02
>> +/* New modes for devices with 3 mini-led mode types */
>> +#define ASUS_MINI_LED_2024_WEAK 0x00
>> +#define ASUS_MINI_LED_2024_STRONG 0x01
>> +#define ASUS_MINI_LED_2024_OFF 0x02
>> +
>> +/* Default limits for tunables available on ASUS ROG laptops */
>> +#define NVIDIA_BOOST_MIN 5
>> +#define NVIDIA_BOOST_MAX 25
>> +#define NVIDIA_TEMP_MIN 75
>> +#define NVIDIA_TEMP_MAX 87
>> +#define PPT_CPU_LIMIT_MIN 5
>> +#define PPT_CPU_LIMIT_MAX 150
>> +#define PPT_CPU_LIMIT_DEFAULT 80
>> +#define PPT_PLATFORM_MIN 5
>> +#define PPT_PLATFORM_MAX 100
>> +#define PPT_PLATFORM_DEFAULT 80
>
> Please try to align the defined values better above.
This is how clang-format using the .clang-format config formats everything. If I change it manually it will be overwritten again.
>
>> +static const struct class *fw_attr_class;
>> +
>> +struct asus_armoury_priv {
>> + struct device *fw_attr_dev;
>> + struct kset *fw_attr_kset;
>> +
>> + u32 mini_led_dev_id;
>> + u32 gpu_mux_dev_id;
>> +
>> + struct mutex mutex;
>
> Please document what this protects.
I don't fully understand if sysfs writes can be parallel or not, so i was making the assumption that they were and if so, we would want to prevent trying to write many WMI at once. If my understanding is lacking please correct me.
>
>> +};
>> +
>> +static struct asus_armoury_priv asus_armoury = { .mutex = __MUTEX_INITIALIZER(
>> + asus_armoury.mutex) };
>
> If you need to break lines, put the members on own lines as usual:
>
> static struct asus_armoury_priv asus_armoury = {
> .mutex = ...,
> };
I think this is how clang formatted it.. weird..
Okay I'll format this and the defs manually and add
// clang-format off etc.
>
>> +
>> +struct fw_attrs_group {
>> + bool pending_reboot;
>> +};
>> +
>> +static struct fw_attrs_group fw_attrs = {
>> + .pending_reboot = false,
>> +};
>> +
>> +struct asus_attr_group {
>> + const struct attribute_group *attr_group;
>> + u32 wmi_devid;
>> +};
>> +
>> +static bool asus_wmi_is_present(u32 dev_id)
>> +{
>> + u32 retval;
>> + int status;
>> +
>> + status = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, dev_id, 0, &retval);
>> + pr_debug("%s called (0x%08x), retval: 0x%08x\n", __func__, dev_id, retval);
>> +
>> + return status == 0 && (retval & ASUS_WMI_DSTS_PRESENCE_BIT);
>> +}
>> +
>> +static void asus_set_reboot_and_signal_event(void)
>> +{
>> + fw_attrs.pending_reboot = true;
>> + kobject_uevent(&asus_armoury.fw_attr_dev->kobj, KOBJ_CHANGE);
>> +}
>> +
>> +static ssize_t pending_reboot_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>> +{
>> + return sysfs_emit(buf, "%d\n", fw_attrs.pending_reboot);
>> +}
>> +
>> +static struct kobj_attribute pending_reboot = __ATTR_RO(pending_reboot);
>> +
>> +static bool asus_bios_requires_reboot(struct kobj_attribute *attr)
>> +{
>> + return !strcmp(attr->attr.name, "gpu_mux_mode");
>> +}
>> +
>> +/**
>> + * attr_int_store() - Generic store function for use with most WMI functions.
>> + * @kobj: Pointer to the driver object.
>> + * @kobj_attribute: Pointer to the attribute calling this function.
>> + * @buf: The buffer to read from, this is parsed to `int` type.
>> + * @count: Required by sysfs attribute macros, pass in from the callee attr.
>> + * @min: Minimum accepted value. Below this returns -EINVAL.
>> + * @max: Maximum accepted value. Above this returns -EINVAL.
>> + * @store_value: Pointer to where the parsed value should be stored.
>> + * @wmi_dev: The WMI function ID to use.
>> + *
>
> Lacks description of what it does (enven a single sentence would make
> this much more useful as a whole). The WMI error value discussion below is
> fine but it feels out-of-place if that's only thing there is.
Oversight on my part. Fixed now.
>> + * The WMI functions available on most ASUS laptops return a 1 as "success", and
>> + * a 0 as failed. However some functions can return n > 1 for additional errors.
>> + * The function currently treats all values which are not 1 as errors, ignoring
>> + * the possible differences in WMI error returns.
>> + *
>> + * Returns: Either count, or an error.
>> + */
>> +static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf,
>> + size_t count, u32 min, u32 max, u32 *store_value, u32 wmi_dev)
>> +{
>> + u32 result, value;
>> + int err;
>> +
>> + err = kstrtouint(buf, 10, &value);
>
> I'd prefer the function be named "..._uint_..." instead of "int" to not
> lose this part of the behavior of the wrapper.
Yes I agree. Thank you for picking that up.
>
>> + if (err)
>> + return err;
>> +
>> + if (value < min || value > max)
>> + return -EINVAL;
>> +
>
>> + mutex_lock(&asus_armoury.mutex);
>> + err = asus_wmi_set_devstate(wmi_dev, value, &result);
>> + mutex_unlock(&asus_armoury.mutex);
>
> Please wrap this so you don't have to do the lock in every single
> callsite. You can use guard() in the wrapper.
Done, new wrap function and a nice chunk of code removed. Thanks for telling about guard().
>> + if (err) {
>> + pr_err("Failed to set %s: %d\n", attr->attr.name, err);
>> + return err;
>> + }
>> +
>> + if (result != 1) {
>> + pr_err("Failed to set %s (result): 0x%x\n", attr->attr.name, result);
>> + return -EIO;
>> + }
>> +
>> + if (store_value != NULL)
>> + *store_value = value;
>> + sysfs_notify(kobj, NULL, attr->attr.name);
>> +
>> + if (asus_bios_requires_reboot(attr))
>> + asus_set_reboot_and_signal_event();
>> +
>> + return count;
>> +}
>> +
>> +/* Mini-LED mode **************************************************************/
>> +static ssize_t mini_led_mode_current_value_show(struct kobject *kobj,
>> + struct kobj_attribute *attr, char *buf)
>> +{
>> + u32 value;
>> + int err;
>> +
>> + err = asus_wmi_get_devstate_dsts(asus_armoury.mini_led_dev_id, &value);
>> + if (err)
>> + return err;
>> +
>> + value &= ASUS_MINI_LED_MODE_MASK;
>> +
>> + /*
>> + * Remap the mode values to match previous generation mini-LED. The last gen
>> + * WMI 0 == off, while on this version WMI 2 == off (flipped).
>> + */
>> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
>> + switch (value) {
>> + case ASUS_MINI_LED_2024_WEAK:
>> + value = ASUS_MINI_LED_ON;
>> + break;
>> + case ASUS_MINI_LED_2024_STRONG:
>> + value = ASUS_MINI_LED_STRONG_MODE;
>> + break;
>> + case ASUS_MINI_LED_2024_OFF:
>> + value = ASUS_MINI_LED_OFF;
>> + break;
>> + }
>> + }
>> +
>> + return sysfs_emit(buf, "%u\n", value);
>> +}
>> +
>> +static ssize_t mini_led_mode_current_value_store(struct kobject *kobj,
>> + struct kobj_attribute *attr, const char *buf,
>> + size_t count)
>
> I'd put the buf & count on the same line. They kind of belong together and
> the lines would be more equal in length too.
ack
>> +{
>> + int result, err;
>> + u32 mode;
>> +
>> + err = kstrtou32(buf, 10, &mode);
>> + if (err)
>> + return err;
>> +
>> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE &&
>> + mode > ASUS_MINI_LED_ON)
>> + return -EINVAL;
>> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2 &&
>> + mode > ASUS_MINI_LED_STRONG_MODE)
>> + return -EINVAL;
>> +
>> + /*
>> + * Remap the mode values so expected behaviour is the same as the last
>> + * generation of mini-LED with 0 == off, 1 == on.
>> + */
>> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
>> + switch (mode) {
>> + case ASUS_MINI_LED_OFF:
>> + mode = ASUS_MINI_LED_2024_OFF;
>> + break;
>> + case ASUS_MINI_LED_ON:
>> + mode = ASUS_MINI_LED_2024_WEAK;
>> + break;
>> + case ASUS_MINI_LED_STRONG_MODE:
>> + mode = ASUS_MINI_LED_2024_STRONG;
>> + break;
>> + }
>> + }
>> +
>> + mutex_lock(&asus_armoury.mutex);
>> + err = asus_wmi_set_devstate(asus_armoury.mini_led_dev_id, mode, &result);
>> + mutex_unlock(&asus_armoury.mutex);
>> + if (err) {
>> + pr_warn("Failed to set mini-LED: %d\n", err);
>> + return err;
>> + }
>> +
>> + if (result != 1) {
>> + pr_warn("Failed to set mini-LED mode (result): 0x%x\n", result);
>> + return -EIO;
>> + }
>> +
>> + sysfs_notify(kobj, NULL, attr->attr.name);
>> +
>> + return count;
>> +}
>> +
>> +static ssize_t mini_led_mode_possible_values_show(struct kobject *kobj,
>> + struct kobj_attribute *attr, char *buf)
>> +{
>> + switch (asus_armoury.mini_led_dev_id) {
>> + case ASUS_WMI_DEVID_MINI_LED_MODE:
>> + return sysfs_emit(buf, "0;1\n");
>> + case ASUS_WMI_DEVID_MINI_LED_MODE2:
>> + return sysfs_emit(buf, "0;1;2\n");
>> + }
>> +
>> + return sysfs_emit(buf, "0\n");
>> +}
>> +
>> +ATTR_GROUP_ENUM_CUSTOM(mini_led_mode, "mini_led_mode", "Set the mini-LED backlight mode");
>> +
>> +static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
>> + struct kobj_attribute *attr, const char *buf,
>> + size_t count)
>> +{
>> + int result, err;
>> + u32 optimus;
>> +
>> + err = kstrtou32(buf, 10, &optimus);
>> + if (err)
>> + return err;
>> +
>> + if (optimus > 1)
>> + return -EINVAL;
>> +
>> + if (asus_wmi_is_present(ASUS_WMI_DEVID_DGPU)) {
>> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_DGPU, &result);
>> + if (err)
>> + return err;
>> + if (result && !optimus) {
>> + err = -ENODEV;
>> + pr_warn("Can not switch MUX to dGPU mode when dGPU is disabled: %02X %02X %d\n",
>> + result, optimus, err);
>
> Printing an error code you synthetized right above the print doesn't feel
> very useful. With that removed, you can just use return -ENODEV;
>
>> + return err;
>> + }
>> + }
>> +
>> + if (asus_wmi_is_present(ASUS_WMI_DEVID_EGPU)) {
>> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU, &result);
>> + if (err)
>> + return err;
>> + if (result && !optimus) {
>> + err = -ENODEV;
>> + pr_warn("Can not switch MUX to dGPU mode when eGPU is enabled: %d\n",
>> + err);
>
> Ditto.
>
>> + return err;
>> + }
>> + }
>> +
>> + mutex_lock(&asus_armoury.mutex);
>> + err = asus_wmi_set_devstate(asus_armoury.gpu_mux_dev_id, optimus, &result);
>> + mutex_unlock(&asus_armoury.mutex);
>> + if (err) {
>> + pr_err("Failed to set GPU MUX mode: %d\n", err);
>> + return err;
>> + }
>> + /* !1 is considered a fail by ASUS */
>> + if (result != 1) {
>> + pr_warn("Failed to set GPU MUX mode (result): 0x%x\n", result);
>> + return -EIO;
>> + }
>
> I wonder if the asus_wmi_set_devstate wrapper should also handle these
> error checks + prints. They seem pretty much duplicated except for the
> error string that could be provided as an argument.
For sure. The wrapper handles both err and result, and all this repeated code is removed.
>> +
>> + sysfs_notify(kobj, NULL, attr->attr.name);
>> + asus_set_reboot_and_signal_event();
>> +
>> + return count;
>> +}
>> +WMI_SHOW_INT(gpu_mux_mode_current_value, "%d\n", asus_armoury.gpu_mux_dev_id);
>> +ATTR_GROUP_BOOL_CUSTOM(gpu_mux_mode, "gpu_mux_mode", "Set the GPU display MUX mode");
>> +
>> +/*
>> + * A user may be required to store the value twice, typical store first, then
>> + * rescan PCI bus to activate power, then store a second time to save correctly.
>> + * The reason for this is that an extra code path in the ACPI is enabled when
>> + * the device and bus are powered.
>> + */
>> +static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
>> + struct kobj_attribute *attr, const char *buf,
>> + size_t count)
>> +{
>> + int result, err;
>> + u32 disable;
>> +
>> + err = kstrtou32(buf, 10, &disable);
>> + if (err)
>> + return err;
>> +
>> + if (disable > 1)
>> + return -EINVAL;
>> +
>> + if (asus_armoury.gpu_mux_dev_id) {
>> + err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
>> + if (err)
>> + return err;
>> + if (!result && disable) {
>> + err = -ENODEV;
>> + pr_warn("Can not disable dGPU when the MUX is in dGPU mode: %d\n", err);
>> + return err;
>> + }
>> + }
>> +
>> + mutex_lock(&asus_armoury.mutex);
>> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_DGPU, disable, &result);
>> + mutex_unlock(&asus_armoury.mutex);
>> + if (err) {
>> + pr_warn("Failed to set dGPU disable: %d\n", err);
>> + return err;
>> + }
>> +
>> + if (result != 1) {
>> + pr_warn("Failed to set dGPU disable (result): 0x%x\n", result);
>> + return -EIO;
>> + }
>> +
>> + sysfs_notify(kobj, NULL, attr->attr.name);
>> +
>> + return count;
>> +}
>> +WMI_SHOW_INT(dgpu_disable_current_value, "%d\n", ASUS_WMI_DEVID_DGPU);
>> +ATTR_GROUP_BOOL_CUSTOM(dgpu_disable, "dgpu_disable", "Disable the dGPU");
>> +
>> +/* The ACPI call to enable the eGPU also disables the internal dGPU */
>> +static ssize_t egpu_enable_current_value_store(struct kobject *kobj, struct kobj_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + int result, err;
>> + u32 enable;
>> +
>> + err = kstrtou32(buf, 10, &enable);
>> + if (err)
>> + return err;
>> +
>> + if (enable > 1)
>> + return -EINVAL;
>> +
>> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU_CONNECTED, &result);
>> + if (err) {
>> + pr_warn("Failed to get eGPU connection status: %d\n", err);
>> + return err;
>> + }
>> +
>> + if (asus_armoury.gpu_mux_dev_id) {
>> + err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
>> + if (err) {
>> + pr_warn("Failed to get GPU MUX status: %d\n", result);
>> + return result;
>> + }
>> + if (!result && enable) {
>> + err = -ENODEV;
>> + pr_warn("Can not enable eGPU when the MUX is in dGPU mode: %d\n", err);
>> + return err;
>> + }
>> + }
>> +
>> + mutex_lock(&asus_armoury.mutex);
>> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_EGPU, enable, &result);
>> + mutex_unlock(&asus_armoury.mutex);
>> + if (err) {
>> + pr_warn("Failed to set eGPU state: %d\n", err);
>> + return err;
>> + }
>> +
>> + if (result != 1) {
>> + pr_warn("Failed to set eGPU state (retval): 0x%x\n", result);
>> + return -EIO;
>> + }
>> +
>> + sysfs_notify(kobj, NULL, attr->attr.name);
>> +
>> + return count;
>> +}
>> +WMI_SHOW_INT(egpu_enable_current_value, "%d\n", ASUS_WMI_DEVID_EGPU);
>> +ATTR_GROUP_BOOL_CUSTOM(egpu_enable, "egpu_enable", "Enable the eGPU (also disables dGPU)");
>> +
>> +/* Simple attribute creation */
>> +ATTR_GROUP_ENUM_INT_RO(charge_mode, "charge_mode", ASUS_WMI_DEVID_CHARGE_MODE, "0;1;2",
>> + "Show the current mode of charging");
>> +
>> +ATTR_GROUP_BOOL_RW(boot_sound, "boot_sound", ASUS_WMI_DEVID_BOOT_SOUND,
>> + "Set the boot POST sound");
>> +ATTR_GROUP_BOOL_RW(mcu_powersave, "mcu_powersave", ASUS_WMI_DEVID_MCU_POWERSAVE,
>> + "Set MCU powersaving mode");
>> +ATTR_GROUP_BOOL_RW(panel_od, "panel_overdrive", ASUS_WMI_DEVID_PANEL_OD,
>> + "Set the panel refresh overdrive");
>> +ATTR_GROUP_BOOL_RO(egpu_connected, "egpu_connected", ASUS_WMI_DEVID_EGPU_CONNECTED,
>> + "Show the eGPU connection status");
>> +
>> +/* If an attribute does not require any special case handling add it here */
>> +static const struct asus_attr_group armoury_attr_groups[] = {
>> + { &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
>> + { &egpu_enable_attr_group, ASUS_WMI_DEVID_EGPU },
>> + { &dgpu_disable_attr_group, ASUS_WMI_DEVID_DGPU },
>> +
>> + { &charge_mode_attr_group, ASUS_WMI_DEVID_CHARGE_MODE },
>> + { &boot_sound_attr_group, ASUS_WMI_DEVID_BOOT_SOUND },
>> + { &mcu_powersave_attr_group, ASUS_WMI_DEVID_MCU_POWERSAVE },
>> + { &panel_od_attr_group, ASUS_WMI_DEVID_PANEL_OD },
>> +};
>> +
>> +static int asus_fw_attr_add(void)
>> +{
>> + int err, i;
>> +
>> + err = fw_attributes_class_get(&fw_attr_class);
>> + if (err)
>> + return err;
>> +
>> + asus_armoury.fw_attr_dev =
>> + device_create(fw_attr_class, NULL, MKDEV(0, 0), NULL, "%s", DRIVER_NAME);
>
> Don't split at = but after MKDEV() arg.
This is another clang-format thing... I'll bite the bullet and manually format for rest of style suggestions in review.
>> +
>> + if (IS_ERR(asus_armoury.fw_attr_dev)) {
>
> Don't leave empty line between call and it's error handling.
>
>> + err = PTR_ERR(asus_armoury.fw_attr_dev);
>> + goto fail_class_get;
>> + }
>> +
>> + asus_armoury.fw_attr_kset =
>> + kset_create_and_add("attributes", NULL, &asus_armoury.fw_attr_dev->kobj);
>
> Don't split at =
>
>> + if (!asus_armoury.fw_attr_kset) {
>> + err = -ENOMEM;
>> + goto err_destroy_classdev;
>> + }
>> +
>> + err = sysfs_create_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
>> + if (err) {
>> + pr_err("Failed to create sysfs level attributes\n");
>> + goto err_destroy_kset;
>> + }
>> +
>> + asus_armoury.mini_led_dev_id = 0;
>> + if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE)) {
>> + asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
>> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
>> + } else if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE2)) {
>> + asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE2;
>> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
>> + }
>> + if (err) {
>> + pr_err("Failed to create sysfs-group for mini_led\n");
>> + goto err_remove_file;
>> + }
>
> asus_armoury.mini_led_dev_id = 0;
> if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE))
> asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
> else if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE2))
> asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE2;
>
> if (asus_armoury.mini_led_dev_id) {
> err = sysfs_create_group(...);
> if (err) {
> ...
> }
> }
>
>> +
>> + asus_armoury.gpu_mux_dev_id = 0;
>> + if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX)) {
>> + asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX;
>> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
>> + } else if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX_VIVO)) {
>> + asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX_VIVO;
>> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
>> + }
>> + if (err) {
>> + pr_err("Failed to create sysfs-group for gpu_mux\n");
>> + goto err_remove_mini_led_group;
>> + }
>
> Same here.
gràzie
>
>> +
>> + for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
>> + if (!asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
>> + continue;
>> +
>> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
>> + armoury_attr_groups[i].attr_group);
>> + if (err) {
>> + pr_err("Failed to create sysfs-group for %s\n",
>> + armoury_attr_groups[i].attr_group->name);
>> + goto err_remove_groups;
>> + } else {
>
> No need to use else since you goto away from the error block.
ack
>> + pr_debug("Created sysfs-group for %s\n",
>> + armoury_attr_groups[i].attr_group->name);
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +err_remove_groups:
>> + while (--i >= 0) {
>> + if (asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
>> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, armoury_attr_groups[i].attr_group);
>> + }
>
>> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
>> +err_remove_mini_led_group:
>> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
>
> Do you need to check if these two got created before calling remove? IIRC,
> some sysfs remove function required that.
>
>> +err_remove_file:
>> + sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
>> +err_destroy_kset:
>> + kset_unregister(asus_armoury.fw_attr_kset);
>> +err_destroy_classdev:
>> + device_destroy(fw_attr_class, MKDEV(0, 0));
>> +fail_class_get:
>> + fw_attributes_class_put();
>> + return err;
>> +}
>> +
>> +/* Init / exit ****************************************************************/
>> +
>> +static int __init asus_fw_init(void)
>> +{
>> + char *wmi_uid;
>> + int err;
>> +
>> + wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
>> + if (!wmi_uid)
>> + return -ENODEV;
>> +
>> + /*
>> + * if equal to "ASUSWMI" then it's DCTS that can't be used for this
>> + * driver, DSTS is required.
>> + */
>> + if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI))
>> + return -ENODEV;
>> +
>> + err = asus_fw_attr_add();
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> +}
>> +
>> +static void __exit asus_fw_exit(void)
>> +{
>> + mutex_lock(&asus_armoury.mutex);
>
> I'm not sure if this really helps anything. What are you trying to protect
> against here with it?
I'm not even sure anymore. Was supposed to be due to my assumptions about how sysfs writes work.
One thing in particular is that the dgpu_disable and egpu_enable calls can take 20+ seconds to complete in acpi, and I don't want to make other WMI calls during that time - TBH I'm not sure of best way to handle it.
>> + sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
>> + kset_unregister(asus_armoury.fw_attr_kset);
>> + device_destroy(fw_attr_class, MKDEV(0, 0));
>> + fw_attributes_class_put();
>> +
>> + mutex_unlock(&asus_armoury.mutex);
>> +}
>> +
>> +module_init(asus_fw_init);
>> +module_exit(asus_fw_exit);
>> +
>> +MODULE_IMPORT_NS(ASUS_WMI);
>> +MODULE_AUTHOR("Luke Jones <luke@ljones.dev>");
>> +MODULE_DESCRIPTION("ASUS BIOS Configuration Driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("wmi:" ASUS_NB_WMI_EVENT_GUID);
>> diff --git a/drivers/platform/x86/asus-armoury.h b/drivers/platform/x86/asus-armoury.h
>> new file mode 100644
>> index 000000000000..4d0dd34c52aa
>> --- /dev/null
>> +++ b/drivers/platform/x86/asus-armoury.h
>> @@ -0,0 +1,146 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * Definitions for kernel modules using asus-armoury driver
>> + *
>> + * Copyright (c) 2024 Luke Jones <luke@ljones.dev>
>> + */
>> +
>> +#ifndef _ASUS_ARMOURY_H_
>> +#define _ASUS_ARMOURY_H_
>> +
>> +#include <linux/types.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define DRIVER_NAME "asus-armoury"
>> +
>> +static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr,
>> + const char *buf, size_t count, u32 min, u32 max,
>> + u32 *store_value, u32 wmi_dev);
>> +
>> +static ssize_t enum_type_show(struct kobject *kobj, struct kobj_attribute *attr,
>> + char *buf)
>> +{
>> + return sysfs_emit(buf, "enumeration\n");
>> +}
>> +
>> +#define __ASUS_ATTR_RO(_func, _name) \
>> + { \
>> + .attr = { .name = __stringify(_name), .mode = 0444 }, \
>> + .show = _func##_##_name##_show, \
>> + }
>
> Align all the continuation \ with tabs.
>
>> +#define __ASUS_ATTR_RO_AS(_name, _show) \
>> + { \
>> + .attr = { .name = __stringify(_name), .mode = 0444 }, \
>> + .show = _show, \
>> + }
>> +
>> +#define __ASUS_ATTR_RW(_func, _name) \
>> + __ATTR(_name, 0644, _func##_##_name##_show, _func##_##_name##_store)
>> +
>> +#define __WMI_STORE_INT(_attr, _min, _max, _wmi) \
>> + static ssize_t _attr##_store(struct kobject *kobj, \
>> + struct kobj_attribute *attr, \
>> + const char *buf, size_t count) \
>> + { \
>> + return attr_int_store(kobj, attr, buf, count, _min, _max, \
>> + NULL, _wmi); \
>> + }
>> +
>> +#define WMI_SHOW_INT(_attr, _fmt, _wmi) \
>> + static ssize_t _attr##_show(struct kobject *kobj, \
>> + struct kobj_attribute *attr, char *buf) \
>> + { \
>> + u32 result; \
>> + int err; \
>
> Empty line here as per the usual style, with the continuation \ obviously.
>
>> + err = asus_wmi_get_devstate_dsts(_wmi, &result); \
>> + if (err) \
>> + return err; \
>> + return sysfs_emit(buf, _fmt, \
>> + result & ~ASUS_WMI_DSTS_PRESENCE_BIT); \
>> + }
>
> I'll stop reviewing on this patch as I'm out of ability to focus. I'll
> take a look at the remaining patches later.
Understandable, thank you for your time so far.
Luke.
>
> --
> i.
>
>> +
>> +/* Create functions and attributes for use in other macros or on their own */
>> +
>> +#define __ATTR_CURRENT_INT_RO(_attr, _wmi) \
>> + WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
>> + static struct kobj_attribute attr_##_attr##_current_value = \
>> + __ASUS_ATTR_RO(_attr, current_value)
>> +
>> +#define __ATTR_CURRENT_INT_RW(_attr, _minv, _maxv, _wmi) \
>> + __WMI_STORE_INT(_attr##_current_value, _minv, _maxv, _wmi); \
>> + WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
>> + static struct kobj_attribute attr_##_attr##_current_value = \
>> + __ASUS_ATTR_RW(_attr, current_value)
>> +
>> +/* Shows a formatted static variable */
>> +#define __ATTR_SHOW_FMT(_prop, _attrname, _fmt, _val) \
>> + static ssize_t _attrname##_##_prop##_show( \
>> + struct kobject *kobj, struct kobj_attribute *attr, char *buf) \
>> + { \
>> + return sysfs_emit(buf, _fmt, _val); \
>> + } \
>> + static struct kobj_attribute attr_##_attrname##_##_prop = \
>> + __ASUS_ATTR_RO(_attrname, _prop)
>> +
>> +/* Boolean style enumeration, base macro. Requires adding show/store */
>> +#define __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname) \
>> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
>> + __ATTR_SHOW_FMT(possible_values, _attrname, "%s\n", _possible); \
>> + static struct kobj_attribute attr_##_attrname##_type = \
>> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
>> + static struct attribute *_attrname##_attrs[] = { \
>> + &attr_##_attrname##_current_value.attr, \
>> + &attr_##_attrname##_display_name.attr, \
>> + &attr_##_attrname##_possible_values.attr, \
>> + &attr_##_attrname##_type.attr, \
>> + NULL \
>> + }; \
>> + static const struct attribute_group _attrname##_attr_group = { \
>> + .name = _fsname, .attrs = _attrname##_attrs \
>> + }
>> +
>> +#define ATTR_GROUP_BOOL_RO(_attrname, _fsname, _wmi, _dispname) \
>> + __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
>> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
>> +
>> +#define ATTR_GROUP_BOOL_RW(_attrname, _fsname, _wmi, _dispname) \
>> + __ATTR_CURRENT_INT_RW(_attrname, 0, 1, _wmi); \
>> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
>> +
>> +/*
>> + * Requires <name>_current_value_show(), <name>_current_value_show()
>> + */
>> +#define ATTR_GROUP_BOOL_CUSTOM(_attrname, _fsname, _dispname) \
>> + static struct kobj_attribute attr_##_attrname##_current_value = \
>> + __ASUS_ATTR_RW(_attrname, current_value); \
>> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
>> +
>> +#define ATTR_GROUP_ENUM_INT_RO(_attrname, _fsname, _wmi, _possible, _dispname) \
>> + __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
>> + __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname)
>> +
>> +/*
>> + * Requires <name>_current_value_show(), <name>_current_value_show()
>> + * and <name>_possible_values_show()
>> + */
>> +#define ATTR_GROUP_ENUM_CUSTOM(_attrname, _fsname, _dispname) \
>> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
>> + static struct kobj_attribute attr_##_attrname##_current_value = \
>> + __ASUS_ATTR_RW(_attrname, current_value); \
>> + static struct kobj_attribute attr_##_attrname##_possible_values = \
>> + __ASUS_ATTR_RO(_attrname, possible_values); \
>> + static struct kobj_attribute attr_##_attrname##_type = \
>> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
>> + static struct attribute *_attrname##_attrs[] = { \
>> + &attr_##_attrname##_current_value.attr, \
>> + &attr_##_attrname##_display_name.attr, \
>> + &attr_##_attrname##_possible_values.attr, \
>> + &attr_##_attrname##_type.attr, \
>> + NULL \
>> + }; \
>> + static const struct attribute_group _attrname##_attr_group = { \
>> + .name = _fsname, .attrs = _attrname##_attrs \
>> + }
>> +
>> +#endif /* _ASUS_BIOSCFG_H_ */
>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>> index 0a5221d65130..3c6d774f4453 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -55,8 +55,6 @@ module_param(fnlock_default, bool, 0444);
>> #define to_asus_wmi_driver(pdrv) \
>> (container_of((pdrv), struct asus_wmi_driver, platform_driver))
>>
>> -#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
>> -
>> #define NOTIFY_BRNUP_MIN 0x11
>> #define NOTIFY_BRNUP_MAX 0x1f
>> #define NOTIFY_BRNDOWN_MIN 0x20
>> @@ -105,8 +103,6 @@ module_param(fnlock_default, bool, 0444);
>> #define USB_INTEL_XUSB2PR 0xD0
>> #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
>>
>> -#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
>> -
>> #define WMI_EVENT_MASK 0xFFFF
>>
>> #define FAN_CURVE_POINTS 8
>> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
>> index 6ea4dedfb85e..21313e1eb6c9 100644
>> --- a/include/linux/platform_data/x86/asus-wmi.h
>> +++ b/include/linux/platform_data/x86/asus-wmi.h
>> @@ -6,6 +6,9 @@
>> #include <linux/types.h>
>> #include <linux/dmi.h>
>>
>> +#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
>> +#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
>> +
>> /* WMI Methods */
>> #define ASUS_WMI_METHODID_SPEC 0x43455053 /* BIOS SPECification */
>> #define ASUS_WMI_METHODID_SFBD 0x44424653 /* Set First Boot Device */
>>
On Wed, 30 Oct 2024, Luke Jones wrote:
> On Wed, 16 Oct 2024, at 4:54 PM, Ilpo Järvinen wrote:
> > On Mon, 30 Sep 2024, Luke D. Jones wrote:
> >
> >> The fw_attributes_class provides a much cleaner interface to all of the
> >> attributes introduced to asus-wmi. This patch moves all of these extra
> >> attributes over to fw_attributes_class, and shifts the bulk of these
> >> definitions to a new kernel module to reduce the clutter of asus-wmi
> >> with the intention of deprecating the asus-wmi attributes in future.
> >>
> >> The work applies only to WMI methods which don't have a clearly defined
> >> place within the sysfs and as a result ended up lumped together in
> >> /sys/devices/platform/asus-nb-wmi/ with no standard API.
> >>
> >> Where possible the fw attrs now implement defaults, min, max, scalar,
> >> choices, etc. As en example dgpu_disable becomes:
> >>
> >> /sys/class/firmware-attributes/asus-armoury/attributes/dgpu_disable/
> >> ├── current_value
> >> ├── display_name
> >> ├── possible_values
> >> └── type
> >>
> >> as do other attributes.
> >>
> >> The ppt_* based attributes are removed in this initial patch as the
> >> implementation is somewhat broken due to the WMI methods requiring a
> >> set of limits on the values accepted (which is not provided by WMI).
> >>
> >> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> >> ---
> >> drivers/platform/x86/Kconfig | 12 +
> >> drivers/platform/x86/Makefile | 1 +
> >> drivers/platform/x86/asus-armoury.c | 593 +++++++++++++++++++++
> >> drivers/platform/x86/asus-armoury.h | 146 +++++
> >> drivers/platform/x86/asus-wmi.c | 4 -
> >> include/linux/platform_data/x86/asus-wmi.h | 3 +
> >> 6 files changed, 755 insertions(+), 4 deletions(-)
> >> create mode 100644 drivers/platform/x86/asus-armoury.c
> >> create mode 100644 drivers/platform/x86/asus-armoury.h
> >>
> >> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> >> index 3875abba5a79..80ec8b45022d 100644
> >> --- a/drivers/platform/x86/Kconfig
> >> +++ b/drivers/platform/x86/Kconfig
> >> @@ -265,6 +265,18 @@ config ASUS_WIRELESS
> >> If you choose to compile this driver as a module the module will be
> >> called asus-wireless.
> >>
> >> +config ASUS_ARMOURY
> >> + tristate "ASUS Armoury driver"
> >> + depends on ASUS_WMI
> >> + select FW_ATTR_CLASS
> >> + help
> >> + Say Y here if you have a WMI aware Asus machine and would like to use the
> >> + firmware_attributes API to control various settings typically exposed in
> >> + the ASUS Armoury Crate application available on Windows.
> >> +
> >> + To compile this driver as a module, choose M here: the module will
> >> + be called asus-armoury.
> >> +
> >> config ASUS_WMI
> >> tristate "ASUS WMI Driver"
> >> depends on ACPI_WMI
> >> +static const struct class *fw_attr_class;
> >> +
> >> +struct asus_armoury_priv {
> >> + struct device *fw_attr_dev;
> >> + struct kset *fw_attr_kset;
> >> +
> >> + u32 mini_led_dev_id;
> >> + u32 gpu_mux_dev_id;
> >> +
> >> + struct mutex mutex;
> >
> > Please document what this protects.
>
> I don't fully understand if sysfs writes can be parallel or not, so i
> was making the assumption that they were and if so, we would want to
> prevent trying to write many WMI at once. If my understanding is lacking
> please correct me.
Yes, sysfs write handlers can run parallel so locking necessary.
I believe your mutex is okay solution for concurrency control but you need
to add a comment on what it protects. It could be also named like
wmi_mutex if it's for that only.
> >> +static int asus_fw_attr_add(void)
> >> +{
> >> + int err, i;
> >> +
> >> + err = fw_attributes_class_get(&fw_attr_class);
> >> + if (err)
> >> + return err;
> >> +
> >> + asus_armoury.fw_attr_dev =
> >> + device_create(fw_attr_class, NULL, MKDEV(0, 0), NULL, "%s", DRIVER_NAME);
> >
> > Don't split at = but after MKDEV() arg.
>
> This is another clang-format thing... I'll bite the bullet and manually
> format for rest of style suggestions in review.
If you want to attempt to keep using it, I suggest you try to look if you
can make small adjustment to its config file to solve its worst misses.
> >> +static void __exit asus_fw_exit(void)
> >> +{
> >> + mutex_lock(&asus_armoury.mutex);
> >
> > I'm not sure if this really helps anything. What are you trying to protect
> > against here with it?
>
> I'm not even sure anymore. Was supposed to be due to my assumptions
> about how sysfs writes work.
It doesn't help any. Either you'd deadlock when calling
sysfs_remove_file() (assuming its waiting for the handlers to finish) or
the write handler is still executed after unlock which equals not taking
mutex here at all. I don't see what it could protect with any success.
> One thing in particular is that the dgpu_disable and egpu_enable calls
> can take 20+ seconds to complete in acpi, and I don't want to make other
> WMI calls during that time - TBH I'm not sure of best way to handle it.
>
> >> + sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> >> + kset_unregister(asus_armoury.fw_attr_kset);
> >> + device_destroy(fw_attr_class, MKDEV(0, 0));
> >> + fw_attributes_class_put();
> >> +
> >> + mutex_unlock(&asus_armoury.mutex);
> >> +}
--
i.
On 9/29/2024 19:00, Luke D. Jones wrote:
> The fw_attributes_class provides a much cleaner interface to all of the
> attributes introduced to asus-wmi. This patch moves all of these extra
> attributes over to fw_attributes_class, and shifts the bulk of these
> definitions to a new kernel module to reduce the clutter of asus-wmi
> with the intention of deprecating the asus-wmi attributes in future.
>
> The work applies only to WMI methods which don't have a clearly defined
> place within the sysfs and as a result ended up lumped together in
> /sys/devices/platform/asus-nb-wmi/ with no standard API.
>
> Where possible the fw attrs now implement defaults, min, max, scalar,
> choices, etc. As en example dgpu_disable becomes:
>
> /sys/class/firmware-attributes/asus-armoury/attributes/dgpu_disable/
> ├── current_value
> ├── display_name
> ├── possible_values
> └── type
>
> as do other attributes.
>
> The ppt_* based attributes are removed in this initial patch as the
> implementation is somewhat broken due to the WMI methods requiring a
> set of limits on the values accepted (which is not provided by WMI).
>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/platform/x86/Kconfig | 12 +
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/asus-armoury.c | 593 +++++++++++++++++++++
> drivers/platform/x86/asus-armoury.h | 146 +++++
> drivers/platform/x86/asus-wmi.c | 4 -
> include/linux/platform_data/x86/asus-wmi.h | 3 +
> 6 files changed, 755 insertions(+), 4 deletions(-)
> create mode 100644 drivers/platform/x86/asus-armoury.c
> create mode 100644 drivers/platform/x86/asus-armoury.h
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 3875abba5a79..80ec8b45022d 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -265,6 +265,18 @@ config ASUS_WIRELESS
> If you choose to compile this driver as a module the module will be
> called asus-wireless.
>
> +config ASUS_ARMOURY
> + tristate "ASUS Armoury driver"
> + depends on ASUS_WMI
> + select FW_ATTR_CLASS
> + help
> + Say Y here if you have a WMI aware Asus machine and would like to use the
> + firmware_attributes API to control various settings typically exposed in
> + the ASUS Armoury Crate application available on Windows.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called asus-armoury.
> +
> config ASUS_WMI
> tristate "ASUS WMI Driver"
> depends on ACPI_WMI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index e1b142947067..fe3e7e7dede8 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_APPLE_GMUX) += apple-gmux.o
> # ASUS
> obj-$(CONFIG_ASUS_LAPTOP) += asus-laptop.o
> obj-$(CONFIG_ASUS_WIRELESS) += asus-wireless.o
> +obj-$(CONFIG_ASUS_ARMOURY) += asus-armoury.o
> obj-$(CONFIG_ASUS_WMI) += asus-wmi.o
> obj-$(CONFIG_ASUS_NB_WMI) += asus-nb-wmi.o
> obj-$(CONFIG_ASUS_TF103C_DOCK) += asus-tf103c-dock.o
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> new file mode 100644
> index 000000000000..0085f1b112ab
> --- /dev/null
> +++ b/drivers/platform/x86/asus-armoury.c
> @@ -0,0 +1,593 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Asus Armoury (WMI) attributes driver. This driver uses the fw_attributes
> + * class to expose the various WMI functions that many gaming and some
> + * non-gaming ASUS laptops have available.
> + * These typically don't fit anywhere else in the sysfs such as under LED class,
> + * hwmon or other, and are set in Windows using the ASUS Armoury Crate tool.
> + *
> + * Copyright(C) 2010 Intel Corporation.
> + * Copyright(C) 2024-2024 Luke Jones <luke@ljones.dev>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/dmi.h>
> +#include <linux/errno.h>
> +#include <linux/fs.h>
> +#include <linux/kernel.h>
> +#include <linux/kmod.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_data/x86/asus-wmi.h>
> +#include <linux/types.h>
> +#include <linux/acpi.h>
> +
> +#include "asus-armoury.h"
> +#include "firmware_attributes_class.h"
> +
> +#define ASUS_NB_WMI_EVENT_GUID "0B3CBB35-E3C2-45ED-91C2-4C5A6D195D1C"
> +
> +#define ASUS_MINI_LED_MODE_MASK 0x03
> +/* Standard modes for devices with only on/off */
> +#define ASUS_MINI_LED_OFF 0x00
> +#define ASUS_MINI_LED_ON 0x01
> +/* Like "on" but the effect is more vibrant or brighter */
> +#define ASUS_MINI_LED_STRONG_MODE 0x02
> +/* New modes for devices with 3 mini-led mode types */
> +#define ASUS_MINI_LED_2024_WEAK 0x00
> +#define ASUS_MINI_LED_2024_STRONG 0x01
> +#define ASUS_MINI_LED_2024_OFF 0x02
> +
> +/* Default limits for tunables available on ASUS ROG laptops */
> +#define NVIDIA_BOOST_MIN 5
> +#define NVIDIA_BOOST_MAX 25
> +#define NVIDIA_TEMP_MIN 75
> +#define NVIDIA_TEMP_MAX 87
> +#define PPT_CPU_LIMIT_MIN 5
> +#define PPT_CPU_LIMIT_MAX 150
> +#define PPT_CPU_LIMIT_DEFAULT 80
> +#define PPT_PLATFORM_MIN 5
> +#define PPT_PLATFORM_MAX 100
> +#define PPT_PLATFORM_DEFAULT 80
> +
> +static const struct class *fw_attr_class;
> +
> +struct asus_armoury_priv {
> + struct device *fw_attr_dev;
> + struct kset *fw_attr_kset;
> +
> + u32 mini_led_dev_id;
> + u32 gpu_mux_dev_id;
> +
> + struct mutex mutex;
> +};
> +
> +static struct asus_armoury_priv asus_armoury = { .mutex = __MUTEX_INITIALIZER(
> + asus_armoury.mutex) };
> +
> +struct fw_attrs_group {
> + bool pending_reboot;
> +};
> +
> +static struct fw_attrs_group fw_attrs = {
> + .pending_reboot = false,
> +};
> +
> +struct asus_attr_group {
> + const struct attribute_group *attr_group;
> + u32 wmi_devid;
> +};
> +
> +static bool asus_wmi_is_present(u32 dev_id)
> +{
> + u32 retval;
> + int status;
> +
> + status = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, dev_id, 0, &retval);
> + pr_debug("%s called (0x%08x), retval: 0x%08x\n", __func__, dev_id, retval);
> +
> + return status == 0 && (retval & ASUS_WMI_DSTS_PRESENCE_BIT);
> +}
> +
> +static void asus_set_reboot_and_signal_event(void)
> +{
> + fw_attrs.pending_reboot = true;
> + kobject_uevent(&asus_armoury.fw_attr_dev->kobj, KOBJ_CHANGE);
> +}
> +
> +static ssize_t pending_reboot_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "%d\n", fw_attrs.pending_reboot);
> +}
> +
> +static struct kobj_attribute pending_reboot = __ATTR_RO(pending_reboot);
> +
> +static bool asus_bios_requires_reboot(struct kobj_attribute *attr)
> +{
> + return !strcmp(attr->attr.name, "gpu_mux_mode");
> +}
> +
> +/**
> + * attr_int_store() - Generic store function for use with most WMI functions.
> + * @kobj: Pointer to the driver object.
> + * @kobj_attribute: Pointer to the attribute calling this function.
> + * @buf: The buffer to read from, this is parsed to `int` type.
> + * @count: Required by sysfs attribute macros, pass in from the callee attr.
> + * @min: Minimum accepted value. Below this returns -EINVAL.
> + * @max: Maximum accepted value. Above this returns -EINVAL.
> + * @store_value: Pointer to where the parsed value should be stored.
> + * @wmi_dev: The WMI function ID to use.
> + *
> + * The WMI functions available on most ASUS laptops return a 1 as "success", and
> + * a 0 as failed. However some functions can return n > 1 for additional errors.
> + * The function currently treats all values which are not 1 as errors, ignoring
> + * the possible differences in WMI error returns.
> + *
> + * Returns: Either count, or an error.
> + */
> +static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf,
> + size_t count, u32 min, u32 max, u32 *store_value, u32 wmi_dev)
> +{
> + u32 result, value;
> + int err;
> +
> + err = kstrtouint(buf, 10, &value);
> + if (err)
> + return err;
> +
> + if (value < min || value > max)
> + return -EINVAL;
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(wmi_dev, value, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_err("Failed to set %s: %d\n", attr->attr.name, err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_err("Failed to set %s (result): 0x%x\n", attr->attr.name, result);
> + return -EIO;
> + }
> +
> + if (store_value != NULL)
> + *store_value = value;
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + if (asus_bios_requires_reboot(attr))
> + asus_set_reboot_and_signal_event();
> +
> + return count;
> +}
> +
> +/* Mini-LED mode **************************************************************/
> +static ssize_t mini_led_mode_current_value_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + u32 value;
> + int err;
> +
> + err = asus_wmi_get_devstate_dsts(asus_armoury.mini_led_dev_id, &value);
> + if (err)
> + return err;
> +
> + value &= ASUS_MINI_LED_MODE_MASK;
> +
> + /*
> + * Remap the mode values to match previous generation mini-LED. The last gen
> + * WMI 0 == off, while on this version WMI 2 == off (flipped).
> + */
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
> + switch (value) {
> + case ASUS_MINI_LED_2024_WEAK:
> + value = ASUS_MINI_LED_ON;
> + break;
> + case ASUS_MINI_LED_2024_STRONG:
> + value = ASUS_MINI_LED_STRONG_MODE;
> + break;
> + case ASUS_MINI_LED_2024_OFF:
> + value = ASUS_MINI_LED_OFF;
> + break;
> + }
> + }
> +
> + return sysfs_emit(buf, "%u\n", value);
> +}
> +
> +static ssize_t mini_led_mode_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int result, err;
> + u32 mode;
> +
> + err = kstrtou32(buf, 10, &mode);
> + if (err)
> + return err;
> +
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE &&
> + mode > ASUS_MINI_LED_ON)
> + return -EINVAL;
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2 &&
> + mode > ASUS_MINI_LED_STRONG_MODE)
> + return -EINVAL;
> +
> + /*
> + * Remap the mode values so expected behaviour is the same as the last
> + * generation of mini-LED with 0 == off, 1 == on.
> + */
> + if (asus_armoury.mini_led_dev_id == ASUS_WMI_DEVID_MINI_LED_MODE2) {
> + switch (mode) {
> + case ASUS_MINI_LED_OFF:
> + mode = ASUS_MINI_LED_2024_OFF;
> + break;
> + case ASUS_MINI_LED_ON:
> + mode = ASUS_MINI_LED_2024_WEAK;
> + break;
> + case ASUS_MINI_LED_STRONG_MODE:
> + mode = ASUS_MINI_LED_2024_STRONG;
> + break;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(asus_armoury.mini_led_dev_id, mode, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_warn("Failed to set mini-LED: %d\n", err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_warn("Failed to set mini-LED mode (result): 0x%x\n", result);
> + return -EIO;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + return count;
> +}
> +
> +static ssize_t mini_led_mode_possible_values_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + switch (asus_armoury.mini_led_dev_id) {
> + case ASUS_WMI_DEVID_MINI_LED_MODE:
> + return sysfs_emit(buf, "0;1\n");
> + case ASUS_WMI_DEVID_MINI_LED_MODE2:
> + return sysfs_emit(buf, "0;1;2\n");
> + }
> +
> + return sysfs_emit(buf, "0\n");
> +}
> +
> +ATTR_GROUP_ENUM_CUSTOM(mini_led_mode, "mini_led_mode", "Set the mini-LED backlight mode");
> +
> +static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int result, err;
> + u32 optimus;
> +
> + err = kstrtou32(buf, 10, &optimus);
> + if (err)
> + return err;
> +
> + if (optimus > 1)
> + return -EINVAL;
> +
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_DGPU)) {
> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_DGPU, &result);
> + if (err)
> + return err;
> + if (result && !optimus) {
> + err = -ENODEV;
> + pr_warn("Can not switch MUX to dGPU mode when dGPU is disabled: %02X %02X %d\n",
> + result, optimus, err);
> + return err;
> + }
> + }
> +
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_EGPU)) {
> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU, &result);
> + if (err)
> + return err;
> + if (result && !optimus) {
> + err = -ENODEV;
> + pr_warn("Can not switch MUX to dGPU mode when eGPU is enabled: %d\n",
> + err);
> + return err;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(asus_armoury.gpu_mux_dev_id, optimus, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_err("Failed to set GPU MUX mode: %d\n", err);
> + return err;
> + }
> + /* !1 is considered a fail by ASUS */
> + if (result != 1) {
> + pr_warn("Failed to set GPU MUX mode (result): 0x%x\n", result);
> + return -EIO;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> + asus_set_reboot_and_signal_event();
> +
> + return count;
> +}
> +WMI_SHOW_INT(gpu_mux_mode_current_value, "%d\n", asus_armoury.gpu_mux_dev_id);
> +ATTR_GROUP_BOOL_CUSTOM(gpu_mux_mode, "gpu_mux_mode", "Set the GPU display MUX mode");
> +
> +/*
> + * A user may be required to store the value twice, typical store first, then
> + * rescan PCI bus to activate power, then store a second time to save correctly.
> + * The reason for this is that an extra code path in the ACPI is enabled when
> + * the device and bus are powered.
> + */
> +static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int result, err;
> + u32 disable;
> +
> + err = kstrtou32(buf, 10, &disable);
> + if (err)
> + return err;
> +
> + if (disable > 1)
> + return -EINVAL;
> +
> + if (asus_armoury.gpu_mux_dev_id) {
> + err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
> + if (err)
> + return err;
> + if (!result && disable) {
> + err = -ENODEV;
> + pr_warn("Can not disable dGPU when the MUX is in dGPU mode: %d\n", err);
> + return err;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_DGPU, disable, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_warn("Failed to set dGPU disable: %d\n", err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_warn("Failed to set dGPU disable (result): 0x%x\n", result);
> + return -EIO;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + return count;
> +}
> +WMI_SHOW_INT(dgpu_disable_current_value, "%d\n", ASUS_WMI_DEVID_DGPU);
> +ATTR_GROUP_BOOL_CUSTOM(dgpu_disable, "dgpu_disable", "Disable the dGPU");
> +
> +/* The ACPI call to enable the eGPU also disables the internal dGPU */
> +static ssize_t egpu_enable_current_value_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int result, err;
> + u32 enable;
> +
> + err = kstrtou32(buf, 10, &enable);
> + if (err)
> + return err;
> +
> + if (enable > 1)
> + return -EINVAL;
> +
> + err = asus_wmi_get_devstate_dsts(ASUS_WMI_DEVID_EGPU_CONNECTED, &result);
> + if (err) {
> + pr_warn("Failed to get eGPU connection status: %d\n", err);
> + return err;
> + }
> +
> + if (asus_armoury.gpu_mux_dev_id) {
> + err = asus_wmi_get_devstate_dsts(asus_armoury.gpu_mux_dev_id, &result);
> + if (err) {
> + pr_warn("Failed to get GPU MUX status: %d\n", result);
> + return result;
> + }
> + if (!result && enable) {
> + err = -ENODEV;
> + pr_warn("Can not enable eGPU when the MUX is in dGPU mode: %d\n", err);
> + return err;
> + }
> + }
> +
> + mutex_lock(&asus_armoury.mutex);
> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_EGPU, enable, &result);
> + mutex_unlock(&asus_armoury.mutex);
> + if (err) {
> + pr_warn("Failed to set eGPU state: %d\n", err);
> + return err;
> + }
> +
> + if (result != 1) {
> + pr_warn("Failed to set eGPU state (retval): 0x%x\n", result);
> + return -EIO;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> +
> + return count;
> +}
> +WMI_SHOW_INT(egpu_enable_current_value, "%d\n", ASUS_WMI_DEVID_EGPU);
> +ATTR_GROUP_BOOL_CUSTOM(egpu_enable, "egpu_enable", "Enable the eGPU (also disables dGPU)");
> +
> +/* Simple attribute creation */
> +ATTR_GROUP_ENUM_INT_RO(charge_mode, "charge_mode", ASUS_WMI_DEVID_CHARGE_MODE, "0;1;2",
> + "Show the current mode of charging");
> +
> +ATTR_GROUP_BOOL_RW(boot_sound, "boot_sound", ASUS_WMI_DEVID_BOOT_SOUND,
> + "Set the boot POST sound");
> +ATTR_GROUP_BOOL_RW(mcu_powersave, "mcu_powersave", ASUS_WMI_DEVID_MCU_POWERSAVE,
> + "Set MCU powersaving mode");
> +ATTR_GROUP_BOOL_RW(panel_od, "panel_overdrive", ASUS_WMI_DEVID_PANEL_OD,
> + "Set the panel refresh overdrive");
> +ATTR_GROUP_BOOL_RO(egpu_connected, "egpu_connected", ASUS_WMI_DEVID_EGPU_CONNECTED,
> + "Show the eGPU connection status");
> +
> +/* If an attribute does not require any special case handling add it here */
> +static const struct asus_attr_group armoury_attr_groups[] = {
> + { &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
> + { &egpu_enable_attr_group, ASUS_WMI_DEVID_EGPU },
> + { &dgpu_disable_attr_group, ASUS_WMI_DEVID_DGPU },
> +
> + { &charge_mode_attr_group, ASUS_WMI_DEVID_CHARGE_MODE },
> + { &boot_sound_attr_group, ASUS_WMI_DEVID_BOOT_SOUND },
> + { &mcu_powersave_attr_group, ASUS_WMI_DEVID_MCU_POWERSAVE },
> + { &panel_od_attr_group, ASUS_WMI_DEVID_PANEL_OD },
> +};
> +
> +static int asus_fw_attr_add(void)
> +{
> + int err, i;
> +
> + err = fw_attributes_class_get(&fw_attr_class);
> + if (err)
> + return err;
> +
> + asus_armoury.fw_attr_dev =
> + device_create(fw_attr_class, NULL, MKDEV(0, 0), NULL, "%s", DRIVER_NAME);
> +
> + if (IS_ERR(asus_armoury.fw_attr_dev)) {
> + err = PTR_ERR(asus_armoury.fw_attr_dev);
> + goto fail_class_get;
> + }
> +
> + asus_armoury.fw_attr_kset =
> + kset_create_and_add("attributes", NULL, &asus_armoury.fw_attr_dev->kobj);
> + if (!asus_armoury.fw_attr_kset) {
> + err = -ENOMEM;
> + goto err_destroy_classdev;
> + }
> +
> + err = sysfs_create_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> + if (err) {
> + pr_err("Failed to create sysfs level attributes\n");
> + goto err_destroy_kset;
> + }
> +
> + asus_armoury.mini_led_dev_id = 0;
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE)) {
> + asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> + } else if (asus_wmi_is_present(ASUS_WMI_DEVID_MINI_LED_MODE2)) {
> + asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE2;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> + }
> + if (err) {
> + pr_err("Failed to create sysfs-group for mini_led\n");
> + goto err_remove_file;
> + }
> +
> + asus_armoury.gpu_mux_dev_id = 0;
> + if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX)) {
> + asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> + } else if (asus_wmi_is_present(ASUS_WMI_DEVID_GPU_MUX_VIVO)) {
> + asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX_VIVO;
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> + }
> + if (err) {
> + pr_err("Failed to create sysfs-group for gpu_mux\n");
> + goto err_remove_mini_led_group;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
> + if (!asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
> + continue;
> +
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> + armoury_attr_groups[i].attr_group);
> + if (err) {
> + pr_err("Failed to create sysfs-group for %s\n",
> + armoury_attr_groups[i].attr_group->name);
> + goto err_remove_groups;
> + } else {
> + pr_debug("Created sysfs-group for %s\n",
> + armoury_attr_groups[i].attr_group->name);
> + }
> + }
> +
> + return 0;
> +
> +err_remove_groups:
> + while (--i >= 0) {
> + if (asus_wmi_is_present(armoury_attr_groups[i].wmi_devid))
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, armoury_attr_groups[i].attr_group);
> + }
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> +err_remove_mini_led_group:
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> +err_remove_file:
> + sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> +err_destroy_kset:
> + kset_unregister(asus_armoury.fw_attr_kset);
> +err_destroy_classdev:
> + device_destroy(fw_attr_class, MKDEV(0, 0));
> +fail_class_get:
> + fw_attributes_class_put();
> + return err;
> +}
> +
> +/* Init / exit ****************************************************************/
> +
> +static int __init asus_fw_init(void)
> +{
> + char *wmi_uid;
> + int err;
> +
> + wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
> + if (!wmi_uid)
> + return -ENODEV;
> +
> + /*
> + * if equal to "ASUSWMI" then it's DCTS that can't be used for this
> + * driver, DSTS is required.
> + */
> + if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI))
> + return -ENODEV;
> +
> + err = asus_fw_attr_add();
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static void __exit asus_fw_exit(void)
> +{
> + mutex_lock(&asus_armoury.mutex);
> +
> + sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> + kset_unregister(asus_armoury.fw_attr_kset);
> + device_destroy(fw_attr_class, MKDEV(0, 0));
> + fw_attributes_class_put();
> +
> + mutex_unlock(&asus_armoury.mutex);
> +}
> +
> +module_init(asus_fw_init);
> +module_exit(asus_fw_exit);
> +
> +MODULE_IMPORT_NS(ASUS_WMI);
> +MODULE_AUTHOR("Luke Jones <luke@ljones.dev>");
> +MODULE_DESCRIPTION("ASUS BIOS Configuration Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("wmi:" ASUS_NB_WMI_EVENT_GUID);
> diff --git a/drivers/platform/x86/asus-armoury.h b/drivers/platform/x86/asus-armoury.h
> new file mode 100644
> index 000000000000..4d0dd34c52aa
> --- /dev/null
> +++ b/drivers/platform/x86/asus-armoury.h
> @@ -0,0 +1,146 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Definitions for kernel modules using asus-armoury driver
> + *
> + * Copyright (c) 2024 Luke Jones <luke@ljones.dev>
> + */
> +
> +#ifndef _ASUS_ARMOURY_H_
> +#define _ASUS_ARMOURY_H_
> +
> +#include <linux/types.h>
> +#include <linux/platform_device.h>
> +
> +#define DRIVER_NAME "asus-armoury"
> +
> +static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count, u32 min, u32 max,
> + u32 *store_value, u32 wmi_dev);
> +
> +static ssize_t enum_type_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "enumeration\n");
> +}
> +
> +#define __ASUS_ATTR_RO(_func, _name) \
> + { \
> + .attr = { .name = __stringify(_name), .mode = 0444 }, \
> + .show = _func##_##_name##_show, \
> + }
> +
> +#define __ASUS_ATTR_RO_AS(_name, _show) \
> + { \
> + .attr = { .name = __stringify(_name), .mode = 0444 }, \
> + .show = _show, \
> + }
> +
> +#define __ASUS_ATTR_RW(_func, _name) \
> + __ATTR(_name, 0644, _func##_##_name##_show, _func##_##_name##_store)
> +
> +#define __WMI_STORE_INT(_attr, _min, _max, _wmi) \
> + static ssize_t _attr##_store(struct kobject *kobj, \
> + struct kobj_attribute *attr, \
> + const char *buf, size_t count) \
> + { \
> + return attr_int_store(kobj, attr, buf, count, _min, _max, \
> + NULL, _wmi); \
> + }
> +
> +#define WMI_SHOW_INT(_attr, _fmt, _wmi) \
> + static ssize_t _attr##_show(struct kobject *kobj, \
> + struct kobj_attribute *attr, char *buf) \
> + { \
> + u32 result; \
> + int err; \
> + err = asus_wmi_get_devstate_dsts(_wmi, &result); \
> + if (err) \
> + return err; \
> + return sysfs_emit(buf, _fmt, \
> + result & ~ASUS_WMI_DSTS_PRESENCE_BIT); \
> + }
> +
> +/* Create functions and attributes for use in other macros or on their own */
> +
> +#define __ATTR_CURRENT_INT_RO(_attr, _wmi) \
> + WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
> + static struct kobj_attribute attr_##_attr##_current_value = \
> + __ASUS_ATTR_RO(_attr, current_value)
> +
> +#define __ATTR_CURRENT_INT_RW(_attr, _minv, _maxv, _wmi) \
> + __WMI_STORE_INT(_attr##_current_value, _minv, _maxv, _wmi); \
> + WMI_SHOW_INT(_attr##_current_value, "%d\n", _wmi); \
> + static struct kobj_attribute attr_##_attr##_current_value = \
> + __ASUS_ATTR_RW(_attr, current_value)
> +
> +/* Shows a formatted static variable */
> +#define __ATTR_SHOW_FMT(_prop, _attrname, _fmt, _val) \
> + static ssize_t _attrname##_##_prop##_show( \
> + struct kobject *kobj, struct kobj_attribute *attr, char *buf) \
> + { \
> + return sysfs_emit(buf, _fmt, _val); \
> + } \
> + static struct kobj_attribute attr_##_attrname##_##_prop = \
> + __ASUS_ATTR_RO(_attrname, _prop)
> +
> +/* Boolean style enumeration, base macro. Requires adding show/store */
> +#define __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname) \
> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
> + __ATTR_SHOW_FMT(possible_values, _attrname, "%s\n", _possible); \
> + static struct kobj_attribute attr_##_attrname##_type = \
> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
> + static struct attribute *_attrname##_attrs[] = { \
> + &attr_##_attrname##_current_value.attr, \
> + &attr_##_attrname##_display_name.attr, \
> + &attr_##_attrname##_possible_values.attr, \
> + &attr_##_attrname##_type.attr, \
> + NULL \
> + }; \
> + static const struct attribute_group _attrname##_attr_group = { \
> + .name = _fsname, .attrs = _attrname##_attrs \
> + }
> +
> +#define ATTR_GROUP_BOOL_RO(_attrname, _fsname, _wmi, _dispname) \
> + __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
> +
> +#define ATTR_GROUP_BOOL_RW(_attrname, _fsname, _wmi, _dispname) \
> + __ATTR_CURRENT_INT_RW(_attrname, 0, 1, _wmi); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
> +
> +/*
> + * Requires <name>_current_value_show(), <name>_current_value_show()
> + */
> +#define ATTR_GROUP_BOOL_CUSTOM(_attrname, _fsname, _dispname) \
> + static struct kobj_attribute attr_##_attrname##_current_value = \
> + __ASUS_ATTR_RW(_attrname, current_value); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, "0;1", _dispname)
> +
> +#define ATTR_GROUP_ENUM_INT_RO(_attrname, _fsname, _wmi, _possible, _dispname) \
> + __ATTR_CURRENT_INT_RO(_attrname, _wmi); \
> + __ATTR_GROUP_ENUM(_attrname, _fsname, _possible, _dispname)
> +
> +/*
> + * Requires <name>_current_value_show(), <name>_current_value_show()
> + * and <name>_possible_values_show()
> + */
> +#define ATTR_GROUP_ENUM_CUSTOM(_attrname, _fsname, _dispname) \
> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
> + static struct kobj_attribute attr_##_attrname##_current_value = \
> + __ASUS_ATTR_RW(_attrname, current_value); \
> + static struct kobj_attribute attr_##_attrname##_possible_values = \
> + __ASUS_ATTR_RO(_attrname, possible_values); \
> + static struct kobj_attribute attr_##_attrname##_type = \
> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
> + static struct attribute *_attrname##_attrs[] = { \
> + &attr_##_attrname##_current_value.attr, \
> + &attr_##_attrname##_display_name.attr, \
> + &attr_##_attrname##_possible_values.attr, \
> + &attr_##_attrname##_type.attr, \
> + NULL \
> + }; \
> + static const struct attribute_group _attrname##_attr_group = { \
> + .name = _fsname, .attrs = _attrname##_attrs \
> + }
> +
> +#endif /* _ASUS_BIOSCFG_H_ */
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 0a5221d65130..3c6d774f4453 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -55,8 +55,6 @@ module_param(fnlock_default, bool, 0444);
> #define to_asus_wmi_driver(pdrv) \
> (container_of((pdrv), struct asus_wmi_driver, platform_driver))
>
> -#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
> -
> #define NOTIFY_BRNUP_MIN 0x11
> #define NOTIFY_BRNUP_MAX 0x1f
> #define NOTIFY_BRNDOWN_MIN 0x20
> @@ -105,8 +103,6 @@ module_param(fnlock_default, bool, 0444);
> #define USB_INTEL_XUSB2PR 0xD0
> #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
>
> -#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
> -
> #define WMI_EVENT_MASK 0xFFFF
>
> #define FAN_CURVE_POINTS 8
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 6ea4dedfb85e..21313e1eb6c9 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -6,6 +6,9 @@
> #include <linux/types.h>
> #include <linux/dmi.h>
>
> +#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
> +#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
> +
> /* WMI Methods */
> #define ASUS_WMI_METHODID_SPEC 0x43455053 /* BIOS SPECification */
> #define ASUS_WMI_METHODID_SFBD 0x44424653 /* Set First Boot Device */
© 2016 - 2024 Red Hat, Inc.