Dear Jiri Kosina,
Thank you for your propt reponse.
Please refer to accossories below.
static const struct hid_device_id samsung_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE) }, { HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_WIRELESS_KBD_MOUSE) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SAMSUNG_ELECTRONICS, USB_DEVICE_ID_SAMSUNG_WIRELESS_KBD) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SAMSUNG_ELECTRONICS, USB_DEVICE_ID_SAMSUNG_WIRELESS_GAMEPAD) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SAMSUNG_ELECTRONICS, USB_DEVICE_ID_SAMSUNG_WIRELESS_ACTIONMOUSE) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SAMSUNG_ELECTRONICS, USB_DEVICE_ID_SAMSUNG_WIRELESS_UNIVERSAL_KBD) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SAMSUNG_ELECTRONICS, USB_DEVICE_ID_SAMSUNG_WIRELESS_MULTI_HOGP_KBD) }, { } }; MODULE_DEVICE_TABLE(hid, samsung_devices);
Mobile users prefer to use Bluetooth devices instead of USB devices. The commit should be reverted for Bluetooth accessories.
Due to internal problem, it takes time to upload Samsung's patch. So, please first revert the commit :)
Thanks
BR. Junwan.
--------- Original Message --------- Sender : Jiri Kosina jikos@kernel.org Date : 2022-03-30 16:52 (GMT+9) Title : Re: Request for reverting the commit for Samsung HID driver On Wed, 30 Mar 2022, 조준완 wrote:
author Greg Kroah-Hartman gregkh@linuxfoundation.org 2021-12-01 19:35:03 +0100 committer Benjamin Tissoires benjamin.tissoires@redhat.com 2021-12-02 15:36:18 +0100 commit 93020953d0fa7035fd036ad87a47ae2b7aa4ae33 (patch) tree f734e9962e28cedcccfbb109e510d39a18ed6d34 /drivers/hid/hid-samsung.c parent 720ac467204a70308bd687927ed475afb904e11b (diff) download linux-93020953d0fa7035fd036ad87a47ae2b7aa4ae33.tar.gz HID: check for valid USB device for many HID drivers Many HID drivers assume that the HID device assigned to them is a USB device as that was the only way HID devices used to be able to be created in Linux. However, with the additional ways that HID devices can be created for many different bus types, that is no longer true, so properly check that we have a USB device associated with the HID device before allowing a driver that makes this assumption to claim it. diff --git a/drivers/hid/hid-samsung.c b/drivers/hid/hid-samsung.c index 2e1c31156eca0..cf5992e970940 100644 --- a/drivers/hid/hid-samsung.c +++ b/drivers/hid/hid-samsung.c @@ -152,6 +152,9 @@ static int samsung_probe(struct hid_device *hdev, int ret; unsigned int cmask = HID_CONNECT_DEFAULT; + if (!hid_is_usb(hdev)) + return -EINVAL; + ret = hid_parse(hdev); if (ret) { hid_err(hdev, "parse failed\n"); Bluetooth HID devices like Keyboard and mice don't work at all because of this commit.
Could you please elaborate a little bit more -- which Bluetooth device (VID/PID) are you referring to please? hid-samsung as-is in mainline should only match against these two: static const struct hid_device_id samsung_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE) }, { HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_WIRELESS_KBD_MOUSE) }, { } }; MODULE_DEVICE_TABLE(hid, samsung_devices); which are both USB devices, not Bluetooth.
We Samsung applies several patches to Samsung drivers for Android models based on Linux kernel Samsung Driver. For example, add Samsung accessories or define key codes for special key processing. We don't want any changes in Samsung hid driver by others.
That's not how Linux development works though.
Soorer or later, I have a plan to contribute a modified driver by Samsung that is currently in use on Samsung Android.
That would be very much appreciated, thanks. Once you do so, you can very well become maintainers of hid-samsung driver (which would make a lot of sense for someone from Samsung to actually participate in development of that driver), and have much better influence on what goes in and in which form.
Anyway, we sincerely hope you will revert this commit because Samsung driver is not just for USB accessories.
I still would like to understand this part, because as far as I can tell, the in-kernel one is. Thanks, -- Jiri Kosina SUSE Labs