Re: [PATCH 3/6] platform/x86/tuxedo: Use intensity according to HID spec

[Ilpo Järvinen]

On Tue, 28 Jul 2026, Aaron Erhardt wrote:

> For RGB LEDs, the HID spec offers an example that uses only two
> intensity values for turning LEDs on and off. All other color values
> are submitted through the color channels individually, thus avoiding
> duplicated handling of brightness.

Hi,

If you want to pursue this, please provide better justification. You 
basically just rewrote the existing comment that told the current behavior 
is intentionally written as it is --- without really explain why you want 
to change it and what was problem with the current behavior.

> Signed-off-by: Aaron Erhardt <aer@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/platform/x86/tuxedo/nb04/wmi_ab.c | 25 ++++++++++++-----------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> index 11babc7c7767..8f1ffca0430d 100644
> --- a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> +++ b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> @@ -553,7 +553,7 @@ static int handle_lamp_attributes_response_report(struct hid_device *hdev,
>  	rep->red_level_count = 0xff;
>  	rep->green_level_count = 0xff;
>  	rep->blue_level_count = 0xff;
> -	rep->intensity_level_count = 0xff;
> +	rep->intensity_level_count = 0x1;
>  	rep->is_programmable = 1;
>  
>  	if (driver_data->kbl_map[lamp_id].code <= 0xe8) {
> @@ -640,22 +640,23 @@ static int handle_lamp_multi_update_report(struct hid_device *hdev,
>  					j + 1;
>  			rgb_configs_j->key_id = key_id;
>  			/*
> -			 * While this driver respects update_channel.intensity
> -			 * according to "HID Usage Tables v1.5" also on RGB
> -			 * leds, the Microsoft MacroPad reference implementation
> +			 * This driver uses update_channel.intensity according to
> +			 * "Color Attributes Examples" in "HID Usage Tables v1.7".
> +			 * Only two intensity values are allowed for turning LEDs
> +			 * on or off, while color and brightness can be controlled
> +			 * through the RGB values. This is also identical to the
> +			 * Microsoft MacroPad reference implementation
>  			 * (https://github.com/microsoft/RP2040MacropadHidSample
> -			 * 1d6c3ad) does not and ignores it. If it turns out
> -			 * that Windows writes intensity = 0 for RGB leds
> -			 * instead of intensity = 255, this driver should also
> -			 * ignore the update_channel.intensity.
> +			 * 1d6c3ad).
>  			 */
> -			intensity_i = rep->update_channels[i].intensity;
> +			intensity_i = min(1, rep->update_channels[i].intensity);
>  			red_i = rep->update_channels[i].red;
>  			green_i = rep->update_channels[i].green;
>  			blue_i = rep->update_channels[i].blue;
> -			rgb_configs_j->red = red_i * intensity_i / 0xff;
> -			rgb_configs_j->green = green_i * intensity_i / 0xff;
> -			rgb_configs_j->blue = blue_i * intensity_i / 0xff;
> +
> +			rgb_configs_j->red = red_i * intensity_i;
> +			rgb_configs_j->green = green_i * intensity_i;
> +			rgb_configs_j->blue = blue_i * intensity_i;
>  
>  			break;
>  		}
> 

-- 
 i.