Re: [PATCH 2/4] platform/x86/uniwill: Implement USB-C power priority setting
From: Werner Sembach
Date: Fri Feb 27 2026 - 13:40:56 EST
Am 27.02.26 um 10:27 schrieb Armin Wolf:
Am 26.02.26 um 16:55 schrieb Werner Sembach:ok
Sorry, i meant USB_C_POWER_PRIORITY_OPTIONS_TEXT[].
Am 26.02.26 um 02:11 schrieb Armin Wolf:
Am 26.02.26 um 01:31 schrieb Werner Sembach:ack
On some devices Uniwill offers the option to set the USB-C port to
prioritise charging or performance. This patch exposes this setting to the
userspace via sysfs for all TUXEDO devices supporting it.
Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
---
drivers/platform/x86/uniwill/uniwill-acpi.c | 99 +++++++++++++++++++--
1 file changed, 94 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index 7ab82cf16f388..4d9657a3b8c10 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -266,8 +266,8 @@
#define BATTERY_CHARGE_FULL_OVER_24H BIT(3)
#define BATTERY_ERM_STATUS_REACHED BIT(4)
-#define EC_ADDR_CHARGE_PRIO 0x07CC
-#define CHARGING_PERFORMANCE BIT(7)
+#define EC_ADDR_USB_C_POWER_PRIORITY 0x07CC
+#define USB_C_POWER_PRIORITY BIT(7)
/* Same bits as EC_ADDR_LIGHTBAR_AC_CTRL except LIGHTBAR_S3_OFF */
#define EC_ADDR_LIGHTBAR_BAT_CTRL 0x07E2
@@ -324,6 +324,7 @@
#define UNIWILL_FEATURE_PRIMARY_FAN BIT(7)
#define UNIWILL_FEATURE_SECONDARY_FAN BIT(8)
#define UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL BIT(9)
+#define UNIWILL_FEATURE_USB_C_POWER_PRIORITY BIT(10)
struct uniwill_data {
struct device *dev;
@@ -343,6 +344,7 @@ struct uniwill_data {
struct mutex input_lock; /* Protects input sequence during notify */
struct input_dev *input_device;
struct notifier_block nb;
+ unsigned int last_usb_c_power_priority;
};
struct uniwill_battery_entry {
@@ -527,6 +529,7 @@ static bool uniwill_writeable_reg(struct device *dev, unsigned int reg)
case EC_ADDR_CTGP_DB_CTGP_OFFSET:
case EC_ADDR_CTGP_DB_TPP_OFFSET:
case EC_ADDR_CTGP_DB_DB_OFFSET:
+ case EC_ADDR_USB_C_POWER_PRIORITY:
return true;
default:
return false;
@@ -565,6 +568,7 @@ static bool uniwill_readable_reg(struct device *dev, unsigned int reg)
case EC_ADDR_CTGP_DB_CTGP_OFFSET:
case EC_ADDR_CTGP_DB_TPP_OFFSET:
case EC_ADDR_CTGP_DB_DB_OFFSET:
+ case EC_ADDR_USB_C_POWER_PRIORITY:
return true;
default:
return false;
@@ -587,6 +591,7 @@ static bool uniwill_volatile_reg(struct device *dev, unsigned int reg)
case EC_ADDR_TRIGGER:
case EC_ADDR_SWITCH_STATUS:
case EC_ADDR_CHARGE_CTRL:
+ case EC_ADDR_USB_C_POWER_PRIORITY:
return true;
default:
return false;
@@ -883,6 +888,86 @@ static int uniwill_nvidia_ctgp_init(struct uniwill_data *data)
return 0;
}
+enum usb_c_power_priority_options {
+ USB_C_POWER_PRIORITY_OPTIONS_CHARGING = 0,
+ USB_C_POWER_PRIORITY_OPTIONS_PERFORMANCE,
+};
+
+static const char * const USB_C_POWER_PRIORITY_OPTIONS_TEXT[] = {
+ [USB_C_POWER_PRIORITY_OPTIONS_CHARGING] = "charging",
+ [USB_C_POWER_PRIORITY_OPTIONS_PERFORMANCE] = "performance",
+};
+
+static const u8 USB_C_POWER_PRIORITY_OPTIONS_VALUE[] = {
+ [USB_C_POWER_PRIORITY_OPTIONS_CHARGING] = 0,
+ [USB_C_POWER_PRIORITY_OPTIONS_PERFORMANCE] = USB_C_POWER_PRIORITY,
+};
+
+static ssize_t usb_c_power_priority_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct uniwill_data *data = dev_get_drvdata(dev);
+ unsigned int value;
+ int ret;
+
+ ret = sysfs_match_string(USB_C_POWER_PRIORITY_OPTIONS_TEXT, buf);
+ if (ret < 0)
+ return ret;
Please put a blank line here.
ack
+ value = USB_C_POWER_PRIORITY_OPTIONS_VALUE[ret];
+
+ ret = regmap_update_bits(data->regmap, EC_ADDR_USB_C_POWER_PRIORITY,
+ USB_C_POWER_PRIORITY, value);
+ if (ret < 0)
+ return ret;
+
I think you need a mutex here, so concurrent writes to the charging priority
sysfs attribute are serialized. This mutex would then also protect concurrent
access to data->last_usb_c_power_priority by the event handler.
+ data->last_usb_c_power_priority = value;
+
+ return count;
+}
+
+static ssize_t usb_c_power_priority_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct uniwill_data *data = dev_get_drvdata(dev);
+ unsigned int value;
+ ssize_t count = 0;
+ int ret;
+
+ ret = regmap_read(data->regmap, EC_ADDR_USB_C_POWER_PRIORITY, &value);
+ if (ret < 0)
+ return ret;
+ value &= USB_C_POWER_PRIORITY;
+
+ for (int i = 0; i < ARRAY_SIZE(USB_C_POWER_PRIORITY_OPTIONS_VALUE); ++i) {
+ if (USB_C_POWER_PRIORITY_OPTIONS_VALUE[i] == value)
+ count += sysfs_emit_at(buf, count, "[%s] ",
+ USB_C_POWER_PRIORITY_OPTIONS_TEXT[i]);
+ else
+ count += sysfs_emit_at(buf, count, "%s ",
+ USB_C_POWER_PRIORITY_OPTIONS_TEXT[i]);
+ }
+ if (count)
+ buf[count - 1] = '\n';
Please just emit USB_C_POWER_PRIORITY_OPTIONS_VALUE[USB_C_POWER_PRIORITY_OPTIONS_PERFORMANCE] directly
when the bit is set, and USB_C_POWER_PRIORITY_OPTIONS_VALUE[USB_C_POWER_PRIORITY_OPTIONS_CHARGING] otherwise.
This for loop provides no real value.
It does give context on which values are available. It it is otherwise not easily deduceable without referring to the documentation.
Also USB_C_POWER_PRIORITY_OPTIONS_VALUE[USB_C_POWER_PRIORITY_OPTIONS_PERFORMANCE] is BIT(7) which is a kinda random number when expressed as an integer.
I do not think that using the "option1 [option2]" syntax provides any real benefit here. The users need to read the documentation
anyway in order to even find this attribute, and the set of available options is always the same.
Please drop this.
ok, suspend is actually not necessary because last value is already saved on write
ack
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(usb_c_power_priority);
+
+static int usb_c_power_priority_restore(struct uniwill_data *data)
+{
+ int ret;
+
+ ret = regmap_update_bits(data->regmap, EC_ADDR_OEM_4, USB_C_POWER_PRIORITY,
+ data->last_usb_c_power_priority);
+ if (ret < 0)
+ return ret;
+
+ return ret;
Please check the feature flag here before performing the register access. You should also
omit "ret" and instead return the result directly.
tuxedo-drivers doesn't have it for this, so it's probably not necessary
Maybe you also need to add the appropriate suspend/resume functions for this?
Please add the appropriate suspend/resume functions anyway, just in case the user has
connected the charger while the system was suspended.
Thanks,
Armin Wolf
ack
+}
+
static struct attribute *uniwill_attrs[] = {
/* Keyboard-related */
&dev_attr_fn_lock.attr,
@@ -893,6 +978,7 @@ static struct attribute *uniwill_attrs[] = {
&dev_attr_breathing_in_suspend.attr,
/* Power-management-related */
&dev_attr_ctgp_offset.attr,
+ &dev_attr_usb_c_power_priority.attr,
NULL
};
@@ -927,6 +1013,11 @@ static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct attribute *a
return attr->mode;
}
+ if (attr == &dev_attr_usb_c_power_priority.attr) {
+ if (uniwill_device_supports(data, UNIWILL_FEATURE_USB_C_POWER_PRIORITY))
+ return attr->mode;
+ }
+
return 0;
}
@@ -1417,9 +1508,7 @@ static int uniwill_notifier_call(struct notifier_block *nb, unsigned long action
return NOTIFY_OK;
case UNIWILL_OSD_DC_ADAPTER_CHANGED:
- /* noop for the time being, will change once charging priority
- * gets implemented.
- */
+ usb_c_power_priority_restore(data);
Please use notifier_from_errno() here.
Thanks,
Armin Wolf
return NOTIFY_OK;
case UNIWILL_OSD_FN_LOCK: