Re: [PATCH v2] HID: ayaneo: Add AYANEO 3 detachable controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing initialization of the multicolor LED channel intensities results in the LED remaining off by default.
- [Medium] The `hw_pattern` implementation completely ignores the brightness requested in the pattern string.
--

--- Patch [1]: [PATCH v2] HID: ayaneo: Add AYANEO 3 detachable controller driver ---
commit e8a8ced2df93a1a83b6b165f28028f3d597aafea
Author: Matías Martínez <hello@matias.me>

HID: ayaneo: Add AYANEO 3 detachable controller driver

This commit adds a driver for the vendor interface of the AYANEO 3
detachable controller. It provides module identification, software eject,
and RGB control of the joystick rings as a multicolor LED class device.

> diff --git a/drivers/hid/hid-ayaneo.c b/drivers/hid/hid-ayaneo.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hid/hid-ayaneo.c
[ ... ]
> @@ -359,25 +359,25 @@ static int aya3_pattern_set(struct led_classdev *cdev,
>  			    struct led_pattern *pattern, u32 len, int repeat)
>  {
>  	struct led_classdev_mc *mc = lcdev_to_mccdev(cdev);
>  	struct aya3 *aya = container_of(mc, struct aya3, mcled);
>  	int ret;
>  
>  	if (len != 2 || pattern[0].brightness || !pattern[1].brightness)
>  		return -EINVAL;
>  
>  	ret = mutex_lock_interruptible(&aya->lock);
>  	if (ret)
>  		return ret;
>  	aya->pulse = true;
>  	ret = aya3_send_config(aya, 0);

[Severity: Medium]
Will this hardware breathing pattern fail to activate if the LED is currently
turned off?

In aya3_pattern_set(), the requested brightness in pattern[1].brightness is
validated but never used to update aya->rgb. If the LED was previously off,
aya->rgb remains {0, 0, 0}.

When aya3_send_config() is called, it checks aya->rgb to determine the mode:

    if (aya->rgb[0] || aya->rgb[1] || aya->rgb[2])
        mode = aya->pulse ? AYA3_RGB_PULSE : AYA3_RGB_SOLID;

If aya->rgb is all zeros, the mode will remain AYA3_RGB_OFF instead of
AYA3_RGB_PULSE, silently ignoring the pattern request. Also, if the LED was
already on, it will pulse with the previous brightness instead of the
requested brightness.

>  	mutex_unlock(&aya->lock);
>  	return ret;
>  }
[ ... ]
> @@ -389,17 +389,17 @@ static int aya3_register_led(struct aya3 *aya)
>  {
>  	struct led_classdev *cdev = &aya->mcled.led_cdev;
>  
>  	aya->subleds[0].color_index = LED_COLOR_ID_RED;
>  	aya->subleds[1].color_index = LED_COLOR_ID_GREEN;
>  	aya->subleds[2].color_index = LED_COLOR_ID_BLUE;
>  	aya->mcled.subled_info = aya->subleds;
>  	aya->mcled.num_colors = 3;

[Severity: Medium]
Is it intentional that the subled channel intensities are left uninitialized?

Because aya is allocated with devm_kzalloc(), the intensity fields in
aya->subleds are zero-initialized. When userspace sets the brightness,
aya3_led_set() calls led_mc_calc_color_components(), which multiplies the
requested brightness by each subled's intensity to calculate the final RGB
values.

Since the intensity is 0, the resulting color components will always be 0,
causing the LED to remain off by default when a brightness is requested.
Should the channel intensity fields be initialized here?

>  
>  	cdev->name = devm_kasprintf(&aya->hdev->dev, GFP_KERNEL,
>  				    "%s:rgb:joystick_rings",
>  				    dev_name(&aya->hdev->dev));
>  	if (!cdev->name)
>  		return -ENOMEM;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824223103.93947-1-hello@matias.me?part=1