diff mbox series

[v4,09/11] HID: asus: add basic RGB support

Message ID 20250324210151.6042-10-lkml@antheas.dev (mailing list archive)
State Superseded
Delegated to: Jiri Kosina
Headers show
Series HID: asus: Add RGB Support to Asus Z13, Ally, unify backlight asus-wmi, and Z13 QOL | expand

Commit Message

Antheas Kapenekakis March 24, 2025, 9:01 p.m. UTC 
Adds basic RGB support to hid-asus through multi-index. The interface
works quite well, but has not gone through much stability testing.
Applied on demand, if userspace does not touch the RGB sysfs, not
even initialization is done. Ensuring compatibility with existing
userspace programs.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 169 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 155 insertions(+), 14 deletions(-)

Comments

kernel test robot March 25, 2025, 6:32 a.m. UTC | #1 
Hi Antheas,

kernel test robot noticed the following build errors:

[auto build test ERROR on 38fec10eb60d687e30c8c6b5420d86e8149f7557]

url:    https://github.com/intel-lab-lkp/linux/commits/Antheas-Kapenekakis/HID-asus-refactor-init-sequence-per-spec/20250325-051852
base:   38fec10eb60d687e30c8c6b5420d86e8149f7557
patch link:    https://lore.kernel.org/r/20250324210151.6042-10-lkml%40antheas.dev
patch subject: [PATCH v4 09/11] HID: asus: add basic RGB support
config: arm64-randconfig-004-20250325 (https://download.01.org/0day-ci/archive/20250325/202503251335.BQVOomT2-lkp@intel.com/config)
compiler: clang version 20.1.1 (https://github.com/llvm/llvm-project 424c2d9b7e4de40d0804dd374721e6411c27d1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250325/202503251335.BQVOomT2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503251335.BQVOomT2-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "devm_led_classdev_multicolor_register_ext" [drivers/hid/hid-asus.ko] undefined!
kernel test robot March 25, 2025, 6:32 a.m. UTC | #2 
Hi Antheas,

kernel test robot noticed the following build errors:

[auto build test ERROR on 38fec10eb60d687e30c8c6b5420d86e8149f7557]

url:    https://github.com/intel-lab-lkp/linux/commits/Antheas-Kapenekakis/HID-asus-refactor-init-sequence-per-spec/20250325-051852
base:   38fec10eb60d687e30c8c6b5420d86e8149f7557
patch link:    https://lore.kernel.org/r/20250324210151.6042-10-lkml%40antheas.dev
patch subject: [PATCH v4 09/11] HID: asus: add basic RGB support
config: riscv-randconfig-002-20250325 (https://download.01.org/0day-ci/archive/20250325/202503251316.lPXAIXIV-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250325/202503251316.lPXAIXIV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503251316.lPXAIXIV-lkp@intel.com/

All errors (new ones prefixed by >>):

   riscv64-linux-ld: drivers/hid/hid-asus.o: in function `asus_kbd_register_leds':
>> drivers/hid/hid-asus.c:676:(.text+0x23f8): undefined reference to `devm_led_classdev_multicolor_register_ext'


vim +676 drivers/hid/hid-asus.c

312a522531f6e5 Antheas Kapenekakis 2025-03-24  645  
af22a610bc3850 Carlo Caione        2017-04-06  646  static int asus_kbd_register_leds(struct hid_device *hdev)
af22a610bc3850 Carlo Caione        2017-04-06  647  {
af22a610bc3850 Carlo Caione        2017-04-06  648  	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
af22a610bc3850 Carlo Caione        2017-04-06  649  	unsigned char kbd_func;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  650  	struct asus_kbd_leds *leds;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  651  	bool no_led;
af22a610bc3850 Carlo Caione        2017-04-06  652  	int ret;
af22a610bc3850 Carlo Caione        2017-04-06  653  
2c82a7b20f7b7a Luke D. Jones       2024-04-16  654  	ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
2c82a7b20f7b7a Luke D. Jones       2024-04-16  655  	if (ret < 0)
2c82a7b20f7b7a Luke D. Jones       2024-04-16  656  		return ret;
2c82a7b20f7b7a Luke D. Jones       2024-04-16  657  
3ebfeb18b44e01 Antheas Kapenekakis 2025-03-24  658  	/* Get keyboard functions */
3ebfeb18b44e01 Antheas Kapenekakis 2025-03-24  659  	ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
b92b80246e0626 Luke D Jones        2020-10-27  660  	if (ret < 0)
b92b80246e0626 Luke D Jones        2020-10-27  661  		return ret;
53078a736fbc60 Luke D. Jones       2025-01-11  662  
53078a736fbc60 Luke D. Jones       2025-01-11  663  	if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
53078a736fbc60 Luke D. Jones       2025-01-11  664  		ret = asus_kbd_disable_oobe(hdev);
53078a736fbc60 Luke D. Jones       2025-01-11  665  		if (ret < 0)
53078a736fbc60 Luke D. Jones       2025-01-11  666  			return ret;
53078a736fbc60 Luke D. Jones       2025-01-11  667  	}
af22a610bc3850 Carlo Caione        2017-04-06  668  
af22a610bc3850 Carlo Caione        2017-04-06  669  	/* Check for backlight support */
af22a610bc3850 Carlo Caione        2017-04-06  670  	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
af22a610bc3850 Carlo Caione        2017-04-06  671  		return -ENODEV;
af22a610bc3850 Carlo Caione        2017-04-06  672  
af22a610bc3850 Carlo Caione        2017-04-06  673  	drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
af22a610bc3850 Carlo Caione        2017-04-06  674  					      sizeof(struct asus_kbd_leds),
af22a610bc3850 Carlo Caione        2017-04-06  675  					      GFP_KERNEL);
af22a610bc3850 Carlo Caione        2017-04-06 @676  	if (!drvdata->kbd_backlight)
af22a610bc3850 Carlo Caione        2017-04-06  677  		return -ENOMEM;
af22a610bc3850 Carlo Caione        2017-04-06  678  
312a522531f6e5 Antheas Kapenekakis 2025-03-24  679  	leds = drvdata->kbd_backlight;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  680  	leds->removed = false;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  681  	leds->brightness = 3;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  682  	leds->hdev = hdev;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  683  	leds->listener.brightness_set = asus_kbd_listener_set;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  684  
312a522531f6e5 Antheas Kapenekakis 2025-03-24  685  	leds->rgb_colors[0] = 0;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  686  	leds->rgb_colors[1] = 0;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  687  	leds->rgb_colors[2] = 0;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  688  	leds->rgb_init = true;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  689  	leds->rgb_set = false;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  690  	leds->mc_led.led_cdev.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
312a522531f6e5 Antheas Kapenekakis 2025-03-24  691  					"asus-%s:rgb:peripheral",
312a522531f6e5 Antheas Kapenekakis 2025-03-24  692  					strlen(hdev->uniq) ?
312a522531f6e5 Antheas Kapenekakis 2025-03-24  693  					hdev->uniq : dev_name(&hdev->dev));
312a522531f6e5 Antheas Kapenekakis 2025-03-24  694  	leds->mc_led.led_cdev.flags = LED_BRIGHT_HW_CHANGED;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  695  	leds->mc_led.led_cdev.max_brightness = 3,
312a522531f6e5 Antheas Kapenekakis 2025-03-24  696  	leds->mc_led.led_cdev.brightness_set = asus_kbd_brightness_set,
312a522531f6e5 Antheas Kapenekakis 2025-03-24  697  	leds->mc_led.led_cdev.brightness_get = asus_kbd_brightness_get,
312a522531f6e5 Antheas Kapenekakis 2025-03-24  698  	leds->mc_led.subled_info = leds->subled_info,
312a522531f6e5 Antheas Kapenekakis 2025-03-24  699  	leds->mc_led.num_colors = ARRAY_SIZE(leds->subled_info),
312a522531f6e5 Antheas Kapenekakis 2025-03-24  700  	leds->subled_info[0].color_index = LED_COLOR_ID_RED;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  701  	leds->subled_info[1].color_index = LED_COLOR_ID_GREEN;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  702  	leds->subled_info[2].color_index = LED_COLOR_ID_BLUE;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  703  
312a522531f6e5 Antheas Kapenekakis 2025-03-24  704  	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_work);
315c537068a13f Pietro Borrello     2023-02-12  705  	spin_lock_init(&drvdata->kbd_backlight->lock);
af22a610bc3850 Carlo Caione        2017-04-06  706  
d37db2009c913c Antheas Kapenekakis 2025-03-24  707  	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
312a522531f6e5 Antheas Kapenekakis 2025-03-24  708  	no_led = !!ret;
d37db2009c913c Antheas Kapenekakis 2025-03-24  709  
312a522531f6e5 Antheas Kapenekakis 2025-03-24  710  	if (drvdata->quirks & QUIRK_ROG_NKEY_RGB) {
312a522531f6e5 Antheas Kapenekakis 2025-03-24  711  		ret = devm_led_classdev_multicolor_register(
312a522531f6e5 Antheas Kapenekakis 2025-03-24  712  			&hdev->dev, &leds->mc_led);
312a522531f6e5 Antheas Kapenekakis 2025-03-24  713  		if (!ret)
312a522531f6e5 Antheas Kapenekakis 2025-03-24  714  			leds->rgb_registered = true;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  715  		no_led &= !!ret;
312a522531f6e5 Antheas Kapenekakis 2025-03-24  716  	}
312a522531f6e5 Antheas Kapenekakis 2025-03-24  717  
312a522531f6e5 Antheas Kapenekakis 2025-03-24  718  	if (no_led) {
af22a610bc3850 Carlo Caione        2017-04-06  719  		/* No need to have this still around */
af22a610bc3850 Carlo Caione        2017-04-06  720  		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
af22a610bc3850 Carlo Caione        2017-04-06  721  	}
af22a610bc3850 Carlo Caione        2017-04-06  722  
312a522531f6e5 Antheas Kapenekakis 2025-03-24  723  	return no_led ? -ENODEV : 0;
af22a610bc3850 Carlo Caione        2017-04-06  724  }
af22a610bc3850 Carlo Caione        2017-04-06  725
Antheas Kapenekakis March 25, 2025, 8:52 a.m. UTC | #3 
My email is not doing too well it seems. It is not just yours Luke.
Hopefully my shared host does not bite me for abusing the IP

On Tue, 25 Mar 2025 at 07:34, kernel test robot <lkp@intel.com> wrote:
>
> Hi Antheas,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 38fec10eb60d687e30c8c6b5420d86e8149f7557]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Antheas-Kapenekakis/HID-asus-refactor-init-sequence-per-spec/20250325-051852
> base:   38fec10eb60d687e30c8c6b5420d86e8149f7557
> patch link:    https://lore.kernel.org/r/20250324210151.6042-10-lkml%40antheas.dev
> patch subject: [PATCH v4 09/11] HID: asus: add basic RGB support
> config: riscv-randconfig-002-20250325 (https://download.01.org/0day-ci/archive/20250325/202503251316.lPXAIXIV-lkp@intel.com/config)
> compiler: riscv64-linux-gcc (GCC) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250325/202503251316.lPXAIXIV-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202503251316.lPXAIXIV-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>    riscv64-linux-ld: drivers/hid/hid-asus.o: in function `asus_kbd_register_leds':
> >> drivers/hid/hid-asus.c:676:(.text+0x23f8): undefined reference to `devm_led_classdev_multicolor_register_ext'
>

Since I have been getting this error by test robot often, what is the
canonical way to check that KConfig is correct before sending patches?

I will try to fix the KConfig and send it later today


Antheas

>
> vim +676 drivers/hid/hid-asus.c
>
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  645
> af22a610bc3850 Carlo Caione        2017-04-06  646  static int asus_kbd_register_leds(struct hid_device *hdev)
> af22a610bc3850 Carlo Caione        2017-04-06  647  {
> af22a610bc3850 Carlo Caione        2017-04-06  648      struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> af22a610bc3850 Carlo Caione        2017-04-06  649      unsigned char kbd_func;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  650      struct asus_kbd_leds *leds;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  651      bool no_led;
> af22a610bc3850 Carlo Caione        2017-04-06  652      int ret;
> af22a610bc3850 Carlo Caione        2017-04-06  653
> 2c82a7b20f7b7a Luke D. Jones       2024-04-16  654      ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> 2c82a7b20f7b7a Luke D. Jones       2024-04-16  655      if (ret < 0)
> 2c82a7b20f7b7a Luke D. Jones       2024-04-16  656              return ret;
> 2c82a7b20f7b7a Luke D. Jones       2024-04-16  657
> 3ebfeb18b44e01 Antheas Kapenekakis 2025-03-24  658      /* Get keyboard functions */
> 3ebfeb18b44e01 Antheas Kapenekakis 2025-03-24  659      ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> b92b80246e0626 Luke D Jones        2020-10-27  660      if (ret < 0)
> b92b80246e0626 Luke D Jones        2020-10-27  661              return ret;
> 53078a736fbc60 Luke D. Jones       2025-01-11  662
> 53078a736fbc60 Luke D. Jones       2025-01-11  663      if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> 53078a736fbc60 Luke D. Jones       2025-01-11  664              ret = asus_kbd_disable_oobe(hdev);
> 53078a736fbc60 Luke D. Jones       2025-01-11  665              if (ret < 0)
> 53078a736fbc60 Luke D. Jones       2025-01-11  666                      return ret;
> 53078a736fbc60 Luke D. Jones       2025-01-11  667      }
> af22a610bc3850 Carlo Caione        2017-04-06  668
> af22a610bc3850 Carlo Caione        2017-04-06  669      /* Check for backlight support */
> af22a610bc3850 Carlo Caione        2017-04-06  670      if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> af22a610bc3850 Carlo Caione        2017-04-06  671              return -ENODEV;
> af22a610bc3850 Carlo Caione        2017-04-06  672
> af22a610bc3850 Carlo Caione        2017-04-06  673      drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> af22a610bc3850 Carlo Caione        2017-04-06  674                                            sizeof(struct asus_kbd_leds),
> af22a610bc3850 Carlo Caione        2017-04-06  675                                            GFP_KERNEL);
> af22a610bc3850 Carlo Caione        2017-04-06 @676      if (!drvdata->kbd_backlight)
> af22a610bc3850 Carlo Caione        2017-04-06  677              return -ENOMEM;
> af22a610bc3850 Carlo Caione        2017-04-06  678
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  679      leds = drvdata->kbd_backlight;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  680      leds->removed = false;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  681      leds->brightness = 3;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  682      leds->hdev = hdev;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  683      leds->listener.brightness_set = asus_kbd_listener_set;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  684
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  685      leds->rgb_colors[0] = 0;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  686      leds->rgb_colors[1] = 0;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  687      leds->rgb_colors[2] = 0;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  688      leds->rgb_init = true;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  689      leds->rgb_set = false;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  690      leds->mc_led.led_cdev.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  691                                      "asus-%s:rgb:peripheral",
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  692                                      strlen(hdev->uniq) ?
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  693                                      hdev->uniq : dev_name(&hdev->dev));
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  694      leds->mc_led.led_cdev.flags = LED_BRIGHT_HW_CHANGED;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  695      leds->mc_led.led_cdev.max_brightness = 3,
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  696      leds->mc_led.led_cdev.brightness_set = asus_kbd_brightness_set,
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  697      leds->mc_led.led_cdev.brightness_get = asus_kbd_brightness_get,
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  698      leds->mc_led.subled_info = leds->subled_info,
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  699      leds->mc_led.num_colors = ARRAY_SIZE(leds->subled_info),
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  700      leds->subled_info[0].color_index = LED_COLOR_ID_RED;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  701      leds->subled_info[1].color_index = LED_COLOR_ID_GREEN;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  702      leds->subled_info[2].color_index = LED_COLOR_ID_BLUE;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  703
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  704      INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_work);
> 315c537068a13f Pietro Borrello     2023-02-12  705      spin_lock_init(&drvdata->kbd_backlight->lock);
> af22a610bc3850 Carlo Caione        2017-04-06  706
> d37db2009c913c Antheas Kapenekakis 2025-03-24  707      ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  708      no_led = !!ret;
> d37db2009c913c Antheas Kapenekakis 2025-03-24  709
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  710      if (drvdata->quirks & QUIRK_ROG_NKEY_RGB) {
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  711              ret = devm_led_classdev_multicolor_register(
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  712                      &hdev->dev, &leds->mc_led);
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  713              if (!ret)
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  714                      leds->rgb_registered = true;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  715              no_led &= !!ret;
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  716      }
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  717
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  718      if (no_led) {
> af22a610bc3850 Carlo Caione        2017-04-06  719              /* No need to have this still around */
> af22a610bc3850 Carlo Caione        2017-04-06  720              devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> af22a610bc3850 Carlo Caione        2017-04-06  721      }
> af22a610bc3850 Carlo Caione        2017-04-06  722
> 312a522531f6e5 Antheas Kapenekakis 2025-03-24  723      return no_led ? -ENODEV : 0;
> af22a610bc3850 Carlo Caione        2017-04-06  724  }
> af22a610bc3850 Carlo Caione        2017-04-06  725
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
Jiri Kosina March 25, 2025, 12:11 p.m. UTC | #4 
On Tue, 25 Mar 2025, Antheas Kapenekakis wrote:

> > [auto build test ERROR on 38fec10eb60d687e30c8c6b5420d86e8149f7557]
> >
> > url:    https://github.com/intel-lab-lkp/linux/commits/Antheas-Kapenekakis/HID-asus-refactor-init-sequence-per-spec/20250325-051852
> > base:   38fec10eb60d687e30c8c6b5420d86e8149f7557
> > patch link:    https://lore.kernel.org/r/20250324210151.6042-10-lkml%40antheas.dev
> > patch subject: [PATCH v4 09/11] HID: asus: add basic RGB support
> > config: riscv-randconfig-002-20250325 (https://download.01.org/0day-ci/archive/20250325/202503251316.lPXAIXIV-lkp@intel.com/config)
> > compiler: riscv64-linux-gcc (GCC) 14.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250325/202503251316.lPXAIXIV-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202503251316.lPXAIXIV-lkp@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> >    riscv64-linux-ld: drivers/hid/hid-asus.o: in function `asus_kbd_register_leds':
> > >> drivers/hid/hid-asus.c:676:(.text+0x23f8): undefined reference to `devm_led_classdev_multicolor_register_ext'
> >
> 
> Since I have been getting this error by test robot often, what is the
> canonical way to check that KConfig is correct before sending patches?
> 
> I will try to fix the KConfig and send it later today

You either need to add driver's dependency on LEDS_CLASS_MULTICOLOR, or 
ifdef those parts out in case it's not set.

Thanks,
Ilpo Järvinen March 25, 2025, 5:02 p.m. UTC | #5 
On Mon, 24 Mar 2025, Antheas Kapenekakis wrote:

> Adds basic RGB support to hid-asus through multi-index. The interface
> works quite well, but has not gone through much stability testing.
> Applied on demand, if userspace does not touch the RGB sysfs, not
> even initialization is done. Ensuring compatibility with existing
> userspace programs.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 169 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 155 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 905453a4eb5b7..3ac1e2dea45bb 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -30,6 +30,7 @@
>  #include <linux/input/mt.h>
>  #include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
>  #include <linux/power_supply.h>
> +#include <linux/led-class-multicolor.h>
>  #include <linux/leds.h>
>  
>  #include "hid-ids.h"
> @@ -85,6 +86,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
>  #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
>  #define QUIRK_HANDLE_GENERIC		BIT(13)
> +#define QUIRK_ROG_NKEY_RGB		BIT(14)
>  
>  #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
>  						 QUIRK_NO_INIT_REPORTS | \
> @@ -97,9 +99,15 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  
>  struct asus_kbd_leds {
>  	struct asus_hid_listener listener;
> +	struct led_classdev_mc mc_led;
> +	struct mc_subled subled_info[3];
>  	struct hid_device *hdev;
>  	struct work_struct work;
>  	unsigned int brightness;
> +	uint8_t rgb_colors[3];

u8

> +	bool rgb_init;
> +	bool rgb_set;
> +	bool rgb_registered;
>  	spinlock_t lock;
>  	bool removed;
>  };
> @@ -504,23 +512,67 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
>  	spin_unlock_irqrestore(&led->lock, flags);
>  }
>  
> -static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
> +static void do_asus_kbd_backlight_set(struct asus_kbd_leds *led, int brightness)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&led->lock, flags);
> +	led->brightness = brightness;
> +	spin_unlock_irqrestore(&led->lock, flags);
> +
> +	asus_schedule_work(led);
> +}
> +
> +static void asus_kbd_listener_set(struct asus_hid_listener *listener,
>  				   int brightness)
>  {
>  	struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
>  						 listener);
> +	do_asus_kbd_backlight_set(led, brightness);
> +	if (led->rgb_registered) {
> +		led->mc_led.led_cdev.brightness = brightness;
> +		led_classdev_notify_brightness_hw_changed(&led->mc_led.led_cdev,
> +							  brightness);
> +	}
> +}
> +
> +static void asus_kbd_brightness_set(struct led_classdev *led_cdev,
> +				    enum led_brightness brightness)
> +{
> +	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
> +	struct asus_kbd_leds *led = container_of(mc_cdev, struct asus_kbd_leds,
> +						 mc_led);
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&led->lock, flags);
> -	led->brightness = brightness;
> +	led->rgb_colors[0] = mc_cdev->subled_info[0].intensity;
> +	led->rgb_colors[1] = mc_cdev->subled_info[1].intensity;
> +	led->rgb_colors[2] = mc_cdev->subled_info[2].intensity;
> +	led->rgb_set = true;
>  	spin_unlock_irqrestore(&led->lock, flags);
>  
> -	asus_schedule_work(led);
> +	do_asus_kbd_backlight_set(led, brightness);
> +}
> +
> +static enum led_brightness asus_kbd_brightness_get(struct led_classdev *led_cdev)
> +{
> +	struct led_classdev_mc *mc_led;
> +	struct asus_kbd_leds *led;
> +	enum led_brightness brightness;
> +	unsigned long flags;
> +
> +	mc_led = lcdev_to_mccdev(led_cdev);
> +	led = container_of(mc_led, struct asus_kbd_leds, mc_led);
> +
> +	spin_lock_irqsave(&led->lock, flags);
> +	brightness = led->brightness;
> +	spin_unlock_irqrestore(&led->lock, flags);
> +
> +	return brightness;
>  }
>  
> -static void asus_kbd_backlight_work(struct work_struct *work)
> +static void asus_kbd_backlight_work(struct asus_kbd_leds *led)
>  {
> -	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
>  	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
>  	int ret;
>  	unsigned long flags;
> @@ -534,10 +586,69 @@ static void asus_kbd_backlight_work(struct work_struct *work)
>  		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
>  }
>  
> +static void asus_kbd_rgb_work(struct asus_kbd_leds *led)
> +{
> +	u8 rgb_buf[][7] = {

Magic 7 should be named with a define.

> +		{ FEATURE_KBD_LED_REPORT_ID1, 0xB3 }, /* set mode */
> +		{ FEATURE_KBD_LED_REPORT_ID1, 0xB5 }, /* apply mode */
> +		{ FEATURE_KBD_LED_REPORT_ID1, 0xB4 }, /* save to mem */
> +	};
> +	unsigned long flags;
> +	uint8_t colors[3];

uint*_t should be used only for uapi. Please use u8 for in kernel 
variables.

> +	bool rgb_init, rgb_set;
> +	int ret;
> +
> +	spin_lock_irqsave(&led->lock, flags);
> +	rgb_init = led->rgb_init;
> +	rgb_set = led->rgb_set;
> +	led->rgb_set = false;
> +	colors[0] = led->rgb_colors[0];
> +	colors[1] = led->rgb_colors[1];
> +	colors[2] = led->rgb_colors[2];
> +	spin_unlock_irqrestore(&led->lock, flags);
> +
> +	if (!rgb_set)
> +		return;
> +
> +	if (rgb_init) {
> +		ret = asus_kbd_init(led->hdev, FEATURE_KBD_LED_REPORT_ID1);
> +		if (ret < 0) {
> +			hid_err(led->hdev, "Asus failed to init RGB: %d\n", ret);
> +			return;
> +		}
> +		spin_lock_irqsave(&led->lock, flags);
> +		led->rgb_init = false;
> +		spin_unlock_irqrestore(&led->lock, flags);
> +	}
> +
> +	/* Protocol is: 54b3 zone (0=all) mode (0=solid) RGB */
> +	rgb_buf[0][4] = colors[0];
> +	rgb_buf[0][5] = colors[1];
> +	rgb_buf[0][6] = colors[2];
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(rgb_buf); i++) {

Add include for ARRAY_SIZE()

> +		ret = asus_kbd_set_report(led->hdev, rgb_buf[i], sizeof(rgb_buf[i]));
> +		if (ret < 0) {
> +			hid_err(led->hdev, "Asus failed to set RGB: %d\n", ret);
> +			return;
> +		}
> +	}
> +}
> +
> +static void asus_kbd_work(struct work_struct *work)
> +{
> +	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds,
> +						 work);
> +	asus_kbd_backlight_work(led);
> +	asus_kbd_rgb_work(led);
> +}
> +
>  static int asus_kbd_register_leds(struct hid_device *hdev)
>  {
>  	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>  	unsigned char kbd_func;
> +	struct asus_kbd_leds *leds;
> +	bool no_led;
>  	int ret;
>  
>  	ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> @@ -565,21 +676,51 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>  	if (!drvdata->kbd_backlight)
>  		return -ENOMEM;
>  
> -	drvdata->kbd_backlight->removed = false;
> -	drvdata->kbd_backlight->brightness = 0;
> -	drvdata->kbd_backlight->hdev = hdev;
> -	drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
> -	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
> +	leds = drvdata->kbd_backlight;
> +	leds->removed = false;
> +	leds->brightness = 3;

Use the max brightness define here?

> +	leds->hdev = hdev;
> +	leds->listener.brightness_set = asus_kbd_listener_set;
> +
> +	leds->rgb_colors[0] = 0;
> +	leds->rgb_colors[1] = 0;
> +	leds->rgb_colors[2] = 0;
> +	leds->rgb_init = true;
> +	leds->rgb_set = false;
> +	leds->mc_led.led_cdev.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
> +					"asus-%s:rgb:peripheral",
> +					strlen(hdev->uniq) ?

hdev->uniq[0] ?

> +					hdev->uniq : dev_name(&hdev->dev));
> +	leds->mc_led.led_cdev.flags = LED_BRIGHT_HW_CHANGED;
> +	leds->mc_led.led_cdev.max_brightness = 3,

Max brightness define.

> +	leds->mc_led.led_cdev.brightness_set = asus_kbd_brightness_set,
> +	leds->mc_led.led_cdev.brightness_get = asus_kbd_brightness_get,
> +	leds->mc_led.subled_info = leds->subled_info,
> +	leds->mc_led.num_colors = ARRAY_SIZE(leds->subled_info),
> +	leds->subled_info[0].color_index = LED_COLOR_ID_RED;
> +	leds->subled_info[1].color_index = LED_COLOR_ID_GREEN;
> +	leds->subled_info[2].color_index = LED_COLOR_ID_BLUE;
> +
> +	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_work);
>  	spin_lock_init(&drvdata->kbd_backlight->lock);
>  
>  	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
> +	no_led = !!ret;

Assigning to bool doesn't require !!.

> +
> +	if (drvdata->quirks & QUIRK_ROG_NKEY_RGB) {
> +		ret = devm_led_classdev_multicolor_register(
> +			&hdev->dev, &leds->mc_led);

IMO, this could go to one line (it's only 87 chars long). At minimum, the 
first arg fits to the first line.

> +		if (!ret)
> +			leds->rgb_registered = true;
> +		no_led &= !!ret;

No !!

> +	}
>  
> -	if (ret < 0) {
> +	if (no_led) {
>  		/* No need to have this still around */
>  		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
>  	}
>  
> -	return ret;
> +	return no_led ? -ENODEV : 0;

Introduction of no_led leads to shadowing error code which is usually 
undesirable. What's the reason you don't want to pass the original ret 
code onward?

If you have a good reason for it, please documented it in the commit 
message and preferrably make that change own change so it can focus on 
that thing only. It would also make the diff here cleaner.

>  }
>  
>  /*
> @@ -1289,7 +1430,7 @@ static const struct hid_device_id asus_devices[] = {
>  	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>  	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>  	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
>  	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> @@ -1318,7 +1459,7 @@ static const struct hid_device_id asus_devices[] = {
>  	 */
>  	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>  		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
>  	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>  		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD) },
>  	{ }
>
diff mbox series

Patch

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 905453a4eb5b7..3ac1e2dea45bb 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -30,6 +30,7 @@ 
 #include <linux/input/mt.h>
 #include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
 #include <linux/power_supply.h>
+#include <linux/led-class-multicolor.h>
 #include <linux/leds.h>
 
 #include "hid-ids.h"
@@ -85,6 +86,7 @@  MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
 #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
 #define QUIRK_HANDLE_GENERIC		BIT(13)
+#define QUIRK_ROG_NKEY_RGB		BIT(14)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS | \
@@ -97,9 +99,15 @@  MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 
 struct asus_kbd_leds {
 	struct asus_hid_listener listener;
+	struct led_classdev_mc mc_led;
+	struct mc_subled subled_info[3];
 	struct hid_device *hdev;
 	struct work_struct work;
 	unsigned int brightness;
+	uint8_t rgb_colors[3];
+	bool rgb_init;
+	bool rgb_set;
+	bool rgb_registered;
 	spinlock_t lock;
 	bool removed;
 };
@@ -504,23 +512,67 @@  static void asus_schedule_work(struct asus_kbd_leds *led)
 	spin_unlock_irqrestore(&led->lock, flags);
 }
 
-static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
+static void do_asus_kbd_backlight_set(struct asus_kbd_leds *led, int brightness)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&led->lock, flags);
+	led->brightness = brightness;
+	spin_unlock_irqrestore(&led->lock, flags);
+
+	asus_schedule_work(led);
+}
+
+static void asus_kbd_listener_set(struct asus_hid_listener *listener,
 				   int brightness)
 {
 	struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
 						 listener);
+	do_asus_kbd_backlight_set(led, brightness);
+	if (led->rgb_registered) {
+		led->mc_led.led_cdev.brightness = brightness;
+		led_classdev_notify_brightness_hw_changed(&led->mc_led.led_cdev,
+							  brightness);
+	}
+}
+
+static void asus_kbd_brightness_set(struct led_classdev *led_cdev,
+				    enum led_brightness brightness)
+{
+	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
+	struct asus_kbd_leds *led = container_of(mc_cdev, struct asus_kbd_leds,
+						 mc_led);
 	unsigned long flags;
 
 	spin_lock_irqsave(&led->lock, flags);
-	led->brightness = brightness;
+	led->rgb_colors[0] = mc_cdev->subled_info[0].intensity;
+	led->rgb_colors[1] = mc_cdev->subled_info[1].intensity;
+	led->rgb_colors[2] = mc_cdev->subled_info[2].intensity;
+	led->rgb_set = true;
 	spin_unlock_irqrestore(&led->lock, flags);
 
-	asus_schedule_work(led);
+	do_asus_kbd_backlight_set(led, brightness);
+}
+
+static enum led_brightness asus_kbd_brightness_get(struct led_classdev *led_cdev)
+{
+	struct led_classdev_mc *mc_led;
+	struct asus_kbd_leds *led;
+	enum led_brightness brightness;
+	unsigned long flags;
+
+	mc_led = lcdev_to_mccdev(led_cdev);
+	led = container_of(mc_led, struct asus_kbd_leds, mc_led);
+
+	spin_lock_irqsave(&led->lock, flags);
+	brightness = led->brightness;
+	spin_unlock_irqrestore(&led->lock, flags);
+
+	return brightness;
 }
 
-static void asus_kbd_backlight_work(struct work_struct *work)
+static void asus_kbd_backlight_work(struct asus_kbd_leds *led)
 {
-	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
 	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
 	int ret;
 	unsigned long flags;
@@ -534,10 +586,69 @@  static void asus_kbd_backlight_work(struct work_struct *work)
 		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
 }
 
+static void asus_kbd_rgb_work(struct asus_kbd_leds *led)
+{
+	u8 rgb_buf[][7] = {
+		{ FEATURE_KBD_LED_REPORT_ID1, 0xB3 }, /* set mode */
+		{ FEATURE_KBD_LED_REPORT_ID1, 0xB5 }, /* apply mode */
+		{ FEATURE_KBD_LED_REPORT_ID1, 0xB4 }, /* save to mem */
+	};
+	unsigned long flags;
+	uint8_t colors[3];
+	bool rgb_init, rgb_set;
+	int ret;
+
+	spin_lock_irqsave(&led->lock, flags);
+	rgb_init = led->rgb_init;
+	rgb_set = led->rgb_set;
+	led->rgb_set = false;
+	colors[0] = led->rgb_colors[0];
+	colors[1] = led->rgb_colors[1];
+	colors[2] = led->rgb_colors[2];
+	spin_unlock_irqrestore(&led->lock, flags);
+
+	if (!rgb_set)
+		return;
+
+	if (rgb_init) {
+		ret = asus_kbd_init(led->hdev, FEATURE_KBD_LED_REPORT_ID1);
+		if (ret < 0) {
+			hid_err(led->hdev, "Asus failed to init RGB: %d\n", ret);
+			return;
+		}
+		spin_lock_irqsave(&led->lock, flags);
+		led->rgb_init = false;
+		spin_unlock_irqrestore(&led->lock, flags);
+	}
+
+	/* Protocol is: 54b3 zone (0=all) mode (0=solid) RGB */
+	rgb_buf[0][4] = colors[0];
+	rgb_buf[0][5] = colors[1];
+	rgb_buf[0][6] = colors[2];
+
+	for (size_t i = 0; i < ARRAY_SIZE(rgb_buf); i++) {
+		ret = asus_kbd_set_report(led->hdev, rgb_buf[i], sizeof(rgb_buf[i]));
+		if (ret < 0) {
+			hid_err(led->hdev, "Asus failed to set RGB: %d\n", ret);
+			return;
+		}
+	}
+}
+
+static void asus_kbd_work(struct work_struct *work)
+{
+	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds,
+						 work);
+	asus_kbd_backlight_work(led);
+	asus_kbd_rgb_work(led);
+}
+
 static int asus_kbd_register_leds(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 	unsigned char kbd_func;
+	struct asus_kbd_leds *leds;
+	bool no_led;
 	int ret;
 
 	ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
@@ -565,21 +676,51 @@  static int asus_kbd_register_leds(struct hid_device *hdev)
 	if (!drvdata->kbd_backlight)
 		return -ENOMEM;
 
-	drvdata->kbd_backlight->removed = false;
-	drvdata->kbd_backlight->brightness = 0;
-	drvdata->kbd_backlight->hdev = hdev;
-	drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
-	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
+	leds = drvdata->kbd_backlight;
+	leds->removed = false;
+	leds->brightness = 3;
+	leds->hdev = hdev;
+	leds->listener.brightness_set = asus_kbd_listener_set;
+
+	leds->rgb_colors[0] = 0;
+	leds->rgb_colors[1] = 0;
+	leds->rgb_colors[2] = 0;
+	leds->rgb_init = true;
+	leds->rgb_set = false;
+	leds->mc_led.led_cdev.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
+					"asus-%s:rgb:peripheral",
+					strlen(hdev->uniq) ?
+					hdev->uniq : dev_name(&hdev->dev));
+	leds->mc_led.led_cdev.flags = LED_BRIGHT_HW_CHANGED;
+	leds->mc_led.led_cdev.max_brightness = 3,
+	leds->mc_led.led_cdev.brightness_set = asus_kbd_brightness_set,
+	leds->mc_led.led_cdev.brightness_get = asus_kbd_brightness_get,
+	leds->mc_led.subled_info = leds->subled_info,
+	leds->mc_led.num_colors = ARRAY_SIZE(leds->subled_info),
+	leds->subled_info[0].color_index = LED_COLOR_ID_RED;
+	leds->subled_info[1].color_index = LED_COLOR_ID_GREEN;
+	leds->subled_info[2].color_index = LED_COLOR_ID_BLUE;
+
+	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_work);
 	spin_lock_init(&drvdata->kbd_backlight->lock);
 
 	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
+	no_led = !!ret;
+
+	if (drvdata->quirks & QUIRK_ROG_NKEY_RGB) {
+		ret = devm_led_classdev_multicolor_register(
+			&hdev->dev, &leds->mc_led);
+		if (!ret)
+			leds->rgb_registered = true;
+		no_led &= !!ret;
+	}
 
-	if (ret < 0) {
+	if (no_led) {
 		/* No need to have this still around */
 		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
 	}
 
-	return ret;
+	return no_led ? -ENODEV : 0;
 }
 
 /*
@@ -1289,7 +1430,7 @@  static const struct hid_device_id asus_devices[] = {
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
@@ -1318,7 +1459,7 @@  static const struct hid_device_id asus_devices[] = {
 	 */
 	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
 		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
 	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
 		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD) },
 	{ }