Re: [PATCH v4] HID: Add full support for Logitech Unifying receivers
From: Jiri Kosina <hidden>
Date: 2011-09-11 08:48:16
Also in:
lkml
On Sat, 10 Sep 2011, Benjamin Tissoires wrote:
From: Nestor Lopez Casado <redacted> From: Nestor Lopez Casado <redacted> With this driver, all the devices paired to a single Unifying receiver are exposed to user processes in separated /input/dev nodes. Keyboards with different layouts can be treated differently, Multiplayer games on single PC (like home theater PC) can differentiate input coming from different kbds paired to the same receiver. Up to now, when Logitech Unifying receivers are connected to a Linux based system, a single keyboard and a single mouse are presented to the HID Layer, even if the Unifying receiver can pair up to six compatible devices. The Unifying receiver by default multiplexes all incoming events (from multiple keyboards/mice) into these two. Signed-off-by: Nestor Lopez Casado <redacted> Signed-off-by: Benjamin Tissoires <redacted> --- Hi guys, this is the v4 of the bus enumerator patch. Since the v3: - it corrects a kernel oops when starting gnome-panel (due to a hid_report not given as a pointer in logi_dj_ll_input_event). - I also added the function logi_dj_reset_resume to re-enable dj mode after coming from suspend to disk in case usb has been shut off. Cheers, Benjamin drivers/hid/Kconfig | 9 + drivers/hid/Makefile | 1 + drivers/hid/hid-core.c | 2 + drivers/hid/hid-ids.h | 2 + drivers/hid/hid-logitech-dj.c | 913 +++++++++++++++++++++++++++++++++++++++++ drivers/hid/hid-logitech-dj.h | 123 ++++++ 6 files changed, 1050 insertions(+), 0 deletions(-) create mode 100644 drivers/hid/hid-logitech-dj.c create mode 100644 drivers/hid/hid-logitech-dj.h
[ ... snip ... ]
quoted hunk
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c new file mode 100644 index 0000000..3963a99 --- /dev/null +++ b/drivers/hid/hid-logitech-dj.c@@ -0,0 +1,913 @@ +/* + * HID driver for Logitech Unifying receivers + * + * Copyright (c) 2011 Logitech (c)
What is this double (c) about? (it is also in other files).
quoted hunk
+/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Should you need to contact me, the author, you can do so by e-mail send + * your message to Nestor Lopez Casado <xnlopez at gmail com>
As the file is not copyrighted by a single person, but a company instead, perhaps this paragraph should rather be removed? [ ... snip ... ]
quoted hunk
+static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, + struct dj_report *dj_report) +{ + /* Called in delayed work context */ + struct hid_device *djrcv_hdev = djrcv_dev->hdev; + struct usb_interface *intf = to_usb_interface(djrcv_hdev->dev.parent); + struct usb_device *usbdev = interface_to_usbdev(intf); + struct hid_device *dj_hiddev; + struct dj_device *dj_dev; + unsigned char tmpstr[20]; + + if (dj_report->report_params[DEVICE_PAIRED_PARAM_SPFUNCTION] & + SPFUNCTION_DEVICE_LIST_EMPTY) { + dbg_hid("%s: device list is empty\n", __func__); + return; + } + + dj_hiddev = hid_allocate_device(); + if (IS_ERR(dj_hiddev)) { + dev_err(&djrcv_hdev->dev, "%s: hid_allocate_device failed\n", + __func__); + return; + } + + dj_hiddev->ll_driver = &logi_dj_ll_driver; + dj_hiddev->hid_output_raw_report = logi_dj_output_hidraw_report; + + dj_hiddev->dev.parent = &djrcv_hdev->dev; + dj_hiddev->bus = BUS_USB; + dj_hiddev->vendor = le16_to_cpu(usbdev->descriptor.idVendor); + dj_hiddev->product = le16_to_cpu(usbdev->descriptor.idProduct); + snprintf(dj_hiddev->name, sizeof(dj_hiddev->name), + "Logitech Unifying Device. Wireless PID:%02x%02x", + dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB], + dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB]); + + usb_make_path(usbdev, dj_hiddev->phys, sizeof(dj_hiddev->phys)); + snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index); + strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys));
Where does the guarantee of 20 chars being enough here come from, please? Other than these minor things, I think the driver is ready to be merged. Thanks everyone for your work on this, -- Jiri Kosina SUSE Labs