Re: Request for reverting the commit for Samsung HID driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 30 Mar 2022, 조준완 wrote:

> author
> Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> 2021-12-01 19:35:03 +0100
> 
> committer
> Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> 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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux