Re: [RFC PATCH 2/3] platform/x86: uniwill-laptop: Implement lightbar for XMG Fusion (L19)

[Armin Wolf <W_Armin@gmx.de>]

Am 22.04.26 um 13:17 schrieb Armin Wolf:
> Am 21.04.26 um 22:01 schrieb Werner Sembach:
>> The XMG Fusion (L19) also has a lightbar but with a different max
>> brightness value of 36 instead of 200.
>>
>> This patch adds a new feature flag for the driver to correctly handle 
>> this
>> case and applies it to the mentioned device.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> ---
>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 19 +++++++++++++------
>>   1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/ 
>> platform/x86/uniwill/uniwill-acpi.c
>> index 7d1fdbacc6871..62d56cc67e2e8 100644
>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> @@ -318,7 +318,6 @@
>>   #define FAN_TABLE_LENGTH    16
>>   #define LED_CHANNELS        3
>> -#define LED_MAX_BRIGHTNESS    200
>>   #define UNIWILL_FEATURE_FN_LOCK            BIT(0)
>>   #define UNIWILL_FEATURE_SUPER_KEY        BIT(1)
>> @@ -344,6 +343,7 @@ struct uniwill_data {
>>       acpi_handle handle;
>>       struct regmap *regmap;
>>       unsigned int features;
>> +    u8 lightbar_max_brightness;
>>       struct acpi_battery_hook hook;
>>       struct mutex battery_lock;    /* Protects the list of currently 
>> registered batteries */
>>       union {
>> @@ -376,6 +376,7 @@ struct uniwill_battery_entry {
>>   struct uniwill_device_descriptor {
>>       unsigned int features;
>> +    u8 lightbar_max_brightness;
>>       /* Executed during driver probing */
>>       int (*probe)(struct uniwill_data *data);
>>   };
>> @@ -1340,7 +1341,7 @@ static int uniwill_led_brightness_set(struct 
>> led_classdev *led_cdev, enum led_br
>>       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);
>> +        value = min(data->lightbar_max_brightness, data- 
>> >led_mc_subled_info[i].brightness);
>>           ret = regmap_write(data->regmap, 
>> uniwill_led_channel_to_ac_reg[i], value);
>>           if (ret < 0)
>>               return ret;
>> @@ -1376,12 +1377,14 @@ static int uniwill_led_init(struct 
>> uniwill_data *data)
>>           LED_COLOR_ID_GREEN,
>>           LED_COLOR_ID_BLUE,
>>       };
>> -    unsigned int value;
>> +    unsigned int value, max_brightness;
>>       int ret;
>>       if (!uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR))
>>           return 0;
>> +    max_brightness = data->lightbar_max_brightness;
> 
> Please drop the max_brightness variable and assign 
> led_cdev.max_brightness and led_cdev.brightness directly.
> 
>> +
>>       ret = devm_mutex_init(data->dev, &data->led_lock);
>>       if (ret < 0)
>>           return ret;
>> @@ -1409,14 +1412,14 @@ static int uniwill_led_init(struct 
>> uniwill_data *data)
>>           return ret;
>>       data->led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
>> -    data->led_mc_cdev.led_cdev.max_brightness = LED_MAX_BRIGHTNESS;
>> +    data->led_mc_cdev.led_cdev.max_brightness = max_brightness;
>>       data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
>>       data->led_mc_cdev.led_cdev.brightness_set_blocking = 
>> uniwill_led_brightness_set;
>>       if (value & LIGHTBAR_S0_OFF)
>>           data->led_mc_cdev.led_cdev.brightness = 0;
>>       else
>> -        data->led_mc_cdev.led_cdev.brightness = LED_MAX_BRIGHTNESS;
>> +        data->led_mc_cdev.led_cdev.brightness = max_brightness;
>>       for (int i = 0; i < LED_CHANNELS; i++) {
>>           data->led_mc_subled_info[i].color_index = color_indices[i];
>> @@ -1429,7 +1432,7 @@ static int uniwill_led_init(struct uniwill_data 
>> *data)
>>            * Make sure that the initial intensity value is not greater 
>> than
>>            * the maximum brightness.
>>            */
>> -        value = min(LED_MAX_BRIGHTNESS, value);
>> +        value = min(max_brightness, value);
>>           ret = regmap_write(data->regmap, 
>> uniwill_led_channel_to_ac_reg[i], value);
>>           if (ret < 0)
>>               return ret;
>> @@ -1882,6 +1885,7 @@ static int uniwill_probe(struct platform_device 
>> *pdev)
>>           return ret;
>>       data->features = device_descriptor.features;
>> +    data->lightbar_max_brightness = 
>> device_descriptor.lightbar_max_brightness;
>>       /*
>>        * Some devices might need to perform some device-specific 
>> initialization steps
>> @@ -2130,11 +2134,13 @@ static struct platform_driver uniwill_driver = {
>>   static struct uniwill_device_descriptor lapqc71a_lapqc71b_descriptor 
>> __initdata = {
>>       .features = UNIWILL_FEATURE_SUPER_KEY |
>> +            UNIWILL_FEATURE_LIGHTBAR |
>>               UNIWILL_FEATURE_BATTERY_CHARGE_LIMIT |
>>               UNIWILL_FEATURE_CPU_TEMP |
>>               UNIWILL_FEATURE_GPU_TEMP |
>>               UNIWILL_FEATURE_PRIMARY_FAN |
>>               UNIWILL_FEATURE_SECONDARY_FAN,
>> +    .lightbar_max_brightness = 36,
>>   };
>>   static struct uniwill_device_descriptor lapac71h_descriptor 
>> __initdata = {
>> @@ -2158,6 +2164,7 @@ static struct uniwill_device_descriptor 
>> lapkc71f_descriptor __initdata = {
>>               UNIWILL_FEATURE_GPU_TEMP |
>>               UNIWILL_FEATURE_PRIMARY_FAN |
>>               UNIWILL_FEATURE_SECONDARY_FAN,
>> +    .lightbar_max_brightness = 200,
> 
> Please also set lightbar_max_brightness to 200 during module 
> initialization when the "force" parameter is set.
> 
> I suggest you rebase this series on top of the "platform/x86: uniwill- 
> laptop: Charging-related improvements" series to avoid merge conflicts.

Forget about the rebase part, the series is already based on this series xd.

Thanks,
Armin Wolf

> 
> Thanks,
> Armin Wolf
> 
>>   };
>>   /*
> 
>