Re: [PATCH] Input: xpad - add support for Beitong KP series controllers

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Aggressive 25ms timeout for USB control message can cause device initialization failures.
- [Low] Build Error / Linker Failure when CONFIG_JOYSTICK_XPAD=y and CONFIG_HID=m.
--

--- Patch [1]: [PATCH] Input: xpad - add support for Beitong KP series controllers ---
commit abec748a4c53845b79c26002cb6f3fc8b88c0708
Author: vegetablecat <[email protected]>

Input: xpad - add support for Beitong KP series controllers

This patch adds support for Beitong KP series controllers by reading the
XUSB10 descriptor to lock them into XInput mode. It also includes an optional
HID receiver driver for the KP40D dongle to prevent hid-generic interference.

> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c

[ ... ]

> @@ -2148,6 +2180,19 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  	xpad->irq_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>  
>  	usb_set_intfdata(intf, xpad);
> +	if (xpad->read_xusb10) {
> +		error = usb_control_msg_recv(udev, 0, 0xee,
> +					     USB_TYPE_VENDOR | USB_DIR_IN |
> +						USB_RECIP_DEVICE,
> +					     0x0000, 0x0004,
> +					     ms_feature_descriptor,
> +					     sizeof(ms_feature_descriptor), 25,

[Severity: Medium]
Is this 25ms timeout for usb_control_msg_recv() too aggressive?

Standard USB timeouts (e.g., USB_CTRL_GET_TIMEOUT) are typically 5000ms. A
25ms timeout risks transient probe failures on busy systems or slower USB
hubs, which could cause the device to fail to stay in XInput mode.

> +					     GFP_KERNEL);
> +		if (error)
> +			dev_warn(&intf->dev,
> +				 "unable to read Microsoft feature descriptor: %d\n",
> +				 error);
> +	}

[ ... ]

> +static int __init xpad_init(void)
> +{
> +	int error;
> +
> +#if IS_ENABLED(CONFIG_HID)
> +	error = hid_register_driver(&xpad_receiver_driver);

[Severity: Low]
Will this cause a build error when CONFIG_JOYSTICK_XPAD=y and CONFIG_HID=m?

IS_ENABLED(CONFIG_HID) evaluates to true even when HID is built as a loadable
module. Since xpad is built into the main kernel image in this configuration,
it cannot link against module symbols, leading to a vmlinux link error.

Should this be #if IS_REACHABLE(CONFIG_HID) or managed via a Kconfig
dependency instead?

> +	if (error)
> +		return error;
> +#endif
> +
> +	error = usb_register(&xpad_driver);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1