[PATCH v2 0/3] platform/x86: uniwill-laptop: Prepare for extending keyboard backlight support

[PATCH v2 0/3] platform/x86: uniwill-laptop: Prepare for extending keyboard backlight support

[Armin Wolf]

This patch series contains a couple of changes suggested by
Werner Sembach from Tuxedo that are necessary for eventually
enabling keyboard backlight support for Tuxedo devices.

The first patch simply cleans up uniwill_kbd_led_init(), while
the remaining two patches make two key properties (single color
vs RGB, max intensity) configurable using the existing device
descriptor infrastructure.

Tested on a Tuxedo InfinityBook Pro 15 Gen 10 AMD.

Changes since v1:
- add Suggested-by tags

Armin Wolf (3):
  platform/x86: uniwill-laptop: Split uniwill_kbd_led_init()
  platform/x86: uniwill-laptop: Remove single color keyboard detection
  platform/x86: uniwill-laptop: Make KBD_LED_MAX_INTENSITY configurable

 drivers/platform/x86/uniwill/uniwill-acpi.c | 132 +++++++++++---------
 1 file changed, 72 insertions(+), 60 deletions(-)

-- 
2.39.5

[PATCH v2 1/3] platform/x86: uniwill-laptop: Split uniwill_kbd_led_init()

[Armin Wolf]

The function uniwill_kbd_led_init() is quite large and doing multiple
things at once:
- general hardware initialisation
- single color keyboard backlight registration
- RGB keyboard backlight registration

Move the last two things into separate functions to increase the
maintainability of uniwill_kbd_led_init().

Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/uniwill/uniwill-acpi.c | 123 +++++++++++---------
 1 file changed, 70 insertions(+), 53 deletions(-)

diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index 310445583832..d27f316800f6 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -1733,7 +1733,24 @@ static enum led_brightness uniwill_kbd_led_mc_brightness_get(struct led_classdev
 	return uniwill_kbd_led_read_brightness(data);
 }
 
-static int uniwill_kbd_led_init(struct uniwill_data *data)
+static int uniwill_white_kbd_led_init(struct uniwill_data *data)
+{
+	struct led_init_data init_data = {
+		.default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT,
+		.devicename = DRIVER_NAME,
+		.devname_mandatory = true,
+	};
+
+	data->kbd_led_cdev.max_brightness = data->kbd_led_max_brightness;
+	data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
+	data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | LED_REJECT_NAME_CONFLICT;
+	data->kbd_led_cdev.brightness_set_blocking = uniwill_kbd_led_brightness_set;
+	data->kbd_led_cdev.brightness_get = uniwill_kbd_led_brightness_get;
+
+	return devm_led_classdev_register_ext(data->dev, &data->kbd_led_cdev, &init_data);
+}
+
+static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
 {
 	unsigned int color_indices[KBD_LED_CHANNELS] = {
 		LED_COLOR_ID_RED,
@@ -1741,6 +1758,7 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
 		LED_COLOR_ID_BLUE,
 	};
 	struct led_init_data init_data = {
+		.default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT,
 		.devicename = DRIVER_NAME,
 		.devname_mandatory = true,
 	};
@@ -1749,57 +1767,6 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
 	unsigned int regval;
 	int ret;
 
-	if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
-		return 0;
-
-	ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
-	if (ret < 0)
-		return ret;
-
-	if (!(regval & CHINA_MODE)) {
-		ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, ENABLE_CHINA_MODE);
-		if (ret < 0)
-			return ret;
-	}
-
-	ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
-	if (ret < 0)
-		return ret;
-
-	regval |= KBD_APPLY;
-	regval &= ~KBD_POWER_OFF;
-	ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
-	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) {
-		init_data.default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT;
-		data->kbd_led_cdev.max_brightness = data->kbd_led_max_brightness;
-		data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
-		data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | LED_REJECT_NAME_CONFLICT;
-		data->kbd_led_cdev.brightness_set_blocking = uniwill_kbd_led_brightness_set;
-		data->kbd_led_cdev.brightness_get = uniwill_kbd_led_brightness_get;
-
-		return devm_led_classdev_register_ext(data->dev, &data->kbd_led_cdev, &init_data);
-	}
-
 	for (int i = 0; i < KBD_LED_CHANNELS; i++) {
 		data->kbd_led_mc_subled_info[i].color_index = color_indices[i];
 
@@ -1851,7 +1818,6 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
 	if (ret < 0)
 		return ret;
 
-	init_data.default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT;
 	data->kbd_led_mc_cdev.led_cdev.max_brightness = data->kbd_led_max_brightness;
 	data->kbd_led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
 	data->kbd_led_mc_cdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED | LED_REJECT_NAME_CONFLICT;
@@ -1864,6 +1830,57 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
 							 &init_data);
 }
 
+static int uniwill_kbd_led_init(struct uniwill_data *data)
+{
+	unsigned int regval;
+	int ret;
+
+	if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
+		return 0;
+
+	ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
+	if (ret < 0)
+		return ret;
+
+	if (!(regval & CHINA_MODE)) {
+		ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, ENABLE_CHINA_MODE);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
+	if (ret < 0)
+		return ret;
+
+	regval |= KBD_APPLY;
+	regval &= ~KBD_POWER_OFF;
+	ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
+	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)
+		return uniwill_white_kbd_led_init(data);
+
+	return uniwill_rgb_kbd_led_init(data);
+}
+
 static unsigned int uniwill_sanitize_battery_threshold(unsigned int value)
 {
 	/* 0 means "charging threshold not active" */
-- 
2.39.5

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

[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.

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

[PATCH v2 3/3] platform/x86: uniwill-laptop: Make KBD_LED_MAX_INTENSITY configurable

[Armin Wolf]

Some (yet to be enabled) Tuxedo devices have a maximum intensity of
200 instead of the default 50. Remove the hardcoded maximum intensity
and use the device descriptor instead.

Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/uniwill/uniwill-acpi.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index 4591ee299a90..5b8e35bf9a51 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -351,7 +351,6 @@
 #define LED_CHANNELS		3
 
 #define KBD_LED_CHANNELS	3
-#define KBD_LED_MAX_INTENSITY	50
 
 #define UNIWILL_FEATURE_FN_LOCK			BIT(0)
 #define UNIWILL_FEATURE_SUPER_KEY		BIT(1)
@@ -403,6 +402,7 @@ struct uniwill_data {
 	struct mc_subled led_mc_subled_info[LED_CHANNELS];
 	bool kbd_led_single_color;
 	u8 kbd_led_max_brightness;
+	u8 kbd_led_max_intensity;
 	unsigned int last_kbd_status;
 	union {
 		struct {
@@ -429,6 +429,7 @@ struct uniwill_device_descriptor {
 	unsigned int features;
 	bool kbd_led_single_color;
 	u8 kbd_led_max_brightness;
+	u8 kbd_led_max_intensity;
 	u8 lightbar_max_brightness;
 	/* Executed during driver probing */
 	int (*probe)(struct uniwill_data *data);
@@ -1780,8 +1781,8 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
 		 * Make sure that the initial intensity value is not greater than
 		 * the maximum intensity.
 		 */
-		if (regval > KBD_LED_MAX_INTENSITY) {
-			regval = KBD_LED_MAX_INTENSITY;
+		if (regval > data->kbd_led_max_intensity) {
+			regval = data->kbd_led_max_intensity;
 			ret = regmap_write(data->regmap, uniwill_kbd_led_channel_to_reg[i], regval);
 			if (ret < 0)
 				return ret;
@@ -1793,7 +1794,7 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
 			intensity_all_zeros = false;
 
 		data->kbd_led_mc_subled_info[i].intensity = regval;
-		data->kbd_led_mc_subled_info[i].max_intensity = KBD_LED_MAX_INTENSITY;
+		data->kbd_led_mc_subled_info[i].max_intensity = data->kbd_led_max_intensity;
 		data->kbd_led_mc_subled_info[i].channel = i;
 	}
 
@@ -2338,6 +2339,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->kbd_led_max_intensity = device_descriptor.kbd_led_max_intensity;
 	data->lightbar_max_brightness = device_descriptor.lightbar_max_brightness;
 
 	/*
@@ -2675,6 +2677,7 @@ static struct uniwill_device_descriptor machenike_l16p_descriptor __initdata = {
 		    UNIWILL_FEATURE_USB_POWERSHARE,
 	.kbd_led_single_color = false,
 	.kbd_led_max_brightness = 4,
+	.kbd_led_max_intensity = 50,
 };
 
 static struct uniwill_device_descriptor lapqc71a_lapqc71b_descriptor __initdata = {
@@ -2858,6 +2861,7 @@ static struct uniwill_device_descriptor x4sp4nal_descriptor __initdata = {
 		    UNIWILL_FEATURE_USB_POWERSHARE,
 	.kbd_led_single_color = true,
 	.kbd_led_max_brightness = 2,
+	.kbd_led_max_intensity = 50,
 };
 
 static const struct dmi_system_id uniwill_dmi_table[] __initconst = {
@@ -3355,6 +3359,7 @@ 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;
+		device_descriptor.kbd_led_max_intensity = 50;
 		/* Some models only support 36 brightness levels per color component */
 		device_descriptor.lightbar_max_brightness = 200;
 		pr_warn("Enabling potentially unsupported features\n");
-- 
2.39.5

Re: [PATCH v2 1/3] platform/x86: uniwill-laptop: Split uniwill_kbd_led_init()

[Werner Sembach]

Am 11.07.26 um 00:35 schrieb Armin Wolf:
> The function uniwill_kbd_led_init() is quite large and doing multiple
> things at once:
> - general hardware initialisation
> - single color keyboard backlight registration
> - RGB keyboard backlight registration
>
> Move the last two things into separate functions to increase the
> maintainability of uniwill_kbd_led_init().
>
> Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>   drivers/platform/x86/uniwill/uniwill-acpi.c | 123 +++++++++++---------
>   1 file changed, 70 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 310445583832..d27f316800f6 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -1733,7 +1733,24 @@ static enum led_brightness uniwill_kbd_led_mc_brightness_get(struct led_classdev
>   	return uniwill_kbd_led_read_brightness(data);
>   }
>   
> -static int uniwill_kbd_led_init(struct uniwill_data *data)
> +static int uniwill_white_kbd_led_init(struct uniwill_data *data)
> +{
> +	struct led_init_data init_data = {
> +		.default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT,
> +		.devicename = DRIVER_NAME,
> +		.devname_mandatory = true,
> +	};
> +
> +	data->kbd_led_cdev.max_brightness = data->kbd_led_max_brightness;
> +	data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
> +	data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | LED_REJECT_NAME_CONFLICT;
> +	data->kbd_led_cdev.brightness_set_blocking = uniwill_kbd_led_brightness_set;
> +	data->kbd_led_cdev.brightness_get = uniwill_kbd_led_brightness_get;
> +
> +	return devm_led_classdev_register_ext(data->dev, &data->kbd_led_cdev, &init_data);
> +}
> +
> +static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
>   {
>   	unsigned int color_indices[KBD_LED_CHANNELS] = {
>   		LED_COLOR_ID_RED,
> @@ -1741,6 +1758,7 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
>   		LED_COLOR_ID_BLUE,
>   	};
>   	struct led_init_data init_data = {
> +		.default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT,
>   		.devicename = DRIVER_NAME,
>   		.devname_mandatory = true,
>   	};
> @@ -1749,57 +1767,6 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
>   	unsigned int regval;
>   	int ret;
>   
> -	if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
> -		return 0;
> -
> -	ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
> -	if (ret < 0)
> -		return ret;
> -
> -	if (!(regval & CHINA_MODE)) {
> -		ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, ENABLE_CHINA_MODE);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -	ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
> -	if (ret < 0)
> -		return ret;
> -
> -	regval |= KBD_APPLY;
> -	regval &= ~KBD_POWER_OFF;
> -	ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
> -	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) {
> -		init_data.default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT;
> -		data->kbd_led_cdev.max_brightness = data->kbd_led_max_brightness;
> -		data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
> -		data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | LED_REJECT_NAME_CONFLICT;
> -		data->kbd_led_cdev.brightness_set_blocking = uniwill_kbd_led_brightness_set;
> -		data->kbd_led_cdev.brightness_get = uniwill_kbd_led_brightness_get;
> -
> -		return devm_led_classdev_register_ext(data->dev, &data->kbd_led_cdev, &init_data);
> -	}
> -
>   	for (int i = 0; i < KBD_LED_CHANNELS; i++) {
>   		data->kbd_led_mc_subled_info[i].color_index = color_indices[i];
>   
> @@ -1851,7 +1818,6 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
>   	if (ret < 0)
>   		return ret;
>   
> -	init_data.default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT;
>   	data->kbd_led_mc_cdev.led_cdev.max_brightness = data->kbd_led_max_brightness;
>   	data->kbd_led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
>   	data->kbd_led_mc_cdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED | LED_REJECT_NAME_CONFLICT;
> @@ -1864,6 +1830,57 @@ static int uniwill_kbd_led_init(struct uniwill_data *data)
>   							 &init_data);
>   }
>   
> +static int uniwill_kbd_led_init(struct uniwill_data *data)
> +{
> +	unsigned int regval;
> +	int ret;
> +
> +	if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
> +		return 0;
> +
> +	ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!(regval & CHINA_MODE)) {
> +		ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, ENABLE_CHINA_MODE);
> +		if (ret < 0)
> +			return ret;
> +	}

I forgot again what this china mode bit is doing?

But overall:

Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>

> +
> +	ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
> +	if (ret < 0)
> +		return ret;
> +
> +	regval |= KBD_APPLY;
> +	regval &= ~KBD_POWER_OFF;
> +	ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
> +	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)
> +		return uniwill_white_kbd_led_init(data);
> +
> +	return uniwill_rgb_kbd_led_init(data);
> +}
> +
>   static unsigned int uniwill_sanitize_battery_threshold(unsigned int value)
>   {
>   	/* 0 means "charging threshold not active" */

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

[Werner Sembach]

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

Re: [PATCH v2 3/3] platform/x86: uniwill-laptop: Make KBD_LED_MAX_INTENSITY configurable

[Werner Sembach]

Am 11.07.26 um 00:35 schrieb Armin Wolf:
> Some (yet to be enabled) Tuxedo devices have a maximum intensity of
> 200 instead of the default 50. Remove the hardcoded maximum intensity
> and use the device descriptor instead.
>
> Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>   drivers/platform/x86/uniwill/uniwill-acpi.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 4591ee299a90..5b8e35bf9a51 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -351,7 +351,6 @@
>   #define LED_CHANNELS		3
>   
>   #define KBD_LED_CHANNELS	3
> -#define KBD_LED_MAX_INTENSITY	50
>   
>   #define UNIWILL_FEATURE_FN_LOCK			BIT(0)
>   #define UNIWILL_FEATURE_SUPER_KEY		BIT(1)
> @@ -403,6 +402,7 @@ struct uniwill_data {
>   	struct mc_subled led_mc_subled_info[LED_CHANNELS];
>   	bool kbd_led_single_color;
>   	u8 kbd_led_max_brightness;
> +	u8 kbd_led_max_intensity;
>   	unsigned int last_kbd_status;
>   	union {
>   		struct {
> @@ -429,6 +429,7 @@ struct uniwill_device_descriptor {
>   	unsigned int features;
>   	bool kbd_led_single_color;
>   	u8 kbd_led_max_brightness;
> +	u8 kbd_led_max_intensity;
>   	u8 lightbar_max_brightness;
>   	/* Executed during driver probing */
>   	int (*probe)(struct uniwill_data *data);
> @@ -1780,8 +1781,8 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
>   		 * Make sure that the initial intensity value is not greater than
>   		 * the maximum intensity.
>   		 */
> -		if (regval > KBD_LED_MAX_INTENSITY) {
> -			regval = KBD_LED_MAX_INTENSITY;
> +		if (regval > data->kbd_led_max_intensity) {
> +			regval = data->kbd_led_max_intensity;
>   			ret = regmap_write(data->regmap, uniwill_kbd_led_channel_to_reg[i], regval);
>   			if (ret < 0)
>   				return ret;
> @@ -1793,7 +1794,7 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
>   			intensity_all_zeros = false;
>   
>   		data->kbd_led_mc_subled_info[i].intensity = regval;
> -		data->kbd_led_mc_subled_info[i].max_intensity = KBD_LED_MAX_INTENSITY;
> +		data->kbd_led_mc_subled_info[i].max_intensity = data->kbd_led_max_intensity;
>   		data->kbd_led_mc_subled_info[i].channel = i;
>   	}
>   
> @@ -2338,6 +2339,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->kbd_led_max_intensity = device_descriptor.kbd_led_max_intensity;
>   	data->lightbar_max_brightness = device_descriptor.lightbar_max_brightness;
>   
>   	/*
> @@ -2675,6 +2677,7 @@ static struct uniwill_device_descriptor machenike_l16p_descriptor __initdata = {
>   		    UNIWILL_FEATURE_USB_POWERSHARE,
>   	.kbd_led_single_color = false,
>   	.kbd_led_max_brightness = 4,
> +	.kbd_led_max_intensity = 50,
>   };
>   
>   static struct uniwill_device_descriptor lapqc71a_lapqc71b_descriptor __initdata = {
> @@ -2858,6 +2861,7 @@ static struct uniwill_device_descriptor x4sp4nal_descriptor __initdata = {
>   		    UNIWILL_FEATURE_USB_POWERSHARE,
>   	.kbd_led_single_color = true,
>   	.kbd_led_max_brightness = 2,
> +	.kbd_led_max_intensity = 50,
>   };
>   
>   static const struct dmi_system_id uniwill_dmi_table[] __initconst = {
> @@ -3355,6 +3359,7 @@ 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;
> +		device_descriptor.kbd_led_max_intensity = 50;
>   		/* Some models only support 36 brightness levels per color component */
>   		device_descriptor.lightbar_max_brightness = 200;
>   		pr_warn("Enabling potentially unsupported features\n");

lgtm

Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>

Re: [PATCH v2 1/3] platform/x86: uniwill-laptop: Split uniwill_kbd_led_init()

[Armin Wolf]

Am 15.07.26 um 17:12 schrieb Werner Sembach:

>
> Am 11.07.26 um 00:35 schrieb Armin Wolf:
>> The function uniwill_kbd_led_init() is quite large and doing multiple
>> things at once:
>> - general hardware initialisation
>> - single color keyboard backlight registration
>> - RGB keyboard backlight registration
>>
>> Move the last two things into separate functions to increase the
>> maintainability of uniwill_kbd_led_init().
>>
>> Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 123 +++++++++++---------
>>   1 file changed, 70 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c 
>> b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> index 310445583832..d27f316800f6 100644
>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> @@ -1733,7 +1733,24 @@ static enum led_brightness 
>> uniwill_kbd_led_mc_brightness_get(struct led_classdev
>>       return uniwill_kbd_led_read_brightness(data);
>>   }
>>   -static int uniwill_kbd_led_init(struct uniwill_data *data)
>> +static int uniwill_white_kbd_led_init(struct uniwill_data *data)
>> +{
>> +    struct led_init_data init_data = {
>> +        .default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT,
>> +        .devicename = DRIVER_NAME,
>> +        .devname_mandatory = true,
>> +    };
>> +
>> +    data->kbd_led_cdev.max_brightness = data->kbd_led_max_brightness;
>> +    data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
>> +    data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | 
>> LED_REJECT_NAME_CONFLICT;
>> +    data->kbd_led_cdev.brightness_set_blocking = 
>> uniwill_kbd_led_brightness_set;
>> +    data->kbd_led_cdev.brightness_get = uniwill_kbd_led_brightness_get;
>> +
>> +    return devm_led_classdev_register_ext(data->dev, 
>> &data->kbd_led_cdev, &init_data);
>> +}
>> +
>> +static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
>>   {
>>       unsigned int color_indices[KBD_LED_CHANNELS] = {
>>           LED_COLOR_ID_RED,
>> @@ -1741,6 +1758,7 @@ static int uniwill_kbd_led_init(struct 
>> uniwill_data *data)
>>           LED_COLOR_ID_BLUE,
>>       };
>>       struct led_init_data init_data = {
>> +        .default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT,
>>           .devicename = DRIVER_NAME,
>>           .devname_mandatory = true,
>>       };
>> @@ -1749,57 +1767,6 @@ static int uniwill_kbd_led_init(struct 
>> uniwill_data *data)
>>       unsigned int regval;
>>       int ret;
>>   -    if (!uniwill_device_supports(data, 
>> UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
>> -        return 0;
>> -
>> -    ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
>> -    if (ret < 0)
>> -        return ret;
>> -
>> -    if (!(regval & CHINA_MODE)) {
>> -        ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, 
>> ENABLE_CHINA_MODE);
>> -        if (ret < 0)
>> -            return ret;
>> -    }
>> -
>> -    ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
>> -    if (ret < 0)
>> -        return ret;
>> -
>> -    regval |= KBD_APPLY;
>> -    regval &= ~KBD_POWER_OFF;
>> -    ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
>> -    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) {
>> -        init_data.default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT;
>> -        data->kbd_led_cdev.max_brightness = 
>> data->kbd_led_max_brightness;
>> -        data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
>> -        data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | 
>> LED_REJECT_NAME_CONFLICT;
>> -        data->kbd_led_cdev.brightness_set_blocking = 
>> uniwill_kbd_led_brightness_set;
>> -        data->kbd_led_cdev.brightness_get = 
>> uniwill_kbd_led_brightness_get;
>> -
>> -        return devm_led_classdev_register_ext(data->dev, 
>> &data->kbd_led_cdev, &init_data);
>> -    }
>> -
>>       for (int i = 0; i < KBD_LED_CHANNELS; i++) {
>>           data->kbd_led_mc_subled_info[i].color_index = 
>> color_indices[i];
>>   @@ -1851,7 +1818,6 @@ static int uniwill_kbd_led_init(struct 
>> uniwill_data *data)
>>       if (ret < 0)
>>           return ret;
>>   -    init_data.default_label = "multicolor:" 
>> LED_FUNCTION_KBD_BACKLIGHT;
>>       data->kbd_led_mc_cdev.led_cdev.max_brightness = 
>> data->kbd_led_max_brightness;
>>       data->kbd_led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
>>       data->kbd_led_mc_cdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED | 
>> LED_REJECT_NAME_CONFLICT;
>> @@ -1864,6 +1830,57 @@ static int uniwill_kbd_led_init(struct 
>> uniwill_data *data)
>>                                &init_data);
>>   }
>>   +static int uniwill_kbd_led_init(struct uniwill_data *data)
>> +{
>> +    unsigned int regval;
>> +    int ret;
>> +
>> +    if (!uniwill_device_supports(data, 
>> UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
>> +        return 0;
>> +
>> +    ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (!(regval & CHINA_MODE)) {
>> +        ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, 
>> ENABLE_CHINA_MODE);
>> +        if (ret < 0)
>> +            return ret;
>> +    }
>
> I forgot again what this china mode bit is doing?
>
> But overall:
>
> Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>

The China mode bit _apparently_ is necessary for proper keyboard backlight control on some models,
the original control center set it unconditionally if the associated support bit is set.

Thanks,
Armin Wolf

>
>> +
>> +    ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    regval |= KBD_APPLY;
>> +    regval &= ~KBD_POWER_OFF;
>> +    ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
>> +    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)
>> +        return uniwill_white_kbd_led_init(data);
>> +
>> +    return uniwill_rgb_kbd_led_init(data);
>> +}
>> +
>>   static unsigned int uniwill_sanitize_battery_threshold(unsigned int 
>> value)
>>   {
>>       /* 0 means "charging threshold not active" */
>

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

[Armin Wolf]

Am 15.07.26 um 17:16 schrieb Werner Sembach:

>
> 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.

Good catch, i will reword this.

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

I decided against that because both feature defines would effectively control the same hardware registers.
Instead i decided to have a common code path for the whole keyboard backlight feature and use a flag for
the RGB support instead.

This also integrates better with the existing code IMHO.

Thanks,
Armin Wolf

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

Re: [PATCH v2 1/3] platform/x86: uniwill-laptop: Split uniwill_kbd_led_init()

[Werner Sembach]

Am 16.07.26 um 01:56 schrieb Armin Wolf:
> Am 15.07.26 um 17:12 schrieb Werner Sembach:
>
>>
>> Am 11.07.26 um 00:35 schrieb Armin Wolf:
>>> The function uniwill_kbd_led_init() is quite large and doing multiple
>>> things at once:
>>> - general hardware initialisation
>>> - single color keyboard backlight registration
>>> - RGB keyboard backlight registration
>>>
>>> Move the last two things into separate functions to increase the
>>> maintainability of uniwill_kbd_led_init().
>>>
>>> Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>> ---
>>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 123 +++++++++++---------
>>>   1 file changed, 70 insertions(+), 53 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c 
>>> b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> index 310445583832..d27f316800f6 100644
>>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> @@ -1733,7 +1733,24 @@ static enum led_brightness 
>>> uniwill_kbd_led_mc_brightness_get(struct led_classdev
>>>       return uniwill_kbd_led_read_brightness(data);
>>>   }
>>>   -static int uniwill_kbd_led_init(struct uniwill_data *data)
>>> +static int uniwill_white_kbd_led_init(struct uniwill_data *data)
>>> +{
>>> +    struct led_init_data init_data = {
>>> +        .default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT,
>>> +        .devicename = DRIVER_NAME,
>>> +        .devname_mandatory = true,
>>> +    };
>>> +
>>> +    data->kbd_led_cdev.max_brightness = data->kbd_led_max_brightness;
>>> +    data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
>>> +    data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | 
>>> LED_REJECT_NAME_CONFLICT;
>>> +    data->kbd_led_cdev.brightness_set_blocking = 
>>> uniwill_kbd_led_brightness_set;
>>> +    data->kbd_led_cdev.brightness_get = uniwill_kbd_led_brightness_get;
>>> +
>>> +    return devm_led_classdev_register_ext(data->dev, &data->kbd_led_cdev, 
>>> &init_data);
>>> +}
>>> +
>>> +static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
>>>   {
>>>       unsigned int color_indices[KBD_LED_CHANNELS] = {
>>>           LED_COLOR_ID_RED,
>>> @@ -1741,6 +1758,7 @@ static int uniwill_kbd_led_init(struct uniwill_data 
>>> *data)
>>>           LED_COLOR_ID_BLUE,
>>>       };
>>>       struct led_init_data init_data = {
>>> +        .default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT,
>>>           .devicename = DRIVER_NAME,
>>>           .devname_mandatory = true,
>>>       };
>>> @@ -1749,57 +1767,6 @@ static int uniwill_kbd_led_init(struct uniwill_data 
>>> *data)
>>>       unsigned int regval;
>>>       int ret;
>>>   -    if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
>>> -        return 0;
>>> -
>>> -    ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    if (!(regval & CHINA_MODE)) {
>>> -        ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, 
>>> ENABLE_CHINA_MODE);
>>> -        if (ret < 0)
>>> -            return ret;
>>> -    }
>>> -
>>> -    ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    regval |= KBD_APPLY;
>>> -    regval &= ~KBD_POWER_OFF;
>>> -    ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
>>> -    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) {
>>> -        init_data.default_label = "white:" LED_FUNCTION_KBD_BACKLIGHT;
>>> -        data->kbd_led_cdev.max_brightness = data->kbd_led_max_brightness;
>>> -        data->kbd_led_cdev.color = LED_COLOR_ID_WHITE;
>>> -        data->kbd_led_cdev.flags = LED_BRIGHT_HW_CHANGED | 
>>> LED_REJECT_NAME_CONFLICT;
>>> -        data->kbd_led_cdev.brightness_set_blocking = 
>>> uniwill_kbd_led_brightness_set;
>>> -        data->kbd_led_cdev.brightness_get = uniwill_kbd_led_brightness_get;
>>> -
>>> -        return devm_led_classdev_register_ext(data->dev, 
>>> &data->kbd_led_cdev, &init_data);
>>> -    }
>>> -
>>>       for (int i = 0; i < KBD_LED_CHANNELS; i++) {
>>>           data->kbd_led_mc_subled_info[i].color_index = color_indices[i];
>>>   @@ -1851,7 +1818,6 @@ static int uniwill_kbd_led_init(struct uniwill_data 
>>> *data)
>>>       if (ret < 0)
>>>           return ret;
>>>   -    init_data.default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT;
>>>       data->kbd_led_mc_cdev.led_cdev.max_brightness = 
>>> data->kbd_led_max_brightness;
>>>       data->kbd_led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
>>>       data->kbd_led_mc_cdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED | 
>>> LED_REJECT_NAME_CONFLICT;
>>> @@ -1864,6 +1830,57 @@ static int uniwill_kbd_led_init(struct uniwill_data 
>>> *data)
>>>                                &init_data);
>>>   }
>>>   +static int uniwill_kbd_led_init(struct uniwill_data *data)
>>> +{
>>> +    unsigned int regval;
>>> +    int ret;
>>> +
>>> +    if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
>>> +        return 0;
>>> +
>>> +    ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &regval);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    if (!(regval & CHINA_MODE)) {
>>> +        ret = regmap_set_bits(data->regmap, EC_ADDR_BIOS_OEM_2, 
>>> ENABLE_CHINA_MODE);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +    }
>>
>> I forgot again what this china mode bit is doing?
>>
>> But overall:
>>
>> Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>
>
> The China mode bit _apparently_ is necessary for proper keyboard backlight 
> control on some models,
> the original control center set it unconditionally if the associated support 
> bit is set.

ah ok, i searched our code: we actually set the the bit you only check: 
https://gitlab.com/tuxedocomputers/development/packages/tuxedo-drivers/-/blob/0b2f8c6ab7ef709343ae74b3de51dcae45357641/src/uniwill_leds.h#L296

but if the other way it what you reversed from the windows driver that's 
probably the proper solution

>
> Thanks,
> Armin Wolf
>
>>
>>> +
>>> +    ret = regmap_read(data->regmap, EC_ADDR_KBD_STATUS, &regval);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    regval |= KBD_APPLY;
>>> +    regval &= ~KBD_POWER_OFF;
>>> +    ret = regmap_write(data->regmap, EC_ADDR_KBD_STATUS, regval);
>>> +    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)
>>> +        return uniwill_white_kbd_led_init(data);
>>> +
>>> +    return uniwill_rgb_kbd_led_init(data);
>>> +}
>>> +
>>>   static unsigned int uniwill_sanitize_battery_threshold(unsigned int value)
>>>   {
>>>       /* 0 means "charging threshold not active" */
>>

Re: [PATCH v2 3/3] platform/x86: uniwill-laptop: Make KBD_LED_MAX_INTENSITY configurable

[Werner Sembach]

Am 15.07.26 um 17:19 schrieb Werner Sembach:
>
> Am 11.07.26 um 00:35 schrieb Armin Wolf:
>> Some (yet to be enabled) Tuxedo devices have a maximum intensity of
>> 200 instead of the default 50. Remove the hardcoded maximum intensity
>> and use the device descriptor instead.

sorry i gave missinformation, we actually convert down to the range 0-50 in td 
for all devices here: 
https://gitlab.com/tuxedocomputers/development/packages/tuxedo-drivers/-/blob/0b2f8c6ab7ef709343ae74b3de51dcae45357641/src/uniwill_leds.h#L106

so this patch can actually be dropped

>>
>> Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c 
>> b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> index 4591ee299a90..5b8e35bf9a51 100644
>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> @@ -351,7 +351,6 @@
>>   #define LED_CHANNELS        3
>>     #define KBD_LED_CHANNELS    3
>> -#define KBD_LED_MAX_INTENSITY    50
>>     #define UNIWILL_FEATURE_FN_LOCK            BIT(0)
>>   #define UNIWILL_FEATURE_SUPER_KEY        BIT(1)
>> @@ -403,6 +402,7 @@ struct uniwill_data {
>>       struct mc_subled led_mc_subled_info[LED_CHANNELS];
>>       bool kbd_led_single_color;
>>       u8 kbd_led_max_brightness;
>> +    u8 kbd_led_max_intensity;
>>       unsigned int last_kbd_status;
>>       union {
>>           struct {
>> @@ -429,6 +429,7 @@ struct uniwill_device_descriptor {
>>       unsigned int features;
>>       bool kbd_led_single_color;
>>       u8 kbd_led_max_brightness;
>> +    u8 kbd_led_max_intensity;
>>       u8 lightbar_max_brightness;
>>       /* Executed during driver probing */
>>       int (*probe)(struct uniwill_data *data);
>> @@ -1780,8 +1781,8 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data 
>> *data)
>>            * Make sure that the initial intensity value is not greater than
>>            * the maximum intensity.
>>            */
>> -        if (regval > KBD_LED_MAX_INTENSITY) {
>> -            regval = KBD_LED_MAX_INTENSITY;
>> +        if (regval > data->kbd_led_max_intensity) {
>> +            regval = data->kbd_led_max_intensity;
>>               ret = regmap_write(data->regmap, 
>> uniwill_kbd_led_channel_to_reg[i], regval);
>>               if (ret < 0)
>>                   return ret;
>> @@ -1793,7 +1794,7 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data 
>> *data)
>>               intensity_all_zeros = false;
>>             data->kbd_led_mc_subled_info[i].intensity = regval;
>> -        data->kbd_led_mc_subled_info[i].max_intensity = KBD_LED_MAX_INTENSITY;
>> +        data->kbd_led_mc_subled_info[i].max_intensity = 
>> data->kbd_led_max_intensity;
>>           data->kbd_led_mc_subled_info[i].channel = i;
>>       }
>>   @@ -2338,6 +2339,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->kbd_led_max_intensity = device_descriptor.kbd_led_max_intensity;
>>       data->lightbar_max_brightness = device_descriptor.lightbar_max_brightness;
>>         /*
>> @@ -2675,6 +2677,7 @@ static struct uniwill_device_descriptor 
>> machenike_l16p_descriptor __initdata = {
>>               UNIWILL_FEATURE_USB_POWERSHARE,
>>       .kbd_led_single_color = false,
>>       .kbd_led_max_brightness = 4,
>> +    .kbd_led_max_intensity = 50,
>>   };
>>     static struct uniwill_device_descriptor lapqc71a_lapqc71b_descriptor 
>> __initdata = {
>> @@ -2858,6 +2861,7 @@ static struct uniwill_device_descriptor 
>> x4sp4nal_descriptor __initdata = {
>>               UNIWILL_FEATURE_USB_POWERSHARE,
>>       .kbd_led_single_color = true,
>>       .kbd_led_max_brightness = 2,
>> +    .kbd_led_max_intensity = 50,
>>   };
>>     static const struct dmi_system_id uniwill_dmi_table[] __initconst = {
>> @@ -3355,6 +3359,7 @@ 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;
>> +        device_descriptor.kbd_led_max_intensity = 50;
>>           /* Some models only support 36 brightness levels per color 
>> component */
>>           device_descriptor.lightbar_max_brightness = 200;
>>           pr_warn("Enabling potentially unsupported features\n");
>
> lgtm
>
> Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>
>

Re: [PATCH v2 3/3] platform/x86: uniwill-laptop: Make KBD_LED_MAX_INTENSITY configurable

[Armin Wolf]

Am 16.07.26 um 17:12 schrieb Werner Sembach:

>
> Am 15.07.26 um 17:19 schrieb Werner Sembach:
>>
>> Am 11.07.26 um 00:35 schrieb Armin Wolf:
>>> Some (yet to be enabled) Tuxedo devices have a maximum intensity of
>>> 200 instead of the default 50. Remove the hardcoded maximum intensity
>>> and use the device descriptor instead.
>
> sorry i gave missinformation, we actually convert down to the range 
> 0-50 in td for all devices here: 
> https://gitlab.com/tuxedocomputers/development/packages/tuxedo-drivers/-/blob/0b2f8c6ab7ef709343ae74b3de51dcae45357641/src/uniwill_leds.h#L106
>
> so this patch can actually be dropped
>
Ok.

Thanks,
Armin Wolf

>>>
>>> Suggested-by: Werner Sembach <wse@tuxedocomputers.com>
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>> ---
>>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 13 +++++++++----
>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c 
>>> b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> index 4591ee299a90..5b8e35bf9a51 100644
>>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> @@ -351,7 +351,6 @@
>>>   #define LED_CHANNELS        3
>>>     #define KBD_LED_CHANNELS    3
>>> -#define KBD_LED_MAX_INTENSITY    50
>>>     #define UNIWILL_FEATURE_FN_LOCK            BIT(0)
>>>   #define UNIWILL_FEATURE_SUPER_KEY        BIT(1)
>>> @@ -403,6 +402,7 @@ struct uniwill_data {
>>>       struct mc_subled led_mc_subled_info[LED_CHANNELS];
>>>       bool kbd_led_single_color;
>>>       u8 kbd_led_max_brightness;
>>> +    u8 kbd_led_max_intensity;
>>>       unsigned int last_kbd_status;
>>>       union {
>>>           struct {
>>> @@ -429,6 +429,7 @@ struct uniwill_device_descriptor {
>>>       unsigned int features;
>>>       bool kbd_led_single_color;
>>>       u8 kbd_led_max_brightness;
>>> +    u8 kbd_led_max_intensity;
>>>       u8 lightbar_max_brightness;
>>>       /* Executed during driver probing */
>>>       int (*probe)(struct uniwill_data *data);
>>> @@ -1780,8 +1781,8 @@ static int uniwill_rgb_kbd_led_init(struct 
>>> uniwill_data *data)
>>>            * Make sure that the initial intensity value is not 
>>> greater than
>>>            * the maximum intensity.
>>>            */
>>> -        if (regval > KBD_LED_MAX_INTENSITY) {
>>> -            regval = KBD_LED_MAX_INTENSITY;
>>> +        if (regval > data->kbd_led_max_intensity) {
>>> +            regval = data->kbd_led_max_intensity;
>>>               ret = regmap_write(data->regmap, 
>>> uniwill_kbd_led_channel_to_reg[i], regval);
>>>               if (ret < 0)
>>>                   return ret;
>>> @@ -1793,7 +1794,7 @@ static int uniwill_rgb_kbd_led_init(struct 
>>> uniwill_data *data)
>>>               intensity_all_zeros = false;
>>>             data->kbd_led_mc_subled_info[i].intensity = regval;
>>> -        data->kbd_led_mc_subled_info[i].max_intensity = 
>>> KBD_LED_MAX_INTENSITY;
>>> +        data->kbd_led_mc_subled_info[i].max_intensity = 
>>> data->kbd_led_max_intensity;
>>>           data->kbd_led_mc_subled_info[i].channel = i;
>>>       }
>>>   @@ -2338,6 +2339,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->kbd_led_max_intensity = 
>>> device_descriptor.kbd_led_max_intensity;
>>>       data->lightbar_max_brightness = 
>>> device_descriptor.lightbar_max_brightness;
>>>         /*
>>> @@ -2675,6 +2677,7 @@ static struct uniwill_device_descriptor 
>>> machenike_l16p_descriptor __initdata = {
>>>               UNIWILL_FEATURE_USB_POWERSHARE,
>>>       .kbd_led_single_color = false,
>>>       .kbd_led_max_brightness = 4,
>>> +    .kbd_led_max_intensity = 50,
>>>   };
>>>     static struct uniwill_device_descriptor 
>>> lapqc71a_lapqc71b_descriptor __initdata = {
>>> @@ -2858,6 +2861,7 @@ static struct uniwill_device_descriptor 
>>> x4sp4nal_descriptor __initdata = {
>>>               UNIWILL_FEATURE_USB_POWERSHARE,
>>>       .kbd_led_single_color = true,
>>>       .kbd_led_max_brightness = 2,
>>> +    .kbd_led_max_intensity = 50,
>>>   };
>>>     static const struct dmi_system_id uniwill_dmi_table[] 
>>> __initconst = {
>>> @@ -3355,6 +3359,7 @@ 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;
>>> +        device_descriptor.kbd_led_max_intensity = 50;
>>>           /* Some models only support 36 brightness levels per color 
>>> component */
>>>           device_descriptor.lightbar_max_brightness = 200;
>>>           pr_warn("Enabling potentially unsupported features\n");
>>
>> lgtm
>>
>> Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>
>>
>