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

[Werner Sembach <wse@tuxedocomputers.com>]

Am 11.07.26 um 00:35 schrieb Armin Wolf:
> It turns out that the code for detecting single color keyboards
> might not work correctly on future models. Remove it and use the
> device descriptor instead.
Technically: It already doesn't work reliable on old devices, that's why the 
device list inside the init function was required. This patch is basically to 
move this to the device_descriptor struct that is already present.
>
> Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>   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..4591ee299a90 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 */
>   #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;

Why do this as a bool instead of a UNIWILL_FEATURE_* define? We already have 
mutually exclusive UNIWILL_FEATURE_* defines for the charging modes.

Best regards

Werner

>   	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 */