| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 11 Sep 2011 10:48:12 +0200 (CEST)
From: Jiri Kosina <jkosina@...e.cz>
To: Benjamin Tissoires <benjamin.tissoires@...il.com>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>, rtitmuss@...itech.com,
ogay@...itech.com, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org,
Nestor Lopez Casado <nlopezcasad@...itech.com>
Subject: Re: [PATCH v4] HID: Add full support for Logitech Unifying
receivers
On Sat, 10 Sep 2011, Benjamin Tissoires wrote:
> From: Nestor Lopez Casado <nlopezcasad@...itech.com>
>
> From: Nestor Lopez Casado <nlopezcasad@...itech.com>
>
> 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 <nlopezcasad@...itech.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...il.com>
> ---
>
> 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 ... ]
> 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).
> +/*
> + * 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 ... ]
> +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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists