[PATCH 0/4] HID: huion/uclogic merge and few additions

[PATCH 0/4] HID: huion/uclogic merge and few additions

[Benjamin Tissoires]

Hi,

So, this is a attempt to get a common base with the out of the tree driver
for Huion and UC-Logic tablets here https://github.com/DIGImend/digimend-kernel-drivers

I am CC-ing DIGImend-devel though the patches are aimed at Jiri's tree upstream.

Nick, I changed a little bit the first patch compared to your version.
I blindly applied HID_QUIRK_MULTI_INPUT. I don't expect anything to break with
this, but I was not able to reinject all the devices you have through uhid to
check.
I'd like you to at least sign both first and second if you are happy with the
code.

The remaining bit for the tablet I have is the switch to the button reporting
mode, but I think that will need a little bit more testing from the users of
the DIDImend project.

Cheers,
Benjamin

Benjamin Tissoires (4):
  HID: uclogic: Set quirks from inside the driver
  HID: uclogic: merge hid-huion driver in hid-uclogic
  HID: uclogic: present only the working interfaces on the Huion tablets
  HID: uclogic: name the input nodes based on their tool

 drivers/hid/Kconfig             |   8 +-
 drivers/hid/Makefile            |   1 -
 drivers/hid/hid-huion.c         | 290 --------------------------------------
 drivers/hid/hid-uclogic.c       | 301 +++++++++++++++++++++++++++++++++++++++-
 drivers/hid/usbhid/hid-quirks.c |   4 -
 5 files changed, 301 insertions(+), 303 deletions(-)
 delete mode 100644 drivers/hid/hid-huion.c

-- 
2.1.0

[PATCH 1/4] HID: uclogic: Set quirks from inside the driver

[Benjamin Tissoires]

Based on a patch from: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>

Most of the tablets handled by hid-uclogic already uses MULTI_INPUT.
Fot the ones which are not quirked in usbhid/hidquirks, they have a
custom report descriptor which contains only one report per HID
interface. For those tablets HID_QUIRK_MULTI_INPUT is transparent.

According to https://github.com/DIGImend/tablets, the only problematic
tablet currently handled by hid-uclogic is the TWHA60 v3. This tablet
presents different report descriptors from the ones currently quirked.
This is not a problem per se, given that this tablet is not supported
currently in this version (it needs the same command than a Huion to
start forwarding events).

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Changes from the original patch:
- apply HID_QUIRK_MULTI_INPUT for all devices
- removed the quirks in usbhid/hid-quirks.c

 drivers/hid/hid-uclogic.c       | 27 +++++++++++++++++++++++++++
 drivers/hid/usbhid/hid-quirks.c |  4 ----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-uclogic.c b/drivers/hid/hid-uclogic.c
index fb8b516..22dccce 100644
--- a/drivers/hid/hid-uclogic.c
+++ b/drivers/hid/hid-uclogic.c
@@ -626,6 +626,32 @@ static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+static int uclogic_probe(struct hid_device *hdev,
+		const struct hid_device_id *id)
+{
+	int rc;
+
+	/*
+	 * libinput requires the pad interface to be on a different node
+	 * than the pen, so use QUIRK_MULTI_INPUT for all tablets.
+	 */
+	hdev->quirks |= HID_QUIRK_MULTI_INPUT;
+
+	rc = hid_parse(hdev);
+	if (rc) {
+		hid_err(hdev, "parse failed\n");
+		return rc;
+	}
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (rc) {
+		hid_err(hdev, "hw start failed\n");
+		return rc;
+	}
+
+	return 0;
+}
+
 static const struct hid_device_id uclogic_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC,
 				USB_DEVICE_ID_UCLOGIC_TABLET_PF1209) },
@@ -648,6 +674,7 @@ MODULE_DEVICE_TABLE(hid, uclogic_devices);
 static struct hid_driver uclogic_driver = {
 	.name = "uclogic",
 	.id_table = uclogic_devices,
+	.probe = uclogic_probe,
 	.report_fixup = uclogic_report_fixup,
 };
 module_hid_driver(uclogic_driver);
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 989c59a..75ca2de 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -106,12 +106,8 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_SYMBOL, USB_DEVICE_ID_SYMBOL_SCANNER_2, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_TPV, USB_DEVICE_ID_TPV_OPTICAL_TOUCHSCREEN, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD, HID_QUIRK_NOGET },
-	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_PF1209, HID_QUIRK_MULTI_INPUT },
-	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP4030U, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_KNA5, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_TWA60, HID_QUIRK_MULTI_INPUT },
-	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP5540U, HID_QUIRK_MULTI_INPUT },
-	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP8060U, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_MEDIA_TABLET_10_6_INCH, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET, HID_QUIRK_MULTI_INPUT },
-- 
2.1.0

Re: [PATCH 1/4] HID: uclogic: Set quirks from inside the driver

[Nikolai Kondrashov]

On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> Based on a patch from: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>
>
> Most of the tablets handled by hid-uclogic already uses MULTI_INPUT.

*use

> Fot the ones which are not quirked in usbhid/hidquirks, they have a

*For

> custom report descriptor which contains only one report per HID
> interface. For those tablets HID_QUIRK_MULTI_INPUT is transparent.
>
> According to https://github.com/DIGImend/tablets, the only problematic
> tablet currently handled by hid-uclogic is the TWHA60 v3. This tablet
> presents different report descriptors from the ones currently quirked.
> This is not a problem per se, given that this tablet is not supported
> currently in this version (it needs the same command than a Huion to

*as a Huion

> start forwarding events).

Thanks, Benjamin. this will make further merges easier.
Regardless of the Grammar Nazi comments above (fixing is up to you):

Reviewed-by: Nikolai Kondrashov <spbnick@gmail.com>

Nick

Re: [PATCH 1/4] HID: uclogic: Set quirks from inside the driver

[Benjamin Tissoires]

On Feb 25 2015 or thereabouts, Nikolai Kondrashov wrote:
> On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> >Based on a patch from: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>
> >
> >Most of the tablets handled by hid-uclogic already uses MULTI_INPUT.
> 
> *use
> 
> >Fot the ones which are not quirked in usbhid/hidquirks, they have a
> 
> *For
> 
> >custom report descriptor which contains only one report per HID
> >interface. For those tablets HID_QUIRK_MULTI_INPUT is transparent.
> >
> >According to https://github.com/DIGImend/tablets, the only problematic
> >tablet currently handled by hid-uclogic is the TWHA60 v3. This tablet
> >presents different report descriptors from the ones currently quirked.
> >This is not a problem per se, given that this tablet is not supported
> >currently in this version (it needs the same command than a Huion to
> 
> *as a Huion
> 
> >start forwarding events).
> 
> Thanks, Benjamin. this will make further merges easier.
> Regardless of the Grammar Nazi comments above (fixing is up to you):

Looks like I will have to send a v2 to fix the 3/4, so I will amend it.

> 
> Reviewed-by: Nikolai Kondrashov <spbnick@gmail.com>

Thanks! Appreciated.

Cheers,
Benjamin

[PATCH 2/4] HID: uclogic: merge hid-huion driver in hid-uclogic

[Benjamin Tissoires]

From: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>

Merge the hid-huion driver into hid-uclogic as all the devices supported
by hid-huion are in fact UC-Logic devices.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Changed from the original patch:
- update Makefile, Kconfig accordingly
- remove one extra pr_err when kzalloc failed (raised by checkpatch.pl)

 drivers/hid/Kconfig       |   8 +-
 drivers/hid/Makefile      |   1 -
 drivers/hid/hid-huion.c   | 290 ----------------------------------------------
 drivers/hid/hid-uclogic.c | 229 +++++++++++++++++++++++++++++++++++-
 4 files changed, 229 insertions(+), 299 deletions(-)
 delete mode 100644 drivers/hid/hid-huion.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 152b006..8a55fd7 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -286,12 +286,6 @@ config HID_GT683R
 	Currently the following devices are know to be supported:
 	  - MSI GT683R
 
-config HID_HUION
-	tristate "Huion tablets"
-	depends on USB_HID
-	---help---
-	Support for Huion 580 tablet.
-
 config HID_KEYTOUCH
 	tristate "Keytouch HID devices"
 	depends on HID
@@ -314,7 +308,7 @@ config HID_UCLOGIC
 	tristate "UC-Logic"
 	depends on HID
 	---help---
-	Support for UC-Logic tablets.
+	Support for UC-Logic and Huion tablets.
 
 config HID_WALTOP
 	tristate "Waltop"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 6f19958..9c399fe 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -41,7 +41,6 @@ obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-mouse.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtekff.o
-obj-$(CONFIG_HID_HUION)		+= hid-huion.o
 obj-$(CONFIG_HID_HYPERV_MOUSE)	+= hid-hyperv.o
 obj-$(CONFIG_HID_ICADE)		+= hid-icade.o
 obj-$(CONFIG_HID_KENSINGTON)	+= hid-kensington.o
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
deleted file mode 100644
index 61b68ca..0000000
--- a/drivers/hid/hid-huion.c
+++ /dev/null
@@ -1,290 +0,0 @@
-/*
- *  HID driver for Huion devices not fully compliant with HID standard
- *
- *  Copyright (c) 2013 Martin Rusko
- *  Copyright (c) 2014 Nikolai Kondrashov
- */
-
-/*
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 of the License, or (at your option)
- * any later version.
- */
-
-#include <linux/device.h>
-#include <linux/hid.h>
-#include <linux/module.h>
-#include <linux/usb.h>
-#include <asm/unaligned.h>
-#include "usbhid/usbhid.h"
-
-#include "hid-ids.h"
-
-/* Report descriptor template placeholder head */
-#define HUION_PH_HEAD	0xFE, 0xED, 0x1D
-
-/* Report descriptor template placeholder IDs */
-enum huion_ph_id {
-	HUION_PH_ID_X_LM,
-	HUION_PH_ID_X_PM,
-	HUION_PH_ID_Y_LM,
-	HUION_PH_ID_Y_PM,
-	HUION_PH_ID_PRESSURE_LM,
-	HUION_PH_ID_NUM
-};
-
-/* Report descriptor template placeholder */
-#define HUION_PH(_ID) HUION_PH_HEAD, HUION_PH_ID_##_ID
-
-/* Fixed report descriptor template */
-static const __u8 huion_tablet_rdesc_template[] = {
-	0x05, 0x0D,             /*  Usage Page (Digitizer),                 */
-	0x09, 0x02,             /*  Usage (Pen),                            */
-	0xA1, 0x01,             /*  Collection (Application),               */
-	0x85, 0x07,             /*      Report ID (7),                      */
-	0x09, 0x20,             /*      Usage (Stylus),                     */
-	0xA0,                   /*      Collection (Physical),              */
-	0x14,                   /*          Logical Minimum (0),            */
-	0x25, 0x01,             /*          Logical Maximum (1),            */
-	0x75, 0x01,             /*          Report Size (1),                */
-	0x09, 0x42,             /*          Usage (Tip Switch),             */
-	0x09, 0x44,             /*          Usage (Barrel Switch),          */
-	0x09, 0x46,             /*          Usage (Tablet Pick),            */
-	0x95, 0x03,             /*          Report Count (3),               */
-	0x81, 0x02,             /*          Input (Variable),               */
-	0x95, 0x03,             /*          Report Count (3),               */
-	0x81, 0x03,             /*          Input (Constant, Variable),     */
-	0x09, 0x32,             /*          Usage (In Range),               */
-	0x95, 0x01,             /*          Report Count (1),               */
-	0x81, 0x02,             /*          Input (Variable),               */
-	0x95, 0x01,             /*          Report Count (1),               */
-	0x81, 0x03,             /*          Input (Constant, Variable),     */
-	0x75, 0x10,             /*          Report Size (16),               */
-	0x95, 0x01,             /*          Report Count (1),               */
-	0xA4,                   /*          Push,                           */
-	0x05, 0x01,             /*          Usage Page (Desktop),           */
-	0x65, 0x13,             /*          Unit (Inch),                    */
-	0x55, 0xFD,             /*          Unit Exponent (-3),             */
-	0x34,                   /*          Physical Minimum (0),           */
-	0x09, 0x30,             /*          Usage (X),                      */
-	0x27, HUION_PH(X_LM),   /*          Logical Maximum (PLACEHOLDER),  */
-	0x47, HUION_PH(X_PM),   /*          Physical Maximum (PLACEHOLDER), */
-	0x81, 0x02,             /*          Input (Variable),               */
-	0x09, 0x31,             /*          Usage (Y),                      */
-	0x27, HUION_PH(Y_LM),   /*          Logical Maximum (PLACEHOLDER),  */
-	0x47, HUION_PH(Y_PM),   /*          Physical Maximum (PLACEHOLDER), */
-	0x81, 0x02,             /*          Input (Variable),               */
-	0xB4,                   /*          Pop,                            */
-	0x09, 0x30,             /*          Usage (Tip Pressure),           */
-	0x27,
-	HUION_PH(PRESSURE_LM),  /*          Logical Maximum (PLACEHOLDER),  */
-	0x81, 0x02,             /*          Input (Variable),               */
-	0xC0,                   /*      End Collection,                     */
-	0xC0                    /*  End Collection                          */
-};
-
-/* Parameter indices */
-enum huion_prm {
-	HUION_PRM_X_LM		= 1,
-	HUION_PRM_Y_LM		= 2,
-	HUION_PRM_PRESSURE_LM	= 4,
-	HUION_PRM_RESOLUTION	= 5,
-	HUION_PRM_NUM
-};
-
-/* Driver data */
-struct huion_drvdata {
-	__u8 *rdesc;
-	unsigned int rsize;
-};
-
-static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
-		unsigned int *rsize)
-{
-	struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
-	switch (hdev->product) {
-	case USB_DEVICE_ID_HUION_TABLET:
-		if (drvdata->rdesc != NULL) {
-			rdesc = drvdata->rdesc;
-			*rsize = drvdata->rsize;
-		}
-		break;
-	}
-	return rdesc;
-}
-
-/**
- * Enable fully-functional tablet mode and determine device parameters.
- *
- * @hdev:	HID device
- */
-static int huion_tablet_enable(struct hid_device *hdev)
-{
-	int rc;
-	struct usb_device *usb_dev = hid_to_usb_dev(hdev);
-	struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
-	__le16 *buf = NULL;
-	size_t len;
-	s32 params[HUION_PH_ID_NUM];
-	s32 resolution;
-	__u8 *p;
-	s32 v;
-
-	/*
-	 * Read string descriptor containing tablet parameters. The specific
-	 * string descriptor and data were discovered by sniffing the Windows
-	 * driver traffic.
-	 * NOTE: This enables fully-functional tablet mode.
-	 */
-	len = HUION_PRM_NUM * sizeof(*buf);
-	buf = kmalloc(len, GFP_KERNEL);
-	if (buf == NULL) {
-		hid_err(hdev, "failed to allocate parameter buffer\n");
-		rc = -ENOMEM;
-		goto cleanup;
-	}
-	rc = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0),
-				USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
-				(USB_DT_STRING << 8) + 0x64,
-				0x0409, buf, len,
-				USB_CTRL_GET_TIMEOUT);
-	if (rc == -EPIPE) {
-		hid_err(hdev, "device parameters not found\n");
-		rc = -ENODEV;
-		goto cleanup;
-	} else if (rc < 0) {
-		hid_err(hdev, "failed to get device parameters: %d\n", rc);
-		rc = -ENODEV;
-		goto cleanup;
-	} else if (rc != len) {
-		hid_err(hdev, "invalid device parameters\n");
-		rc = -ENODEV;
-		goto cleanup;
-	}
-
-	/* Extract device parameters */
-	params[HUION_PH_ID_X_LM] = le16_to_cpu(buf[HUION_PRM_X_LM]);
-	params[HUION_PH_ID_Y_LM] = le16_to_cpu(buf[HUION_PRM_Y_LM]);
-	params[HUION_PH_ID_PRESSURE_LM] =
-		le16_to_cpu(buf[HUION_PRM_PRESSURE_LM]);
-	resolution = le16_to_cpu(buf[HUION_PRM_RESOLUTION]);
-	if (resolution == 0) {
-		params[HUION_PH_ID_X_PM] = 0;
-		params[HUION_PH_ID_Y_PM] = 0;
-	} else {
-		params[HUION_PH_ID_X_PM] = params[HUION_PH_ID_X_LM] *
-						1000 / resolution;
-		params[HUION_PH_ID_Y_PM] = params[HUION_PH_ID_Y_LM] *
-						1000 / resolution;
-	}
-
-	/* Allocate fixed report descriptor */
-	drvdata->rdesc = devm_kmalloc(&hdev->dev,
-				sizeof(huion_tablet_rdesc_template),
-				GFP_KERNEL);
-	if (drvdata->rdesc == NULL) {
-		hid_err(hdev, "failed to allocate fixed rdesc\n");
-		rc = -ENOMEM;
-		goto cleanup;
-	}
-	drvdata->rsize = sizeof(huion_tablet_rdesc_template);
-
-	/* Format fixed report descriptor */
-	memcpy(drvdata->rdesc, huion_tablet_rdesc_template,
-		drvdata->rsize);
-	for (p = drvdata->rdesc;
-	     p <= drvdata->rdesc + drvdata->rsize - 4;) {
-		if (p[0] == 0xFE && p[1] == 0xED && p[2] == 0x1D &&
-		    p[3] < sizeof(params)) {
-			v = params[p[3]];
-			put_unaligned(cpu_to_le32(v), (s32 *)p);
-			p += 4;
-		} else {
-			p++;
-		}
-	}
-
-	rc = 0;
-
-cleanup:
-	kfree(buf);
-	return rc;
-}
-
-static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
-{
-	int rc;
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-	struct huion_drvdata *drvdata;
-
-	/* Allocate and assign driver data */
-	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
-	if (drvdata == NULL) {
-		hid_err(hdev, "failed to allocate driver data\n");
-		return -ENOMEM;
-	}
-	hid_set_drvdata(hdev, drvdata);
-
-	switch (id->product) {
-	case USB_DEVICE_ID_HUION_TABLET:
-		/* If this is the pen interface */
-		if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
-			rc = huion_tablet_enable(hdev);
-			if (rc) {
-				hid_err(hdev, "tablet enabling failed\n");
-				return rc;
-			}
-		}
-		break;
-	}
-
-	rc = hid_parse(hdev);
-	if (rc) {
-		hid_err(hdev, "parse failed\n");
-		return rc;
-	}
-
-	rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-	if (rc) {
-		hid_err(hdev, "hw start failed\n");
-		return rc;
-	}
-
-	return 0;
-}
-
-static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
-			u8 *data, int size)
-{
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-
-	/* If this is a pen input report */
-	if (intf->cur_altsetting->desc.bInterfaceNumber == 0 &&
-	    report->type == HID_INPUT_REPORT &&
-	    report->id == 0x07 && size >= 2)
-		/* Invert the in-range bit */
-		data[1] ^= 0x40;
-
-	return 0;
-}
-
-static const struct hid_device_id huion_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_HUION_TABLET) },
-	{ }
-};
-MODULE_DEVICE_TABLE(hid, huion_devices);
-
-static struct hid_driver huion_driver = {
-	.name = "huion",
-	.id_table = huion_devices,
-	.probe = huion_probe,
-	.report_fixup = huion_report_fixup,
-	.raw_event = huion_raw_event,
-};
-module_hid_driver(huion_driver);
-
-MODULE_AUTHOR("Martin Rusko");
-MODULE_DESCRIPTION("Huion HID driver");
-MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-uclogic.c b/drivers/hid/hid-uclogic.c
index 22dccce..397f1df 100644
--- a/drivers/hid/hid-uclogic.c
+++ b/drivers/hid/hid-uclogic.c
@@ -1,7 +1,8 @@
 /*
  *  HID driver for UC-Logic devices not fully compliant with HID standard
  *
- *  Copyright (c) 2010 Nikolai Kondrashov
+ *  Copyright (c) 2010-2014 Nikolai Kondrashov
+ *  Copyright (c) 2013 Martin Rusko
  */
 
 /*
@@ -15,6 +16,8 @@
 #include <linux/hid.h>
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <asm/unaligned.h>
+#include "usbhid/usbhid.h"
 
 #include "hid-ids.h"
 
@@ -546,11 +549,90 @@ static __u8 twha60_rdesc_fixed1[] = {
 	0xC0        /*  End Collection              */
 };
 
+/* Report descriptor template placeholder head */
+#define UCLOGIC_PH_HEAD	0xFE, 0xED, 0x1D
+
+/* Report descriptor template placeholder IDs */
+enum uclogic_ph_id {
+	UCLOGIC_PH_ID_X_LM,
+	UCLOGIC_PH_ID_X_PM,
+	UCLOGIC_PH_ID_Y_LM,
+	UCLOGIC_PH_ID_Y_PM,
+	UCLOGIC_PH_ID_PRESSURE_LM,
+	UCLOGIC_PH_ID_NUM
+};
+
+/* Report descriptor template placeholder */
+#define UCLOGIC_PH(_ID) UCLOGIC_PH_HEAD, UCLOGIC_PH_ID_##_ID
+
+/* Fixed report descriptor template */
+static const __u8 uclogic_tablet_rdesc_template[] = {
+	0x05, 0x0D,             /*  Usage Page (Digitizer),                 */
+	0x09, 0x02,             /*  Usage (Pen),                            */
+	0xA1, 0x01,             /*  Collection (Application),               */
+	0x85, 0x07,             /*      Report ID (7),                      */
+	0x09, 0x20,             /*      Usage (Stylus),                     */
+	0xA0,                   /*      Collection (Physical),              */
+	0x14,                   /*          Logical Minimum (0),            */
+	0x25, 0x01,             /*          Logical Maximum (1),            */
+	0x75, 0x01,             /*          Report Size (1),                */
+	0x09, 0x42,             /*          Usage (Tip Switch),             */
+	0x09, 0x44,             /*          Usage (Barrel Switch),          */
+	0x09, 0x46,             /*          Usage (Tablet Pick),            */
+	0x95, 0x03,             /*          Report Count (3),               */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0x95, 0x03,             /*          Report Count (3),               */
+	0x81, 0x03,             /*          Input (Constant, Variable),     */
+	0x09, 0x32,             /*          Usage (In Range),               */
+	0x95, 0x01,             /*          Report Count (1),               */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0x95, 0x01,             /*          Report Count (1),               */
+	0x81, 0x03,             /*          Input (Constant, Variable),     */
+	0x75, 0x10,             /*          Report Size (16),               */
+	0x95, 0x01,             /*          Report Count (1),               */
+	0xA4,                   /*          Push,                           */
+	0x05, 0x01,             /*          Usage Page (Desktop),           */
+	0x65, 0x13,             /*          Unit (Inch),                    */
+	0x55, 0xFD,             /*          Unit Exponent (-3),             */
+	0x34,                   /*          Physical Minimum (0),           */
+	0x09, 0x30,             /*          Usage (X),                      */
+	0x27, UCLOGIC_PH(X_LM), /*          Logical Maximum (PLACEHOLDER),  */
+	0x47, UCLOGIC_PH(X_PM), /*          Physical Maximum (PLACEHOLDER), */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0x09, 0x31,             /*          Usage (Y),                      */
+	0x27, UCLOGIC_PH(Y_LM), /*          Logical Maximum (PLACEHOLDER),  */
+	0x47, UCLOGIC_PH(Y_PM), /*          Physical Maximum (PLACEHOLDER), */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0xB4,                   /*          Pop,                            */
+	0x09, 0x30,             /*          Usage (Tip Pressure),           */
+	0x27,
+	UCLOGIC_PH(PRESSURE_LM),/*          Logical Maximum (PLACEHOLDER),  */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0xC0,                   /*      End Collection,                     */
+	0xC0                    /*  End Collection                          */
+};
+
+/* Parameter indices */
+enum uclogic_prm {
+	UCLOGIC_PRM_X_LM	= 1,
+	UCLOGIC_PRM_Y_LM	= 2,
+	UCLOGIC_PRM_PRESSURE_LM	= 4,
+	UCLOGIC_PRM_RESOLUTION	= 5,
+	UCLOGIC_PRM_NUM
+};
+
+/* Driver data */
+struct uclogic_drvdata {
+	__u8 *rdesc;
+	unsigned int rsize;
+};
+
 static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 					unsigned int *rsize)
 {
 	struct usb_interface *iface = to_usb_interface(hdev->dev.parent);
 	__u8 iface_num = iface->cur_altsetting->desc.bInterfaceNumber;
+	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
 
 	switch (hdev->product) {
 	case USB_DEVICE_ID_UCLOGIC_TABLET_PF1209:
@@ -621,15 +703,120 @@ static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 			break;
 		}
 		break;
+	default:
+		if (drvdata->rdesc != NULL) {
+			rdesc = drvdata->rdesc;
+			*rsize = drvdata->rsize;
+		}
 	}
 
 	return rdesc;
 }
 
+/**
+ * Enable fully-functional tablet mode and determine device parameters.
+ *
+ * @hdev:	HID device
+ */
+static int uclogic_tablet_enable(struct hid_device *hdev)
+{
+	int rc;
+	struct usb_device *usb_dev = hid_to_usb_dev(hdev);
+	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
+	__le16 *buf = NULL;
+	size_t len;
+	s32 params[UCLOGIC_PH_ID_NUM];
+	s32 resolution;
+	__u8 *p;
+	s32 v;
+
+	/*
+	 * Read string descriptor containing tablet parameters. The specific
+	 * string descriptor and data were discovered by sniffing the Windows
+	 * driver traffic.
+	 * NOTE: This enables fully-functional tablet mode.
+	 */
+	len = UCLOGIC_PRM_NUM * sizeof(*buf);
+	buf = kmalloc(len, GFP_KERNEL);
+	if (buf == NULL) {
+		hid_err(hdev, "failed to allocate parameter buffer\n");
+		rc = -ENOMEM;
+		goto cleanup;
+	}
+	rc = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0),
+				USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
+				(USB_DT_STRING << 8) + 0x64,
+				0x0409, buf, len,
+				USB_CTRL_GET_TIMEOUT);
+	if (rc == -EPIPE) {
+		hid_err(hdev, "device parameters not found\n");
+		rc = -ENODEV;
+		goto cleanup;
+	} else if (rc < 0) {
+		hid_err(hdev, "failed to get device parameters: %d\n", rc);
+		rc = -ENODEV;
+		goto cleanup;
+	} else if (rc != len) {
+		hid_err(hdev, "invalid device parameters\n");
+		rc = -ENODEV;
+		goto cleanup;
+	}
+
+	/* Extract device parameters */
+	params[UCLOGIC_PH_ID_X_LM] = le16_to_cpu(buf[UCLOGIC_PRM_X_LM]);
+	params[UCLOGIC_PH_ID_Y_LM] = le16_to_cpu(buf[UCLOGIC_PRM_Y_LM]);
+	params[UCLOGIC_PH_ID_PRESSURE_LM] =
+		le16_to_cpu(buf[UCLOGIC_PRM_PRESSURE_LM]);
+	resolution = le16_to_cpu(buf[UCLOGIC_PRM_RESOLUTION]);
+	if (resolution == 0) {
+		params[UCLOGIC_PH_ID_X_PM] = 0;
+		params[UCLOGIC_PH_ID_Y_PM] = 0;
+	} else {
+		params[UCLOGIC_PH_ID_X_PM] = params[UCLOGIC_PH_ID_X_LM] *
+						1000 / resolution;
+		params[UCLOGIC_PH_ID_Y_PM] = params[UCLOGIC_PH_ID_Y_LM] *
+						1000 / resolution;
+	}
+
+	/* Allocate fixed report descriptor */
+	drvdata->rdesc = devm_kzalloc(&hdev->dev,
+				sizeof(uclogic_tablet_rdesc_template),
+				GFP_KERNEL);
+	if (drvdata->rdesc == NULL) {
+		hid_err(hdev, "failed to allocate fixed rdesc\n");
+		rc = -ENOMEM;
+		goto cleanup;
+	}
+	drvdata->rsize = sizeof(uclogic_tablet_rdesc_template);
+
+	/* Format fixed report descriptor */
+	memcpy(drvdata->rdesc, uclogic_tablet_rdesc_template,
+		drvdata->rsize);
+	for (p = drvdata->rdesc;
+	     p <= drvdata->rdesc + drvdata->rsize - 4;) {
+		if (p[0] == 0xFE && p[1] == 0xED && p[2] == 0x1D &&
+		    p[3] < sizeof(params)) {
+			v = params[p[3]];
+			put_unaligned(cpu_to_le32(v), (s32 *)p);
+			p += 4;
+		} else {
+			p++;
+		}
+	}
+
+	rc = 0;
+
+cleanup:
+	kfree(buf);
+	return rc;
+}
+
 static int uclogic_probe(struct hid_device *hdev,
 		const struct hid_device_id *id)
 {
 	int rc;
+	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+	struct uclogic_drvdata *drvdata;
 
 	/*
 	 * libinput requires the pad interface to be on a different node
@@ -637,6 +824,26 @@ static int uclogic_probe(struct hid_device *hdev,
 	 */
 	hdev->quirks |= HID_QUIRK_MULTI_INPUT;
 
+	/* Allocate and assign driver data */
+	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (drvdata == NULL)
+		return -ENOMEM;
+
+	hid_set_drvdata(hdev, drvdata);
+
+	switch (id->product) {
+	case USB_DEVICE_ID_HUION_TABLET:
+		/* If this is the pen interface */
+		if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
+			rc = uclogic_tablet_enable(hdev);
+			if (rc) {
+				hid_err(hdev, "tablet enabling failed\n");
+				return rc;
+			}
+		}
+		break;
+	}
+
 	rc = hid_parse(hdev);
 	if (rc) {
 		hid_err(hdev, "parse failed\n");
@@ -652,6 +859,21 @@ static int uclogic_probe(struct hid_device *hdev,
 	return 0;
 }
 
+static int uclogic_raw_event(struct hid_device *hdev, struct hid_report *report,
+			u8 *data, int size)
+{
+	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+
+	/* If this is a pen input report */
+	if (intf->cur_altsetting->desc.bInterfaceNumber == 0 &&
+	    report->type == HID_INPUT_REPORT &&
+	    report->id == 0x07 && size >= 2)
+		/* Invert the in-range bit */
+		data[1] ^= 0x40;
+
+	return 0;
+}
+
 static const struct hid_device_id uclogic_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC,
 				USB_DEVICE_ID_UCLOGIC_TABLET_PF1209) },
@@ -667,6 +889,8 @@ static const struct hid_device_id uclogic_devices[] = {
 				USB_DEVICE_ID_UCLOGIC_WIRELESS_TABLET_TWHL850) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC,
 				USB_DEVICE_ID_UCLOGIC_TABLET_TWHA60) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_HUION_TABLET) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, uclogic_devices);
@@ -676,7 +900,10 @@ static struct hid_driver uclogic_driver = {
 	.id_table = uclogic_devices,
 	.probe = uclogic_probe,
 	.report_fixup = uclogic_report_fixup,
+	.raw_event = uclogic_raw_event,
 };
 module_hid_driver(uclogic_driver);
 
+MODULE_AUTHOR("Martin Rusko");
+MODULE_AUTHOR("Nikolai Kondrashov");
 MODULE_LICENSE("GPL");
-- 
2.1.0

Re: [PATCH 2/4] HID: uclogic: merge hid-huion driver in hid-uclogic

[Nikolai Kondrashov]

On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> From: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>
>
> Merge the hid-huion driver into hid-uclogic as all the devices supported
> by hid-huion are in fact UC-Logic devices.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>
> Changed from the original patch:
> - update Makefile, Kconfig accordingly
> - remove one extra pr_err when kzalloc failed (raised by checkpatch.pl)
>
>   drivers/hid/Kconfig       |   8 +-
>   drivers/hid/Makefile      |   1 -
>   drivers/hid/hid-huion.c   | 290 ----------------------------------------------
>   drivers/hid/hid-uclogic.c | 229 +++++++++++++++++++++++++++++++++++-
>   4 files changed, 229 insertions(+), 299 deletions(-)
>   delete mode 100644 drivers/hid/hid-huion.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 152b006..8a55fd7 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -286,12 +286,6 @@ config HID_GT683R
>   	Currently the following devices are know to be supported:
>   	  - MSI GT683R
>
> -config HID_HUION
> -	tristate "Huion tablets"
> -	depends on USB_HID
> -	---help---
> -	Support for Huion 580 tablet.
> -
>   config HID_KEYTOUCH
>   	tristate "Keytouch HID devices"
>   	depends on HID
> @@ -314,7 +308,7 @@ config HID_UCLOGIC
>   	tristate "UC-Logic"
>   	depends on HID
>   	---help---
> -	Support for UC-Logic tablets.
> +	Support for UC-Logic and Huion tablets.

I'm not sure this is worth changing. We'll need to at least add Yiynova and
Ugee here later, which we're likely to forget. Meanwhile, essentially, they're
UC-Logic underneath.

Apart from this (which can be left as is, IMHO):

Reviewed-by: Nikolai Kondrashov <spbnick@gmail.com>

BTW, will you be submitting changes to digimend-kernel-drivers to align it
with what you're submitting upstream?

Nick

Re: [PATCH 2/4] HID: uclogic: merge hid-huion driver in hid-uclogic

[Benjamin Tissoires]

On Feb 25 2015 or thereabouts, Nikolai Kondrashov wrote:
> On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> >From: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>
> >
> >Merge the hid-huion driver into hid-uclogic as all the devices supported
> >by hid-huion are in fact UC-Logic devices.
> >
> >Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >---
> >
> >Changed from the original patch:
> >- update Makefile, Kconfig accordingly
> >- remove one extra pr_err when kzalloc failed (raised by checkpatch.pl)
> >
> >  drivers/hid/Kconfig       |   8 +-
> >  drivers/hid/Makefile      |   1 -
> >  drivers/hid/hid-huion.c   | 290 ----------------------------------------------
> >  drivers/hid/hid-uclogic.c | 229 +++++++++++++++++++++++++++++++++++-
> >  4 files changed, 229 insertions(+), 299 deletions(-)
> >  delete mode 100644 drivers/hid/hid-huion.c
> >
> >diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> >index 152b006..8a55fd7 100644
> >--- a/drivers/hid/Kconfig
> >+++ b/drivers/hid/Kconfig
> >@@ -286,12 +286,6 @@ config HID_GT683R
> >  	Currently the following devices are know to be supported:
> >  	  - MSI GT683R
> >
> >-config HID_HUION
> >-	tristate "Huion tablets"
> >-	depends on USB_HID
> >-	---help---
> >-	Support for Huion 580 tablet.
> >-
> >  config HID_KEYTOUCH
> >  	tristate "Keytouch HID devices"
> >  	depends on HID
> >@@ -314,7 +308,7 @@ config HID_UCLOGIC
> >  	tristate "UC-Logic"
> >  	depends on HID
> >  	---help---
> >-	Support for UC-Logic tablets.
> >+	Support for UC-Logic and Huion tablets.
> 
> I'm not sure this is worth changing. We'll need to at least add Yiynova and
> Ugee here later, which we're likely to forget. Meanwhile, essentially, they're
> UC-Logic underneath.

Well, true, we will likely forget it. But on the other hand, we should
be consistant with the current state of the kernel. Right now, Huion
users can look for huion in the menuconfig and find something. We should
not remove it :/

> 
> Apart from this (which can be left as is, IMHO):
> 
> Reviewed-by: Nikolai Kondrashov <spbnick@gmail.com>

Could I have your Signed-off-by here, it's mainly your patch, and so I
kept your From.

> 
> BTW, will you be submitting changes to digimend-kernel-drivers to align it
> with what you're submitting upstream?

Yep, as soon as the changes hit Jiri's tree (so they will end end in
Linus' tree at some point), I'll sync the digimend-kernel-drivers.

Cheers,
Benjamin

Re: [PATCH 2/4] HID: uclogic: merge hid-huion driver in hid-uclogic

[Nikolai Kondrashov]

On 02/25/2015 11:28 PM, Benjamin Tissoires wrote:
> On Feb 25 2015 or thereabouts, Nikolai Kondrashov wrote:
>> On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
>>
>> Reviewed-by: Nikolai Kondrashov <spbnick@gmail.com>
>
> Could I have your Signed-off-by here, it's mainly your patch, and so I
> kept your From.

Sure:

Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>

or, if necessary:

Signed-off-by: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>

>> BTW, will you be submitting changes to digimend-kernel-drivers to align it
>> with what you're submitting upstream?
>
> Yep, as soon as the changes hit Jiri's tree (so they will end end in
> Linus' tree at some point), I'll sync the digimend-kernel-drivers.

Great :)!

Nick

[PATCH 3/4] HID: uclogic: present only the working interfaces on the Huion tablets

[Benjamin Tissoires]

The Huion tablets show 3 interfaces. Only the first and the third
are currently used.
Also remove HID_QUIRK_MULTI_INPUT for the third interface to not
create more than needed input nodes.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-uclogic.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hid/hid-uclogic.c b/drivers/hid/hid-uclogic.c
index 397f1df..c03e076 100644
--- a/drivers/hid/hid-uclogic.c
+++ b/drivers/hid/hid-uclogic.c
@@ -840,6 +840,11 @@ static int uclogic_probe(struct hid_device *hdev,
 				hid_err(hdev, "tablet enabling failed\n");
 				return rc;
 			}
+		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
+			hdev->quirks &= ~HID_QUIRK_MULTI_INPUT;
+		} else {
+			/* Ignore all unused interfaces */
+			return -ENODEV;
 		}
 		break;
 	}
-- 
2.1.0

Re: [PATCH 3/4] HID: uclogic: present only the working interfaces on the Huion tablets

[Nikolai Kondrashov]

On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> The Huion tablets show 3 interfaces. Only the first and the third
> are currently used.
> Also remove HID_QUIRK_MULTI_INPUT for the third interface to not
> create more than needed input nodes.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>   drivers/hid/hid-uclogic.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hid/hid-uclogic.c b/drivers/hid/hid-uclogic.c
> index 397f1df..c03e076 100644
> --- a/drivers/hid/hid-uclogic.c
> +++ b/drivers/hid/hid-uclogic.c
> @@ -840,6 +840,11 @@ static int uclogic_probe(struct hid_device *hdev,
>   				hid_err(hdev, "tablet enabling failed\n");
>   				return rc;
>   			}
> +		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
> +			hdev->quirks &= ~HID_QUIRK_MULTI_INPUT;
> +		} else {
> +			/* Ignore all unused interfaces */
> +			return -ENODEV;
>   		}
>   		break;
>   	}
>

I would hesitate to remove the mouse interface as there are relatively fresh
UC-Logic tablets [1] out there which have a mouse, and which could respond to
the initialization. These mice are of questionable usefulness, true, but
leaving the interface enabled could make it easier for other people to
experiment with using this driver with their tablets.

What do you think, Benjamin?

Nick

[1] http://digimend.github.io/tablets/UC-Logic_TWHL850/

Re: [PATCH 3/4] HID: uclogic: present only the working interfaces on the Huion tablets

[Benjamin Tissoires]

On Feb 25 2015 or thereabouts, Nikolai Kondrashov wrote:
> On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> >The Huion tablets show 3 interfaces. Only the first and the third
> >are currently used.
> >Also remove HID_QUIRK_MULTI_INPUT for the third interface to not
> >create more than needed input nodes.
> >
> >Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >---
> >  drivers/hid/hid-uclogic.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> >diff --git a/drivers/hid/hid-uclogic.c b/drivers/hid/hid-uclogic.c
> >index 397f1df..c03e076 100644
> >--- a/drivers/hid/hid-uclogic.c
> >+++ b/drivers/hid/hid-uclogic.c
> >@@ -840,6 +840,11 @@ static int uclogic_probe(struct hid_device *hdev,
> >  				hid_err(hdev, "tablet enabling failed\n");
> >  				return rc;
> >  			}
> >+		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
> >+			hdev->quirks &= ~HID_QUIRK_MULTI_INPUT;
> >+		} else {
> >+			/* Ignore all unused interfaces */
> >+			return -ENODEV;
> >  		}
> >  		break;
> >  	}
> >
> 
> I would hesitate to remove the mouse interface as there are relatively fresh
> UC-Logic tablets [1] out there which have a mouse, and which could respond to
> the initialization. These mice are of questionable usefulness, true, but
> leaving the interface enabled could make it easier for other people to
> experiment with using this driver with their tablets.

OK, I will just remove the quirk for the mouse interface then.

> 
> What do you think, Benjamin?
> 

You have a better knowledge of the currently available devices. So I'll
follow your ideas.

> Nick
> 
> [1] http://digimend.github.io/tablets/UC-Logic_TWHL850/

Re: [PATCH 3/4] HID: uclogic: present only the working interfaces on the Huion tablets

[Benjamin Tissoires]

On Feb 25 2015 or thereabouts, Benjamin Tissoires wrote:
> On Feb 25 2015 or thereabouts, Nikolai Kondrashov wrote:
> > On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> > >The Huion tablets show 3 interfaces. Only the first and the third
> > >are currently used.
> > >Also remove HID_QUIRK_MULTI_INPUT for the third interface to not
> > >create more than needed input nodes.
> > >
> > >Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > >---
> > >  drivers/hid/hid-uclogic.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > >diff --git a/drivers/hid/hid-uclogic.c b/drivers/hid/hid-uclogic.c
> > >index 397f1df..c03e076 100644
> > >--- a/drivers/hid/hid-uclogic.c
> > >+++ b/drivers/hid/hid-uclogic.c
> > >@@ -840,6 +840,11 @@ static int uclogic_probe(struct hid_device *hdev,
> > >  				hid_err(hdev, "tablet enabling failed\n");
> > >  				return rc;
> > >  			}
> > >+		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
> > >+			hdev->quirks &= ~HID_QUIRK_MULTI_INPUT;
> > >+		} else {
> > >+			/* Ignore all unused interfaces */
> > >+			return -ENODEV;
> > >  		}
> > >  		break;
> > >  	}
> > >
> > 
> > I would hesitate to remove the mouse interface as there are relatively fresh
> > UC-Logic tablets [1] out there which have a mouse, and which could respond to
> > the initialization. These mice are of questionable usefulness, true, but
> > leaving the interface enabled could make it easier for other people to
> > experiment with using this driver with their tablets.
> 
> OK, I will just remove the quirk for the mouse interface then.
> 

I ended up removing this patch in the series. Having
HID_QUIRK_MULTI_INPUT set allows a fine tuning of the names of the input
nodes so we can just enhance the list of suffix and get something
sensible for the users.

The only concern is that there will be a second "Pen" interface, but
that can be removed based on the PID (or fixed with a report descriptor).
I'll add an extra patch for that too.

Cheers,
Benjamin

[PATCH 4/4] HID: uclogic: name the input nodes based on their tool

[Benjamin Tissoires]

We append "Pen", "Pad", "Mouse" or "Keyboard" suffix to the appropriate
input node to match what the Wacom driver does and be more convenient for
the user to know which one is which.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-uclogic.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/hid/hid-uclogic.c b/drivers/hid/hid-uclogic.c
index c03e076..7042238 100644
--- a/drivers/hid/hid-uclogic.c
+++ b/drivers/hid/hid-uclogic.c
@@ -713,6 +713,45 @@ static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+static void uclogic_input_configured(struct hid_device *hdev,
+		struct hid_input *hi)
+{
+	char *name;
+	const char *suffix = NULL;
+	struct hid_field *field;
+	size_t len;
+
+	/* no report associated (HID_QUIRK_MULTI_INPUT not set) */
+	if (!hi->report)
+		return;
+
+	field = hi->report->field[0];
+
+	switch (field->application) {
+	case HID_GD_KEYBOARD:
+		suffix = "Keyboard";
+		break;
+	case HID_GD_MOUSE:
+		suffix = "Mouse";
+		break;
+	case HID_GD_KEYPAD:
+		suffix = "Pad";
+		break;
+	case HID_DG_PEN:
+		suffix = "Pen";
+		break;
+	}
+
+	if (suffix) {
+		len = strlen(hdev->name) + 2 + strlen(suffix);
+		name = devm_kzalloc(&hi->input->dev, len, GFP_KERNEL);
+		if (name) {
+			snprintf(name, len, "%s %s", hdev->name, suffix);
+			hi->input->name = name;
+		}
+	}
+}
+
 /**
  * Enable fully-functional tablet mode and determine device parameters.
  *
@@ -906,6 +945,7 @@ static struct hid_driver uclogic_driver = {
 	.probe = uclogic_probe,
 	.report_fixup = uclogic_report_fixup,
 	.raw_event = uclogic_raw_event,
+	.input_configured = uclogic_input_configured,
 };
 module_hid_driver(uclogic_driver);
 
-- 
2.1.0

Re: [PATCH 4/4] HID: uclogic: name the input nodes based on their tool

[Nikolai Kondrashov]

On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> We append "Pen", "Pad", "Mouse" or "Keyboard" suffix to the appropriate
> input node to match what the Wacom driver does and be more convenient for
> the user to know which one is which.

As I said before, this is a very welcome change. Thanks, Benjamin!
However, I'm not familiar with input_configured hook yet, so cannot give my
Reviewed-by.

> +	if (suffix) {
> +		len = strlen(hdev->name) + 2 + strlen(suffix);
> +		name = devm_kzalloc(&hi->input->dev, len, GFP_KERNEL);
> +		if (name) {
> +			snprintf(name, len, "%s %s", hdev->name, suffix);
> +			hi->input->name = name;
> +		}

I only feel a bit uneasy about duplicating buffer size knowledge here, as I
said before, but the code is short, so it'll probably be OK.

Nick

Re: [PATCH 4/4] HID: uclogic: name the input nodes based on their tool

[Benjamin Tissoires]

On Feb 25 2015 or thereabouts, Nikolai Kondrashov wrote:
> On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
> >We append "Pen", "Pad", "Mouse" or "Keyboard" suffix to the appropriate
> >input node to match what the Wacom driver does and be more convenient for
> >the user to know which one is which.
> 
> As I said before, this is a very welcome change. Thanks, Benjamin!
> However, I'm not familiar with input_configured hook yet, so cannot give my
> Reviewed-by.

Well, the hook is called just before we present the input to the user
space. So the input node is already set and its corresponding report
has been parsed (or the whole descriptor has been parsed if
QUIRK_MULTI_INPUT is not set).

> 
> >+	if (suffix) {
> >+		len = strlen(hdev->name) + 2 + strlen(suffix);
> >+		name = devm_kzalloc(&hi->input->dev, len, GFP_KERNEL);
> >+		if (name) {
> >+			snprintf(name, len, "%s %s", hdev->name, suffix);
> >+			hi->input->name = name;
> >+		}
> 
> I only feel a bit uneasy about duplicating buffer size knowledge here, as I
> said before, but the code is short, so it'll probably be OK.

I am not quite sure what you mean with "duplicating buffer size
knowledge"...

Cheers,
Benjamin

> 
> Nick

Re: [PATCH 4/4] HID: uclogic: name the input nodes based on their tool

[Nikolai Kondrashov]

On 02/25/2015 11:36 PM, Benjamin Tissoires wrote:
> On Feb 25 2015 or thereabouts, Nikolai Kondrashov wrote:
>> On 02/25/2015 02:05 AM, Benjamin Tissoires wrote:
>>> We append "Pen", "Pad", "Mouse" or "Keyboard" suffix to the appropriate
>>> input node to match what the Wacom driver does and be more convenient for
>>> the user to know which one is which.
>>
>> As I said before, this is a very welcome change. Thanks, Benjamin!
>> However, I'm not familiar with input_configured hook yet, so cannot give my
>> Reviewed-by.
>
> Well, the hook is called just before we present the input to the user
> space. So the input node is already set and its corresponding report
> has been parsed (or the whole descriptor has been parsed if
> QUIRK_MULTI_INPUT is not set).

Thanks for the explanation, but basing my Reviewed-by on your explanation
wouldn't be right, even though this seems simple :) I'll read up the kernel
code tomorrow.

>>
>>> +	if (suffix) {
>>> +		len = strlen(hdev->name) + 2 + strlen(suffix);
>>> +		name = devm_kzalloc(&hi->input->dev, len, GFP_KERNEL);
>>> +		if (name) {
>>> +			snprintf(name, len, "%s %s", hdev->name, suffix);
>>> +			hi->input->name = name;
>>> +		}
>>
>> I only feel a bit uneasy about duplicating buffer size knowledge here, as I
>> said before, but the code is short, so it'll probably be OK.
>
> I am not quite sure what you mean with "duplicating buffer size
> knowledge"...

Argh, nevermind, just sleepy me, sorry. You fixed it, thanks, looks fine :)

Nick