Re: [PATCH 1/5] platform/x86: uniwill-laptop: Report correct lightbar brightness

[Ilpo Järvinen]

On Sat, 5 Sep 2026, Armin Wolf wrote:

> The LED sysfs ABI now allows us to specify the maximum values for the
> global brightness and the color intensity values independently. At the
> same time the maximum brightness value reported for the lightbar (like
> 200) does not match the real value supported by the hardware (1),
> potentially confusing users.
> 
> Report the correct maximum brightness value and only use the intensity
> values for controlling the lightbar color.
> 
> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
> ---
>  drivers/platform/x86/uniwill/uniwill-acpi.c | 35 ++++++++++-----------
>  1 file changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 7a2eeaec4c96..7420e1dd4716 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -398,7 +398,7 @@ struct uniwill_data {
>  	struct mutex super_key_lock;	/* Protects the toggling of the super key lock state */
>  	struct list_head batteries;
>  	struct mutex led_lock;		/* Protects writes to the lightbar registers */
> -	u8 lightbar_max_brightness;
> +	u8 lightbar_max_intensity;
>  	struct led_classdev_mc led_mc_cdev;
>  	struct mc_subled led_mc_subled_info[LED_CHANNELS];
>  	bool kbd_led_single_color;
> @@ -429,7 +429,7 @@ struct uniwill_device_descriptor {
>  	unsigned int features;
>  	bool kbd_led_single_color;
>  	u8 kbd_led_max_brightness;
> -	u8 lightbar_max_brightness;
> +	u8 lightbar_max_intensity;
>  	/* Executed during driver probing */
>  	int (*probe)(struct uniwill_data *data);
>  };
> @@ -1509,20 +1509,16 @@ static int uniwill_led_brightness_set(struct led_classdev *led_cdev, enum led_br
>  	unsigned int value;
>  	int ret;
>  
> -	ret = led_mc_calc_color_components(led_mc_cdev, brightness);
> -	if (ret < 0)
> -		return ret;
> -
>  	guard(mutex)(&data->led_lock);
>  
>  	for (int i = 0; i < LED_CHANNELS; i++) {
> -		/* Prevent the brightness values from overflowing */
> -		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);
> +		ret = regmap_write(data->regmap, uniwill_led_channel_to_ac_reg[i],
> +				   data->led_mc_subled_info[i].intensity);
>  		if (ret < 0)
>  			return ret;
>  
> -		ret = regmap_write(data->regmap, uniwill_led_channel_to_bat_reg[i], value);
> +		ret = regmap_write(data->regmap, uniwill_led_channel_to_bat_reg[i],
> +				   data->led_mc_subled_info[i].intensity);
>  		if (ret < 0)
>  			return ret;
>  	}
> @@ -1586,14 +1582,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 = data->lightbar_max_brightness;
> +	data->led_mc_cdev.led_cdev.max_brightness = 1;
>  	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 = data->lightbar_max_brightness;
> +		data->led_mc_cdev.led_cdev.brightness = 1;
>  
>  	for (int i = 0; i < LED_CHANNELS; i++) {
>  		data->led_mc_subled_info[i].color_index = color_indices[i];
> @@ -1604,9 +1600,9 @@ static int uniwill_led_init(struct uniwill_data *data)
>  
>  		/*
>  		 * Make sure that the initial intensity value is not greater than
> -		 * the maximum brightness.
> +		 * the maximum intensity.
>  		 */
> -		value = min(data->lightbar_max_brightness, value);
> +		value = min(data->lightbar_max_intensity, value);
>  		ret = regmap_write(data->regmap, uniwill_led_channel_to_ac_reg[i], value);
>  		if (ret < 0)
>  			return ret;
> @@ -1616,6 +1612,7 @@ static int uniwill_led_init(struct uniwill_data *data)
>  			return ret;
>  
>  		data->led_mc_subled_info[i].intensity = value;
> +		data->led_mc_subled_info[i].max_intensity = data->lightbar_max_intensity;
>  		data->led_mc_subled_info[i].channel = i;
>  	}
>  
> @@ -2338,7 +2335,7 @@ static int uniwill_probe(struct platform_device *pdev)
>  	data->features = device_descriptor.features;
>  	data->kbd_led_single_color = device_descriptor.kbd_led_single_color;
>  	data->kbd_led_max_brightness = device_descriptor.kbd_led_max_brightness;
> -	data->lightbar_max_brightness = device_descriptor.lightbar_max_brightness;
> +	data->lightbar_max_intensity = device_descriptor.lightbar_max_intensity;
>  
>  	/*
>  	 * Some devices might need to perform some device-specific initialization steps
> @@ -2685,7 +2682,7 @@ static struct uniwill_device_descriptor lapqc71a_lapqc71b_descriptor __initdata
>  		    UNIWILL_FEATURE_GPU_TEMP |
>  		    UNIWILL_FEATURE_PRIMARY_FAN |
>  		    UNIWILL_FEATURE_SECONDARY_FAN,
> -	.lightbar_max_brightness = 36,
> +	.lightbar_max_intensity = 36,
>  };
>  
>  static struct uniwill_device_descriptor lapac71h_descriptor __initdata = {
> @@ -2709,7 +2706,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,
> +	.lightbar_max_intensity = 200,
>  };

Can you respin this on top of what's in the review-ilpo-next I think there 
are now more entries to touch after Werner's descriptor splits.

>  /*
> @@ -3355,8 +3352,8 @@ static int __init uniwill_init(void)
>  		device_descriptor.kbd_led_single_color = false;
>  		/* Some models only support 3 brightness levels */
>  		device_descriptor.kbd_led_max_brightness = 4;
> -		/* Some models only support 36 brightness levels per color component */
> -		device_descriptor.lightbar_max_brightness = 200;
> +		/* Some models only support 36 intensity levels per color component */
> +		device_descriptor.lightbar_max_intensity = 200;
>  		pr_warn("Enabling potentially unsupported features\n");
>  	}
>  
> 

-- 
 i.