Re: [PATCH 1/2] platform/x86: uniwill-laptop: Introduce device descriptor system

From: Werner Sembach
Date: Tue Dec 16 2025 - 10:30:35 EST

Next message: kernel test robot: "Re: [PATCH v2 3/3] randomize_kstack: Unify random source across arches"
Previous message: Tomi Valkeinen: "[PATCH v4 00/15] media: rcar: Streams support"
In reply to: Ilpo Järvinen: "Re: [PATCH 1/2] platform/x86: uniwill-laptop: Introduce device descriptor system"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Am 16.12.25 um 16:20 schrieb Ilpo Järvinen:

On Tue, 16 Dec 2025, Werner Sembach wrote:

Am 16.12.25 um 14:40 schrieb Ilpo Järvinen:

On Thu, 4 Dec 2025, Werner Sembach wrote:

From: Armin Wolf <W_Armin@xxxxxx>

Future additions to the driver will depend on device-specific
initialization steps. Extend the DMI-based feature detection system
to include device descriptors. Each descriptor contains a bitmap of
supported features and a set of callback for performing
device-specific initialization.

Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
Co-developed-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
---
drivers/platform/x86/uniwill/uniwill-acpi.c | 168 +++++++++++++++++---
1 file changed, 142 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c
b/drivers/platform/x86/uniwill/uniwill-acpi.c
index bd7e63dd51810..01192c32608e5 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -322,6 +322,7 @@ struct uniwill_data {
struct device *dev;
acpi_handle handle;
struct regmap *regmap;
+ unsigned int features;
struct acpi_battery_hook hook;
unsigned int last_charge_ctrl;
struct mutex battery_lock; /* Protects the list of currently
registered batteries */
@@ -341,12 +342,21 @@ struct uniwill_battery_entry {
struct power_supply *battery;
};
+struct uniwill_device_descriptor {
+ unsigned int features;
+ /* Executed during driver probing */
+ int (*probe)(struct uniwill_data *data);
+};
+
static bool force;
module_param_unsafe(force, bool, 0);
MODULE_PARM_DESC(force, "Force loading without checking for supported
devices\n");
-/* Feature bitmask since the associated registers are not reliable */
-static unsigned int supported_features;
+/*
+ * Contains device specific data like the feature bitmap since
+ * the associated registers are not always reliable.
+ */
+static struct uniwill_device_descriptor device_descriptor
__ro_after_init;
static const char * const uniwill_temp_labels[] = {
"CPU",
@@ -411,6 +421,13 @@ static const struct key_entry uniwill_keymap[] = {
{ KE_END }
};
+static inline bool uniwill_device_supports(struct uniwill_data *data,
+ unsigned int features_mask,
+ unsigned int features)
+{
+ return (data->features & features_mask) == features;
+}
+
static int uniwill_ec_reg_write(void *context, unsigned int reg,
unsigned int val)
{
union acpi_object params[2] = {
@@ -799,24 +816,31 @@ static struct attribute *uniwill_attrs[] = {
static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct
attribute *attr, int n)
{
+ struct device *dev = kobj_to_dev(kobj);
+ struct uniwill_data *data = dev_get_drvdata(dev);
+
if (attr == &dev_attr_fn_lock_toggle_enable.attr) {
- if (supported_features & UNIWILL_FEATURE_FN_LOCK_TOGGLE)
+ if (uniwill_device_supports(data,
UNIWILL_FEATURE_FN_LOCK_TOGGLE,
+ UNIWILL_FEATURE_FN_LOCK_TOGGLE))
return attr->mode;
}
if (attr == &dev_attr_super_key_toggle_enable.attr) {
- if (supported_features & UNIWILL_FEATURE_SUPER_KEY_TOGGLE)
+ if (uniwill_device_supports(data,
UNIWILL_FEATURE_SUPER_KEY_TOGGLE,
+ UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
return attr->mode;
}
if (attr == &dev_attr_touchpad_toggle_enable.attr) {
- if (supported_features & UNIWILL_FEATURE_TOUCHPAD_TOGGLE)
+ if (uniwill_device_supports(data,
UNIWILL_FEATURE_TOUCHPAD_TOGGLE,
+ UNIWILL_FEATURE_TOUCHPAD_TOGGLE))
return attr->mode;
}
if (attr == &dev_attr_rainbow_animation.attr ||
attr == &dev_attr_breathing_in_suspend.attr) {
- if (supported_features & UNIWILL_FEATURE_LIGHTBAR)
+ if (uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR,
+ UNIWILL_FEATURE_LIGHTBAR))
return attr->mode;
}
@@ -944,7 +968,8 @@ static int uniwill_hwmon_init(struct uniwill_data
*data)
{
struct device *hdev;
- if (!(supported_features & UNIWILL_FEATURE_HWMON))
+ if (!uniwill_device_supports(data, UNIWILL_FEATURE_HWMON,
+ UNIWILL_FEATURE_HWMON))
return 0;
hdev = devm_hwmon_device_register_with_info(data->dev,
"uniwill", data,
@@ -1019,7 +1044,8 @@ static int uniwill_led_init(struct uniwill_data
*data)
unsigned int value;
int ret;
- if (!(supported_features & UNIWILL_FEATURE_LIGHTBAR))
+ if (!uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR,
+ UNIWILL_FEATURE_LIGHTBAR))
return 0;
ret = devm_mutex_init(data->dev, &data->led_lock);
@@ -1232,7 +1258,8 @@ static int uniwill_battery_init(struct uniwill_data
*data)
{
int ret;
- if (!(supported_features & UNIWILL_FEATURE_BATTERY))
+ if (!uniwill_device_supports(data, UNIWILL_FEATURE_BATTERY,
+ UNIWILL_FEATURE_BATTERY))
return 0;
ret = devm_mutex_init(data->dev, &data->battery_lock);
@@ -1361,6 +1388,19 @@ static int uniwill_probe(struct platform_device
*pdev)
if (ret < 0)
return ret;
+ data->features = device_descriptor.features;
+
+ /*
+ * Some devices might need to perform some device-specific
initialization steps
+ * before the supported features are initialized. Because of this we
have to call
+ * this callback just after the EC itself was initialized.
+ */
+ if (device_descriptor.probe) {
+ ret = device_descriptor.probe(data);
+ if (ret < 0)
+ return ret;
+ }
+
ret = uniwill_battery_init(data);
if (ret < 0)
return ret;
@@ -1385,7 +1425,8 @@ static void uniwill_shutdown(struct platform_device
*pdev)
static int uniwill_suspend_keyboard(struct uniwill_data *data)
{
- if (!(supported_features & UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
+ if (!uniwill_device_supports(data, UNIWILL_FEATURE_SUPER_KEY_TOGGLE,
+ UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
return 0;
/*
@@ -1397,7 +1438,8 @@ static int uniwill_suspend_keyboard(struct
uniwill_data *data)
static int uniwill_suspend_battery(struct uniwill_data *data)
{
- if (!(supported_features & UNIWILL_FEATURE_BATTERY))
+ if (!uniwill_device_supports(data, UNIWILL_FEATURE_BATTERY,
+ UNIWILL_FEATURE_BATTERY))
return 0;
/*
@@ -1432,7 +1474,8 @@ static int uniwill_resume_keyboard(struct
uniwill_data *data)
unsigned int value;
int ret;
- if (!(supported_features & UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
+ if (!uniwill_device_supports(data, UNIWILL_FEATURE_SUPER_KEY_TOGGLE,
+ UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
return 0;
ret = regmap_read(data->regmap, EC_ADDR_SWITCH_STATUS,
&value);
@@ -1448,7 +1491,8 @@ static int uniwill_resume_keyboard(struct
uniwill_data *data)
static int uniwill_resume_battery(struct uniwill_data *data)
{
- if (!(supported_features & UNIWILL_FEATURE_BATTERY))
+ if (!uniwill_device_supports(data, UNIWILL_FEATURE_BATTERY,
+ UNIWILL_FEATURE_BATTERY))
return 0;
return regmap_update_bits(data->regmap, EC_ADDR_CHARGE_CTRL,
CHARGE_CTRL_MASK,
@@ -1496,6 +1540,25 @@ static struct platform_driver uniwill_driver = {
.shutdown = uniwill_shutdown,
};
+static struct uniwill_device_descriptor lapac71h_descriptor __initdata
= {
+ .features = UNIWILL_FEATURE_FN_LOCK_TOGGLE |
+ UNIWILL_FEATURE_SUPER_KEY_TOGGLE |
+ UNIWILL_FEATURE_TOUCHPAD_TOGGLE |
+ UNIWILL_FEATURE_BATTERY |
+ UNIWILL_FEATURE_HWMON
+};
+
+static struct uniwill_device_descriptor lapkc71f_descriptor __initdata =
{
+ .features = UNIWILL_FEATURE_FN_LOCK_TOGGLE |
+ UNIWILL_FEATURE_SUPER_KEY_TOGGLE |
+ UNIWILL_FEATURE_TOUCHPAD_TOGGLE |
+ UNIWILL_FEATURE_LIGHTBAR |
+ UNIWILL_FEATURE_BATTERY |
+ UNIWILL_FEATURE_HWMON
+};
+
+static struct uniwill_device_descriptor empty_descriptor __initdata = {};
+
static const struct dmi_system_id uniwill_dmi_table[] __initconst = {
{
.ident = "XMG FUSION 15",
@@ -1503,6 +1566,7 @@ static const struct dmi_system_id
uniwill_dmi_table[] __initconst = {
DMI_MATCH(DMI_SYS_VENDOR, "SchenkerTechnologiesGmbH"),
DMI_EXACT_MATCH(DMI_BOARD_NAME, "LAPQC71A"),
},
+ .driver_data = &empty_descriptor,

Hi,

Is there some advantage of having an "empty descriptor" over just NULL
checking its presence in the code?

One less "if"

That pays no respect to devs who have read those dummy driver_data
lines. ;-)

Thanks for doing it anyway ^^

v2 of this patch set is already finished, but i'm still waiting for the special case ibp gen7 being tested before sending it.

In the long run (with more features implemented and tested) there probably
wont be any device using the empty descriptor, then it can be removed again.

Fair (so I'm fine with keeping empty_descriptor for now).

Next message: kernel test robot: "Re: [PATCH v2 3/3] randomize_kstack: Unify random source across arches"
Previous message: Tomi Valkeinen: "[PATCH v4 00/15] media: rcar: Streams support"
In reply to: Ilpo Järvinen: "Re: [PATCH 1/2] platform/x86: uniwill-laptop: Introduce device descriptor system"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]