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

[Armin Wolf <W_Armin@gmx.de>]

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.

Thanks,
Armin Wolf

>   };
>   
>   /*