Re: [PATCH 1/7] platform/x86: uniwill-laptop: Add keyboard backlight support
From: Werner Sembach
Date: Thu Jul 09 2026 - 09:29:10 EST
[...]
Good point, in this case using the device descriptor for this feature detectionyeah the original cc and tuxedo-drivers has these small lists all over the place ... also don't like it thereGood point, but in this case i think having this small device list is OK, because the original control center+
+static int uniwill_kbd_led_init(struct uniwill_data *data)
+{
+ unsigned int color_indices[KBD_LED_CHANNELS] = {
+ LED_COLOR_ID_RED,
+ LED_COLOR_ID_GREEN,
+ LED_COLOR_ID_BLUE,
+ };
+ struct led_init_data init_data = {
+ .devicename = DRIVER_NAME,
+ .devname_mandatory = true,
+ };
+ bool intensity_all_zeros = true;
+ bool needs_trigger = false;
+ 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, ®val);
+ 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, ®val);
+ 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];
+
+ ret = regmap_read(data->regmap, uniwill_kbd_led_channel_to_reg[i], ®val);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * 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;
+ ret = regmap_write(data->regmap, uniwill_kbd_led_channel_to_reg[i], regval);
+ if (ret < 0)
+ return ret;
+
+ needs_trigger = true;
+ }
+
+ if (regval)
+ 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].channel = i;
+ }
+
+ /* See uniwill_kbd_led_mc_brightness_set() for an explaination. */
+ if (intensity_all_zeros) {
+ for (int i = 0; i < KBD_LED_CHANNELS; i++) {
+ data->kbd_led_mc_subled_info[i].intensity = 1;
+ ret = regmap_write(data->regmap, uniwill_kbd_led_channel_to_reg[i], 1);
+ if (ret < 0)
+ return ret;
+ }
+
+ needs_trigger = true;
+ }
+
+ if (needs_trigger) {
+ ret = regmap_write_bits(data->regmap, EC_ADDR_TRIGGER, RGB_APPLY_COLOR,
+ RGB_APPLY_COLOR);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = devm_mutex_init(data->dev, &data->kbd_rgb_led_lock);
+ 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;
+ data->kbd_led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_kbd_led_mc_brightness_set;
+ data->kbd_led_mc_cdev.led_cdev.brightness_get = uniwill_kbd_led_mc_brightness_get;
+ data->kbd_led_mc_cdev.subled_info = data->kbd_led_mc_subled_info;
+ data->kbd_led_mc_cdev.num_colors = KBD_LED_CHANNELS;
+
+ return devm_led_classdev_multicolor_register_ext(data->dev, &data->kbd_led_mc_cdev,
+ &init_data);
+}
+
Sorry I know I'm way to late with my comment, but I'm not at all a fan of how complex this whole function is:
- When we have to set max brightness on a per device basis anyway, we can also set rgb or white only there and can skip this whole detection mechanism with the supported bits that are unreliable anyway
- we have a 2nd "supported devices/features list" inside this function which is abstracting away from the actual supported feature list
Should i try to spin up something much simpler, but that requires explicit setting in the device descriptor?
Best regards
Werner
does something similar.
If we encounter a device that does not work with this strategy then we can indeed
move the code into the device descriptor.
later we might not be able to get the information anymore which devices are rgb and which are white only when the supported devices lists grows, then we are stuck to reading the feature bits.
best regards,
Werner
might indeed be better in the long term.
Do you want to send the patches or should i do it?
I will not stop you if you have time to do it ^^
you are probably quicker in working on your own code
i would do a
UNIWILL_FEATURE_KBL_WHITE_ONLY and a
UNIWILL_FEATURE_KBL_ONE_ZONE_RGB (there are also Uniwill devices with per-key RGB, but that is controlled via a usb device)
also required is a way to set max intensity via the device descriptor, most tuxedo devices have 200 as max value there and not 50
best regards,
Werner
[...]
Thanks,
Armin Wolf