[PATCH 5/6] platform/x86/tuxedo: Update and extend documentation

[PATCH 5/6] platform/x86/tuxedo: Update and extend documentation

[Aaron Erhardt]

Fill-in missing details, update information and
clean up some obsolete documentation.

Signed-off-by: Aaron Erhardt <aer@tuxedocomputers.com>
---
 drivers/platform/x86/tuxedo/nb04/wmi_ab.c | 51 ++++++++++++-----------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
index 2b985b030197..5dedf93f6d75 100644
--- a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
+++ b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
@@ -1,9 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * This driver implements the WMI AB device found on TUXEDO notebooks with board
- * vendor NB04.
+ * vendor NB04. This enables keyboard backlight control via a virtual HID
+ * LampArray device.
+ *
+ * The device will be available through the regular HID interfaces, such as
+ * hidraw and can be used by any userspace program that implements the HID
+ * LampArray standard.
  *
  * Copyright (C) 2024-2025 Werner Sembach <wse@tuxedocomputers.com>
+ * Copyright (C) 2026 Aaron Erhardt <aer@tuxedocomputers.com>
  */
 
 #include <linux/dmi.h>
@@ -488,12 +494,14 @@ static int handle_lamp_array_attributes_report(struct hid_device *hdev,
 	struct tux_hdev_driver_data_t *driver_data = hdev->driver_data;
 
 	rep->lamp_count = driver_data->lamp_count;
+
+	// Physical dimensions of the Sirius 16 keyboard
 	rep->bounding_box_width_in_micrometers = 368000;
 	rep->bounding_box_height_in_micrometers = 266000;
 	rep->bounding_box_depth_in_micrometers = 30000;
 	/*
 	 * LampArrayKindKeyboard, see "26.2.1 LampArrayKind Values" of
-	 * "HID Usage Tables v1.5"
+	 * "HID Usage Tables v1.7"
 	 */
 	rep->lamp_array_kind = 1;
 	// Some guessed value for interval microseconds
@@ -547,7 +555,7 @@ static int handle_lamp_attributes_response_report(struct hid_device *hdev,
 	rep->update_latency_in_microseconds = 100;
 	/*
 	 * LampPurposeControl, see "26.3.1 LampPurposes Flags" of
-	 * "HID Usage Tables v1.5"
+	 * "HID Usage Tables v1.7"
 	 */
 	rep->lamp_purpose = 1;
 	rep->red_level_count = 0xff;
@@ -560,8 +568,8 @@ static int handle_lamp_attributes_response_report(struct hid_device *hdev,
 		rep->input_binding = driver_data->kbl_map[lamp_id].code;
 	} else {
 		/*
-		 * Everything bigger is reserved/undefined, see
-		 * "10 Keyboard/Keypad Page (0x07)" of "HID Usage Tables v1.5"
+		 * Everything bigger than 0xe8 is reserved/undefined, see
+		 * "10 Keyboard/Keypad Page (0x07)" of "HID Usage Tables v1.7"
 		 * and should return 0, see "26.8.3 Lamp Attributes" of the same
 		 * document.
 		 */
@@ -606,10 +614,7 @@ static int handle_lamp_multi_update_report(struct hid_device *hdev,
 	u8 key_id, key_id_j, intensity_i, red_i, green_i, blue_i;
 	int ret;
 
-	/*
-	 * Catching misformatted lamp_multi_update_report and fail silently
-	 * according to "HID Usage Tables v1.5"
-	 */
+	// Catch bad reports and fail silently according to "HID Usage Tables v1.7"
 	for (unsigned int i = 0; i < rep->lamp_count; ++i) {
 		if (rep->lamp_id[i] > driver_data->lamp_count) {
 			hid_dbg(hdev, "Out of bounds lamp_id in lamp_multi_update_report. Skipping whole report!\n");
@@ -624,6 +629,7 @@ static int handle_lamp_multi_update_report(struct hid_device *hdev,
 		}
 	}
 
+	// Fill kbl_set_multiple_keys_in update buffer
 	for (unsigned int i = 0; i < rep->lamp_count; ++i) {
 		key_id = driver_data->kbl_map[rep->lamp_id[i]].code;
 
@@ -632,6 +638,8 @@ static int handle_lamp_multi_update_report(struct hid_device *hdev,
 		     ++j) {
 			rgb_configs_j = &next->kbl_set_multiple_keys_in.rgb_configs[j];
 			key_id_j = rgb_configs_j->key_id;
+
+			// Search for existing or empty entry
 			if (key_id_j != 0x00 && key_id_j != key_id)
 				continue;
 
@@ -691,10 +699,7 @@ static int handle_lamp_range_update_report(struct hid_device *hdev,
 	struct lamp_rgbi_tuple_t *update_channels_j;
 	int ret;
 
-	/*
-	 * Catching misformatted lamp_range_update_report and fail silently
-	 * according to "HID Usage Tables v1.5"
-	 */
+	// Catch bad reports and fail silently according to "HID Usage Tables v1.7"
 	if (rep->lamp_id_start > rep->lamp_id_end) {
 		hid_dbg(hdev, "lamp_id_start > lamp_id_end in lamp_range_update_report. Skipping whole report!\n");
 		return sizeof(*rep);
@@ -706,8 +711,8 @@ static int handle_lamp_range_update_report(struct hid_device *hdev,
 	}
 
 	/*
-	 * Break handle_lamp_range_update_report call down to multiple
-	 * handle_lamp_multi_update_report calls to easily ensure that mixing
+	 * Break handle_lamp_range_update_report call down into multiple
+	 * handle_lamp_multi_update_report calls to ensure that mixing
 	 * handle_lamp_range_update_report and handle_lamp_multi_update_report
 	 * does not break things.
 	 */
@@ -750,15 +755,12 @@ static int handle_lamp_array_control_report(struct hid_device *hdev __always_unu
 					    struct lamp_array_control_report_t *rep)
 {
 	/*
-	 * The keyboards firmware doesn't have any built in controls and the
-	 * built in effects are not implemented so this is a NOOP.
-	 * According to the HID Documentation (HID Usage Tables v1.5) this
+	 * The keyboards firmware doesn't have any built-in controls and the
+	 * built-in effects are not implemented so this is a NOOP.
+	 * According to the HID Documentation (HID Usage Tables v1.7) this
 	 * function is optional and can be removed from the HID Report
 	 * Descriptor, but it should first be confirmed that userspace respects
-	 * this possibility too. The Microsoft MacroPad reference implementation
-	 * (https://github.com/microsoft/RP2040MacropadHidSample 1d6c3ad)
-	 * already deviates from the spec at another point, see
-	 * handle_lamp_*_update_report.
+	 * this possibility too.
 	 */
 
 	return sizeof(*rep);
@@ -894,8 +896,8 @@ static struct wmi_driver tuxedo_nb04_wmi_tux_driver = {
 };
 
 /*
- * We don't know if the WMI API is stable and how unique the GUID is for this
- * ODM. To be on the safe side we therefore only run this driver on tested
+ * We don't know whether the WMI API is stable and how unique the GUID is for
+ * this ODM. To be on the safe side we therefore only run this driver on tested
  * devices defined by this list.
  */
 static const struct dmi_system_id tested_devices_dmi_table[] __initconst = {
@@ -933,4 +935,5 @@ module_exit(tuxedo_nb04_wmi_tux_exit);
 
 MODULE_DESCRIPTION("Virtual HID LampArray interface for TUXEDO NB04 devices");
 MODULE_AUTHOR("Werner Sembach <wse@tuxedocomputers.com>");
+MODULE_AUTHOR("Aaron Erhardt <aer@tuxedocomputers.com>");
 MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH 6/6] MAINTAINERS: Add Aaron Erhardt as maintainer of TUXEDO DRIVERS

[Aaron Erhardt]

I'm actively working on this driver as part of my responsibilities now.

Signed-off-by: Aaron Erhardt <aer@tuxedocomputers.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a674e36529f7..792eb26722b3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27551,6 +27551,7 @@ F:	tools/testing/selftests/turbostat/
 
 TUXEDO DRIVERS
 M:	Werner Sembach <wse@tuxedocomputers.com>
+M:	Aaron Erhardt <aer@tuxedocomputers.com>
 L:	platform-driver-x86@vger.kernel.org
 S:	Supported
 F:	drivers/platform/x86/tuxedo/
-- 
2.43.0

Re: [PATCH 5/6] platform/x86/tuxedo: Update and extend documentation

[Ilpo Järvinen]

On Tue, 28 Jul 2026, Aaron Erhardt wrote:

> Fill-in missing details, update information and
> clean up some obsolete documentation.
> 
> Signed-off-by: Aaron Erhardt <aer@tuxedocomputers.com>
> ---
>  drivers/platform/x86/tuxedo/nb04/wmi_ab.c | 51 ++++++++++++-----------
>  1 file changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> index 2b985b030197..5dedf93f6d75 100644
> --- a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> +++ b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> @@ -1,9 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * This driver implements the WMI AB device found on TUXEDO notebooks with board
> - * vendor NB04.
> + * vendor NB04. This enables keyboard backlight control via a virtual HID
> + * LampArray device.
> + *
> + * The device will be available through the regular HID interfaces, such as
> + * hidraw and can be used by any userspace program that implements the HID
> + * LampArray standard.
>   *
>   * Copyright (C) 2024-2025 Werner Sembach <wse@tuxedocomputers.com>
> + * Copyright (C) 2026 Aaron Erhardt <aer@tuxedocomputers.com>
>   */
>  
>  #include <linux/dmi.h>
> @@ -488,12 +494,14 @@ static int handle_lamp_array_attributes_report(struct hid_device *hdev,
>  	struct tux_hdev_driver_data_t *driver_data = hdev->driver_data;
>  
>  	rep->lamp_count = driver_data->lamp_count;
> +
> +	// Physical dimensions of the Sirius 16 keyboard
>  	rep->bounding_box_width_in_micrometers = 368000;
>  	rep->bounding_box_height_in_micrometers = 266000;
>  	rep->bounding_box_depth_in_micrometers = 30000;
>  	/*
>  	 * LampArrayKindKeyboard, see "26.2.1 LampArrayKind Values" of
> -	 * "HID Usage Tables v1.5"
> +	 * "HID Usage Tables v1.7"
>  	 */
>  	rep->lamp_array_kind = 1;
>  	// Some guessed value for interval microseconds
> @@ -547,7 +555,7 @@ static int handle_lamp_attributes_response_report(struct hid_device *hdev,
>  	rep->update_latency_in_microseconds = 100;
>  	/*
>  	 * LampPurposeControl, see "26.3.1 LampPurposes Flags" of
> -	 * "HID Usage Tables v1.5"
> +	 * "HID Usage Tables v1.7"
>  	 */
>  	rep->lamp_purpose = 1;
>  	rep->red_level_count = 0xff;
> @@ -560,8 +568,8 @@ static int handle_lamp_attributes_response_report(struct hid_device *hdev,
>  		rep->input_binding = driver_data->kbl_map[lamp_id].code;
>  	} else {
>  		/*
> -		 * Everything bigger is reserved/undefined, see
> -		 * "10 Keyboard/Keypad Page (0x07)" of "HID Usage Tables v1.5"
> +		 * Everything bigger than 0xe8 is reserved/undefined, see
> +		 * "10 Keyboard/Keypad Page (0x07)" of "HID Usage Tables v1.7"
>  		 * and should return 0, see "26.8.3 Lamp Attributes" of the same
>  		 * document.
>  		 */
> @@ -606,10 +614,7 @@ static int handle_lamp_multi_update_report(struct hid_device *hdev,
>  	u8 key_id, key_id_j, intensity_i, red_i, green_i, blue_i;
>  	int ret;
>  
> -	/*
> -	 * Catching misformatted lamp_multi_update_report and fail silently
> -	 * according to "HID Usage Tables v1.5"
> -	 */
> +	// Catch bad reports and fail silently according to "HID Usage Tables v1.7"
>  	for (unsigned int i = 0; i < rep->lamp_count; ++i) {
>  		if (rep->lamp_id[i] > driver_data->lamp_count) {
>  			hid_dbg(hdev, "Out of bounds lamp_id in lamp_multi_update_report. Skipping whole report!\n");
> @@ -624,6 +629,7 @@ static int handle_lamp_multi_update_report(struct hid_device *hdev,
>  		}
>  	}
>  
> +	// Fill kbl_set_multiple_keys_in update buffer
>  	for (unsigned int i = 0; i < rep->lamp_count; ++i) {
>  		key_id = driver_data->kbl_map[rep->lamp_id[i]].code;
>  
> @@ -632,6 +638,8 @@ static int handle_lamp_multi_update_report(struct hid_device *hdev,
>  		     ++j) {
>  			rgb_configs_j = &next->kbl_set_multiple_keys_in.rgb_configs[j];
>  			key_id_j = rgb_configs_j->key_id;
> +
> +			// Search for existing or empty entry
>  			if (key_id_j != 0x00 && key_id_j != key_id)
>  				continue;
>  
> @@ -691,10 +699,7 @@ static int handle_lamp_range_update_report(struct hid_device *hdev,
>  	struct lamp_rgbi_tuple_t *update_channels_j;
>  	int ret;
>  
> -	/*
> -	 * Catching misformatted lamp_range_update_report and fail silently
> -	 * according to "HID Usage Tables v1.5"
> -	 */
> +	// Catch bad reports and fail silently according to "HID Usage Tables v1.7"
>  	if (rep->lamp_id_start > rep->lamp_id_end) {
>  		hid_dbg(hdev, "lamp_id_start > lamp_id_end in lamp_range_update_report. Skipping whole report!\n");
>  		return sizeof(*rep);
> @@ -706,8 +711,8 @@ static int handle_lamp_range_update_report(struct hid_device *hdev,
>  	}
>  
>  	/*
> -	 * Break handle_lamp_range_update_report call down to multiple
> -	 * handle_lamp_multi_update_report calls to easily ensure that mixing
> +	 * Break handle_lamp_range_update_report call down into multiple
> +	 * handle_lamp_multi_update_report calls to ensure that mixing
>  	 * handle_lamp_range_update_report and handle_lamp_multi_update_report
>  	 * does not break things.
>  	 */
> @@ -750,15 +755,12 @@ static int handle_lamp_array_control_report(struct hid_device *hdev __always_unu
>  					    struct lamp_array_control_report_t *rep)
>  {
>  	/*
> -	 * The keyboards firmware doesn't have any built in controls and the
> -	 * built in effects are not implemented so this is a NOOP.
> -	 * According to the HID Documentation (HID Usage Tables v1.5) this
> +	 * The keyboards firmware doesn't have any built-in controls and the

keyboard's ?

> +	 * built-in effects are not implemented so this is a NOOP.
> +	 * According to the HID Documentation (HID Usage Tables v1.7) this
>  	 * function is optional and can be removed from the HID Report
>  	 * Descriptor, but it should first be confirmed that userspace respects
> -	 * this possibility too. The Microsoft MacroPad reference implementation
> -	 * (https://github.com/microsoft/RP2040MacropadHidSample 1d6c3ad)
> -	 * already deviates from the spec at another point, see
> -	 * handle_lamp_*_update_report.
> +	 * this possibility too.

It's not enough justification for me to accept this with only stating 
"clean up some obsolete documentation" (in the changelog). If you make a 
change like this that clearly removes information, it would be better to 
put each into own patch and properly justify it.

Simple language fixes and additions are (mostly) okay in a larger patch.

>  	 */
>  
>  	return sizeof(*rep);
> @@ -894,8 +896,8 @@ static struct wmi_driver tuxedo_nb04_wmi_tux_driver = {
>  };
>  
>  /*
> - * We don't know if the WMI API is stable and how unique the GUID is for this
> - * ODM. To be on the safe side we therefore only run this driver on tested
> + * We don't know whether the WMI API is stable and how unique the GUID is for
> + * this ODM. To be on the safe side we therefore only run this driver on tested
>   * devices defined by this list.
>   */
>  static const struct dmi_system_id tested_devices_dmi_table[] __initconst = {
> @@ -933,4 +935,5 @@ module_exit(tuxedo_nb04_wmi_tux_exit);
>  
>  MODULE_DESCRIPTION("Virtual HID LampArray interface for TUXEDO NB04 devices");
>  MODULE_AUTHOR("Werner Sembach <wse@tuxedocomputers.com>");
> +MODULE_AUTHOR("Aaron Erhardt <aer@tuxedocomputers.com>");
>  MODULE_LICENSE("GPL");
> 

-- 
 i.