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:
- [High] The HID device ID for the receiver dongle uses an incorrect vendor ID (`0x20dd`), which conflicts with the `0x20bc` vendor ID defined in the USB device table, preventing the driver from binding.
- [Low] Missing Kconfig dependency on `CONFIG_HID` for `CONFIG_JOYSTICK_XPAD` leads to build and linkage failures when HID is disabled or built as a module.
--

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

This commit adds support for Beitong KP series controllers by reading the 
XUSB10 descriptor to lock them into XInput mode. It also introduces a 
minimal HID receiver driver for the KP40D receiver dongle to prevent 
hid-generic from interfering.

> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index feb8f368f834e..d33ca181dfe8b 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
[ ... ]
> @@ -332,6 +334,32 @@ static const struct xpad_device {
>  	{ 0x1bad, 0xfd00, "Razer Onza TE", 0, XTYPE_XBOX360 },
>  	{ 0x1bad, 0xfd01, "Razer Onza", 0, XTYPE_XBOX360 },
>  	{ 0x1ee9, 0x1590, "ZOTAC Gaming Zone", 0, XTYPE_XBOX360 },
[ ... ]
> +	{ 0x20bc, 0x515b, "BETOP BTP-KP40D Controller (White)", 0, XTYPE_XBOX360, FLAG_READ_XUSB10 },
> +	{ 0x20bc, 0x515c, "BETOP BTP-KP40D Controller (White)", 0, XTYPE_XBOX360, FLAG_READ_XUSB10 },
> +	{ 0x20bc, 0x515d, "BETOP BTP-KP40F Controller (White)", 0, XTYPE_XBOX360, FLAG_READ_XUSB10 },
[ ... ]
> @@ -2312,7 +2357,71 @@ static struct usb_driver xpad_driver = {
[ ... ]
> +static const struct hid_device_id xpad_receiver_devices[] = {
> +	{ HID_USB_DEVICE(0x20dd, 0x515c) },
> +	{ }
> +};

[Severity: High]
Is the vendor ID 0x20dd in the HID device table correct?

The USB device table defines product 0x515c with vendor ID 0x20bc,
but the HID device ID table uses 0x20dd. Since a child HID device inherits
its vendor ID from the underlying USB device, will the HID driver fail to
bind to the dongle if it is looking for 0x20dd instead of 0x20bc?

> +MODULE_DEVICE_TABLE(hid, xpad_receiver_devices);
> +
> +static struct hid_driver xpad_receiver_driver = {
> +	.name = "xpad-kp40d-receiver",
> +	.id_table = xpad_receiver_devices,
> +	.probe = xpad_receiver_probe,
> +	.remove = xpad_receiver_remove,
> +};
> +
> +static int __init xpad_init(void)
> +{
> +	int error;
> +
> +	error = hid_register_driver(&xpad_receiver_driver);

[Severity: Low]
Does this driver now require a dependency on the HID subsystem in Kconfig?

Since xpad_init() now unconditionally calls hid_register_driver(), will
building the kernel with CONFIG_JOYSTICK_XPAD=y and CONFIG_HID=m (or
disabled) cause a linker failure due to undefined HID core symbols?

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

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