Re: [PATCH v4 2/2] platform/x86: uniwill-laptop: Remove single color keyboard detection

Am 21.07.26 um 10:44 schrieb Ilpo Järvinen:

> On Mon, 20 Jul 2026, Armin Wolf wrote:
>
>> Having a ad-hoc device whitelist inside uniwill_kbd_led_init()
>> to work around unreliable KBD_WHITE_ONLY values conflicts with
>> the idea of the device descriptor infrastructure.
>>
>> Remove the ad-hoc device whitelist and use the device descriptor
>> infrastructure instead.
>>
>> Suggested-by: Werner Sembach <[email protected]>
>> Reviewed-by: Werner Sembach <[email protected]>
>> Signed-off-by: Armin Wolf <[email protected]>
>> ---
>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 32 +++++++--------------
>>   1 file changed, 11 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> index d27f316800f6..7a2eeaec4c96 100644
>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> @@ -255,6 +255,7 @@
>>   #define FAN_CURVE_LENGTH		5
>>   
>>   #define EC_ADDR_KBD_STATUS		0x078C
>> +/* Unreliable on some models, use the device descriptor instead. */
>>   #define KBD_WHITE_ONLY			BIT(0)
>>   #define KBD_POWER_OFF			BIT(1)
>>   #define KBD_TURBO_LEVEL_MASK		GENMASK(3, 2)
>> @@ -400,7 +401,7 @@ struct uniwill_data {
>>   	u8 lightbar_max_brightness;
>>   	struct led_classdev_mc led_mc_cdev;
>>   	struct mc_subled led_mc_subled_info[LED_CHANNELS];
>> -	bool single_color_kbd;
>> +	bool kbd_led_single_color;
>>   	u8 kbd_led_max_brightness;
>>   	unsigned int last_kbd_status;
>>   	union {
>> @@ -426,6 +427,7 @@ struct uniwill_battery_entry {
>>   
>>   struct uniwill_device_descriptor {
>>   	unsigned int features;
>> +	bool kbd_led_single_color;
>>   	u8 kbd_led_max_brightness;
>>   	u8 lightbar_max_brightness;
>>   	/* Executed during driver probing */
>> @@ -1629,7 +1631,7 @@ static int uniwill_notify_kbd_led(struct uniwill_data *data, int brightness)
>>   	struct led_classdev *led_cdev;
>>   	int ret;
>>   
>> -	if (data->single_color_kbd)
>> +	if (data->kbd_led_single_color)
>>   		led_cdev = &data->kbd_led_cdev;
>>   	else
>>   		led_cdev = &data->kbd_led_mc_cdev.led_cdev;
>> @@ -1858,24 +1860,7 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
>>   	if (ret < 0)
>>   		return ret;
>>   
>> -	switch (data->project_id) {
>> -	case PROJECT_ID_PF:
>> -	case PROJECT_ID_PF4MU_PF4MN_PF5MU:
>> -	case PROJECT_ID_PH4TRX1:
>> -	case PROJECT_ID_PH4TUX1:
>> -	case PROJECT_ID_PH4TQX1:
>> -	case PROJECT_ID_PH6TRX1:
>> -	case PROJECT_ID_PH6TQXX:
>> -	case PROJECT_ID_PHXAXXX:
>> -	case PROJECT_ID_PHXPXXX:
>> -		data->single_color_kbd = true;
>> -		break;
>> -	default:
>> -		data->single_color_kbd = regval & KBD_WHITE_ONLY;
>> -		break;
>> -	}
>> -
>> -	if (data->single_color_kbd)
>> +	if (data->kbd_led_single_color)
>>   		return uniwill_white_kbd_led_init(data);
>>   
>>   	return uniwill_rgb_kbd_led_init(data);
>> @@ -2351,6 +2336,7 @@ static int uniwill_probe(struct platform_device *pdev)
>>   		return ret;
>>   
>>   	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;
>>   
>> @@ -2580,7 +2566,7 @@ static int uniwill_resume_kbd_led(struct uniwill_data *data)
>>   	if (ret < 0)
>>   		return ret;
>>   
>> -	if (data->single_color_kbd)
>> +	if (data->kbd_led_single_color)
>>   		return 0;
>>   
>>   	return regmap_write_bits(data->regmap, EC_ADDR_TRIGGER, RGB_APPLY_COLOR, RGB_APPLY_COLOR);
>> @@ -2687,6 +2673,7 @@ static struct uniwill_device_descriptor machenike_l16p_descriptor __initdata = {
>>   		    UNIWILL_FEATURE_KEYBOARD_BACKLIGHT |
>>   		    UNIWILL_FEATURE_AC_AUTO_BOOT |
>>   		    UNIWILL_FEATURE_USB_POWERSHARE,
>> +	.kbd_led_single_color = false,
>>   	.kbd_led_max_brightness = 4,
>>   };
>>   
>> @@ -2869,6 +2856,7 @@ static struct uniwill_device_descriptor x4sp4nal_descriptor __initdata = {
>>   		    UNIWILL_FEATURE_KEYBOARD_BACKLIGHT |
>>   		    UNIWILL_FEATURE_AC_AUTO_BOOT |
>>   		    UNIWILL_FEATURE_USB_POWERSHARE,
>> +	.kbd_led_single_color = true,
>>   	.kbd_led_max_brightness = 2,
>>   };
>>   
>> @@ -3363,6 +3351,8 @@ static int __init uniwill_init(void)
>>   	if (force) {
>>   		/* Assume that the device supports all features except the charge limit */
>>   		device_descriptor.features = UINT_MAX & ~UNIWILL_FEATURE_BATTERY_CHARGE_LIMIT;
>> +		/* Some models only have a (white) single color keyboard backlight */
>> +		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 */
>>
> Hi Armin,
>
> I've applied this series now to the review-ilpo-next branch.
>   
> You may want to check if you still need/want to store project_id (sashiko
> mentioned it's no longer used). If you want it gone, just send another
> patch.

Thanks, i will continue to utilize this field in the future.

Armin Wolf