linux-kernel - Re: [RFC PATCH 2/3] platform/x86: Add Uniwill laptop driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c689db31-60cc-4494-b700-88744376f589@tuxedocomputers.com>
Date: Mon, 30 Jun 2025 14:32:34 +0200
From: Werner Sembach <wse@...edocomputers.com>
To: Armin Wolf <W_Armin@....de>, Pőcze Barnabás
 <pobrn@...tonmail.com>, ilpo.jarvinen@...ux.intel.com, hdegoede@...hat.com,
 chumuzero@...il.com, corbet@....net, cs@...edo.de, ggo@...edocomputers.com
Cc: linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 platform-driver-x86@...r.kernel.org
Subject: Re: [RFC PATCH 2/3] platform/x86: Add Uniwill laptop driver

Hi,

Am 28.06.25 um 01:09 schrieb Armin Wolf:
> Am 25.06.25 um 17:59 schrieb Pőcze Barnabás:
>
>> Hi
>>
>> 2025. 06. 23. 0:36 keltezéssel, Armin Wolf írta:
>>> Am 22.06.25 um 23:37 schrieb Pőcze Barnabás:
>>>
>>>> Hi
>>>>
>>>>
>>>> 2025. 06. 15. 19:59 keltezéssel, Armin Wolf írta:
>>>>> Add a new driver for Uniwill laptops. The driver uses a ACPI WMI
>>>>> interface to talk with the embedded controller, but relies on a
>>>>> DMI whitelist for autoloading since Uniwill just copied the WMI
>>>>> GUID from the Windows driver example.
>>>>>
>>>>> The driver is reverse-engineered based on the following information:
>>>>> - OEM software from intel
>>>>> - https://github.com/pobrn/qc71_laptop
>>>> Oh... I suppose an end of an era for me...
>>> I now remember that we interacted on the mailing lists before, sorry for not 
>>> CCing
>>> you on this patch series.
>>>
>>> Do you want a Co-developed-by tag on those patches?
>> I'll leave it up to you.
>>
>>
>>>>> - https://github.com/tuxedocomputers/tuxedo-drivers
>>>>> - https://github.com/tuxedocomputers/tuxedo-control-center
>>>>>
>>>>> The underlying EC supports various features, including hwmon sensors,
>>>>> battery charge limiting, a RGB lightbar and keyboard-related controls.
>>>>>
>>>>> Reported-by: cyear <chumuzero@...il.com>
>>>>> Closes: https://github.com/lm-sensors/lm-sensors/issues/508
>>>>> Closes: https://github.com/Wer-Wolf/uniwill-laptop/issues/3
>>>>> Signed-off-by: Armin Wolf <W_Armin@....de>
>>>>> ---
>>>>>      .../ABI/testing/sysfs-driver-uniwill-laptop   |   53 +
>>>>>      Documentation/wmi/devices/uniwill-laptop.rst  |  109 ++
>>>>>      MAINTAINERS                                   |    8 +
>>>>>      drivers/platform/x86/uniwill/Kconfig          |   17 +
>>>>>      drivers/platform/x86/uniwill/Makefile         |    1 +
>>>>>      drivers/platform/x86/uniwill/uniwill-laptop.c | 1477 +++++++++++++++++
>>>>>      drivers/platform/x86/uniwill/uniwill-wmi.c    |    3 +-
>>>>>      7 files changed, 1667 insertions(+), 1 deletion(-)
>>>>>      create mode 100644 Documentation/ABI/testing/sysfs-driver-uniwill-laptop
>>>>>      create mode 100644 Documentation/wmi/devices/uniwill-laptop.rst
>>>>>      create mode 100644 drivers/platform/x86/uniwill/uniwill-laptop.c
>>>>>
>>> [...]
>>>>> +
>>>>> +static const unsigned int uniwill_led_channel_to_bat_reg[LED_CHANNELS] = {
>>>>> +    EC_ADDR_LIGHTBAR_BAT_RED,
>>>>> +    EC_ADDR_LIGHTBAR_BAT_GREEN,
>>>>> +    EC_ADDR_LIGHTBAR_BAT_BLUE,
>>>>> +};
>>>>> +
>>>>> +static const unsigned int uniwill_led_channel_to_ac_reg[LED_CHANNELS] = {
>>>>> +    EC_ADDR_LIGHTBAR_AC_RED,
>>>>> +    EC_ADDR_LIGHTBAR_AC_GREEN,
>>>>> +    EC_ADDR_LIGHTBAR_AC_BLUE,
>>>>> +};
>>>>> +
>>>>> +static int uniwill_led_brightness_set(struct led_classdev *led_cdev, enum 
>>>>> led_brightness brightness)
>>>>> +{
>>>>> +    struct led_classdev_mc *led_mc_cdev = lcdev_to_mccdev(led_cdev);
>>>>> +    struct uniwill_data *data = container_of(led_mc_cdev, struct 
>>>>> uniwill_data, led_mc_cdev);
>>>>> +    unsigned int value;
>>>>> +    int ret;
>>>>> +
>>>>> +    ret = led_mc_calc_color_components(led_mc_cdev, brightness);
>>>>> +    if (ret < 0)
>>>>> +        return ret;
>>>>> +
>>>>> +    for (int i = 0; i < LED_CHANNELS; i++) {
>>>>> +        /* Prevent the brightness values from overflowing */
>>>>> +        value = min(LED_MAX_BRIGHTNESS, 
>>>>> data->led_mc_subled_info[i].brightness);
>>>>> +        ret = regmap_write(data->regmap, 
>>>>> uniwill_led_channel_to_ac_reg[i], value);
>>>> This is interesting. I am not sure which "control center" application you 
>>>> have looked at,
>>>> but I found many lookup tables based on the exact model, etc. For example, 
>>>> on my laptop
>>>> any value larger than 36 will simply turn that color component off. Have 
>>>> you seen
>>>> anything like that?
>>> I was using the Intel NUC studio software application during 
>>> reverse-engineering and had a user
>>> test the resulting code on a Intel NUC notebook. AFAIK the OEM software did 
>>> not use a lookup table.
>>>
>>> If we extend this driver in the future then we might indeed use the quirk 
>>> system to change the max.
>>> LED brightness depending on the model.
>> I see. So everything up to 200 works. And after that do you know if it turns 
>> off or what happens?
>
> The user who tested the driver reported that "the brightest lightbar setting 
> is 200", so i assume
> that the lightbar simply clamps the values. However i would not trust the EC 
> firmware in the slightest,
> i can definitely imagine that other models react differently.

Iirc at least for keyboard backlight on tf devices there was a value that could 
be overwritten to make the values 0-255 instead of 0-200, maybe this is also 
true for the lightbar, but i don't know if this affects the livespan of the leds.

Best regards,

Werner