Thread: [DIGImend-devel] [PATCHES] hid: Add support for more Huion tablets
Brought to you by:
spb_nick
Menu
▾
▴
digimend-devel
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 12:43:26
|
Hi everyone, These patches add support for more Huion tablet models. At least five models were verified working with the out-of-tree driver [1] which these patches are based on. This in-tree driver was verified working with Huion H610N. The Huion tablets seem to all use a single product ID and the iProduct strings are inconsistent. So there isn't much to identify the models with so far, although the magic string descriptor containing tablet parameters has some internal model name which might be used to identify them in the future. For now, though, the driver leaves the mouse and keyboard interfaces enabled unconditionally as some models do use them, and it doesn't try to adjust keyboard report descriptor or interpret keyboard reports to make reported key combinations more suitable to Linux applications as they might be different between models. The driver uses a somewhat non-standard approach: it puts the parameters extracted from the magic string descriptor into a report descriptor template and then feeds it to the generic HID layer. This appears simpler than creating the input device and then parsing the reports in the driver. However, if the particular approach or implementation is wrong or undesirable in some way, I'm ready to change it. Thank you. Nick drivers/hid/hid-core.c | 2 +- drivers/hid/hid-huion.c | 271 ++++++++++++++++++++++++++++++++---------------- drivers/hid/hid-ids.h | 2 +- 3 files changed, 182 insertions(+), 93 deletions(-) [1] https://github.com/DIGImend/huion-driver |
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 12:43:27
|
Use word "tablet" in identifiers and comments instead of referring to specific
Huion tablet model name, as the product ID is used by at least several tablet
models.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-core.c | 2 +-
drivers/hid/hid-huion.c | 25 +++++++++++++------------
drivers/hid/hid-ids.h | 2 +-
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8ed66fd..84ed7d4 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1782,7 +1782,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A070) },
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A072) },
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A081) },
- { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_580) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
{ HID_USB_DEVICE(USB_VENDOR_ID_JESS2, USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ION, USB_DEVICE_ID_ICADE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON, USB_DEVICE_ID_KS_SLIMBLADE) },
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index cbf4da4..25d01cd 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -2,6 +2,7 @@
* HID driver for Huion devices not fully compliant with HID standard
*
* Copyright (c) 2013 Martin Rusko
+ * Copyright (c) 2014 Nikolai Kondrashov
*/
/*
@@ -19,11 +20,11 @@
#include "hid-ids.h"
-/* Original Huion 580 report descriptor size */
-#define HUION_580_RDESC_ORIG_SIZE 177
+/* Original tablet report descriptor size */
+#define HUION_TABLET_RDESC_ORIG_SIZE 177
-/* Fixed Huion 580 report descriptor */
-static __u8 huion_580_rdesc_fixed[] = {
+/* Fixed tablet report descriptor */
+static __u8 huion_tablet_rdesc_fixed[] = {
0x05, 0x0D, /* Usage Page (Digitizer), */
0x09, 0x02, /* Usage (Pen), */
0xA1, 0x01, /* Collection (Application), */
@@ -72,10 +73,10 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
switch (hdev->product) {
- case USB_DEVICE_ID_HUION_580:
- if (*rsize == HUION_580_RDESC_ORIG_SIZE) {
- rdesc = huion_580_rdesc_fixed;
- *rsize = sizeof(huion_580_rdesc_fixed);
+ case USB_DEVICE_ID_HUION_TABLET:
+ if (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
+ rdesc = huion_tablet_rdesc_fixed;
+ *rsize = sizeof(huion_tablet_rdesc_fixed);
}
break;
}
@@ -108,11 +109,11 @@ static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret;
struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
- /* Ignore interfaces 1 (mouse) and 2 (keyboard) for Huion 580 tablet,
+ /* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
* as they are not used
*/
switch (id->product) {
- case USB_DEVICE_ID_HUION_580:
+ case USB_DEVICE_ID_HUION_TABLET:
if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
return -ENODEV;
break;
@@ -131,7 +132,7 @@ static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
switch (id->product) {
- case USB_DEVICE_ID_HUION_580:
+ case USB_DEVICE_ID_HUION_TABLET:
ret = huion_tablet_enable(hdev);
if (ret) {
hid_err(hdev, "tablet enabling failed\n");
@@ -158,7 +159,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
}
static const struct hid_device_id huion_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_580) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
{ }
};
MODULE_DEVICE_TABLE(hid, huion_devices);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 48b66bb..fe2f163 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -448,7 +448,7 @@
#define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
#define USB_VENDOR_ID_HUION 0x256c
-#define USB_DEVICE_ID_HUION_580 0x006e
+#define USB_DEVICE_ID_HUION_TABLET 0x006e
#define USB_VENDOR_ID_IDEACOM 0x1cb6
#define USB_DEVICE_ID_IDEACOM_IDC6650 0x6650
--
2.0.1
|
|
From: Benjamin T. <ben...@gm...> - 2014-07-23 14:30:09
|
On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <sp...@gm...> wrote: > Use word "tablet" in identifiers and comments instead of referring to specific > Huion tablet model name, as the product ID is used by at least several tablet > models. > > Signed-off-by: Nikolai Kondrashov <sp...@gm...> > --- Reviewed-by: Benjamin Tissoires <ben...@re...> Cheers, Benjamin |
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 12:43:29
|
Limit inverting the in-range bit in raw reports to tablet product ID
only. This will make adding handling of other, non-tablet products,
easier.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-huion.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 25d01cd..db24472 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -151,9 +151,15 @@ err:
static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *data, int size)
{
- /* If this is a pen input report then invert the in-range bit */
- if (report->type == HID_INPUT_REPORT && report->id == 0x07 && size >= 2)
- data[1] ^= 0x40;
+ switch (hdev->product) {
+ case USB_DEVICE_ID_HUION_TABLET:
+ /* If this is a pen input report */
+ if (report->type == HID_INPUT_REPORT &&
+ report->id == 0x07 && size >= 2)
+ /* Invert the in-range bit */
+ data[1] ^= 0x40;
+ break;
+ }
return 0;
}
--
2.0.1
|
|
From: Benjamin T. <ben...@gm...> - 2014-07-23 14:34:39
|
On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <sp...@gm...> wrote:
> Limit inverting the in-range bit in raw reports to tablet product ID
> only. This will make adding handling of other, non-tablet products,
> easier.
>
> Signed-off-by: Nikolai Kondrashov <sp...@gm...>
> ---
I am not particularly a big fan of this one. You are here adding a
test which will be called at each raw_event but currently only tablet
products are bound to hid-huion. Even in the rest of the series, you
add another VID/PID, but it still has the same PID.
So I would say that this will be nice to have when we really have the
problem, not now.
But if you tell me that you already have the need for it, I am fine
with it. It's just that this commit message + the rest of the patch
series makes me feel like this is just a superflous test.
So, in its current state:
NACK
Cheers,
Benjamin
> drivers/hid/hid-huion.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
> index 25d01cd..db24472 100644
> --- a/drivers/hid/hid-huion.c
> +++ b/drivers/hid/hid-huion.c
> @@ -151,9 +151,15 @@ err:
> static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
> u8 *data, int size)
> {
> - /* If this is a pen input report then invert the in-range bit */
> - if (report->type == HID_INPUT_REPORT && report->id == 0x07 && size >= 2)
> - data[1] ^= 0x40;
> + switch (hdev->product) {
> + case USB_DEVICE_ID_HUION_TABLET:
> + /* If this is a pen input report */
> + if (report->type == HID_INPUT_REPORT &&
> + report->id == 0x07 && size >= 2)
> + /* Invert the in-range bit */
> + data[1] ^= 0x40;
> + break;
> + }
>
> return 0;
> }
> --
> 2.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to maj...@vg...
> More majordomo info at http://vger.kernel.org/majordomo-info.html
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 14:40:29
|
On 07/23/2014 05:34 PM, Benjamin Tissoires wrote: > On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <sp...@gm...> wrote: >> Limit inverting the in-range bit in raw reports to tablet product ID >> only. This will make adding handling of other, non-tablet products, >> easier. >> >> Signed-off-by: Nikolai Kondrashov <sp...@gm...> >> --- > > I am not particularly a big fan of this one. You are here adding a > test which will be called at each raw_event but currently only tablet > products are bound to hid-huion. Even in the rest of the series, you > add another VID/PID, but it still has the same PID. > > So I would say that this will be nice to have when we really have the > problem, not now. > > But if you tell me that you already have the need for it, I am fine > with it. It's just that this commit message + the rest of the patch > series makes me feel like this is just a superflous test. > > So, in its current state: > NACK I had doubts about this one myself, but left it in just for consistency with some other drivers. I'll drop it in the next version then. Thank you. Nick |
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 16:32:17
|
Hi everyone, This is the second version of the patch set adding support for more Huion tablets [1]. This version has the "hid: huion: Invert in-range on specific product" patch removed as requested by Benjamin Tissoires. Tested with Huion H610N. Nick [PATCH 1/4] hid: huion: Use "tablet" instead of specific model [PATCH 2/4] hid: huion: Don't ignore other interfaces [PATCH 3/4] hid: huion: Switch to generating report descriptor [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID drivers/hid/hid-core.c | 2 +- drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++---------------- drivers/hid/hid-ids.h | 2 +- 3 files changed, 177 insertions(+), 92 deletions(-) [1] http://thread.gmane.org/gmane.linux.kernel.input/37187 |
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 16:32:20
|
Use word "tablet" in identifiers and comments instead of referring to specific
Huion tablet model name, as the product ID is used by at least several tablet
models.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-core.c | 2 +-
drivers/hid/hid-huion.c | 25 +++++++++++++------------
drivers/hid/hid-ids.h | 2 +-
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8ed66fd..84ed7d4 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1782,7 +1782,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A070) },
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A072) },
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A081) },
- { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_580) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
{ HID_USB_DEVICE(USB_VENDOR_ID_JESS2, USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ION, USB_DEVICE_ID_ICADE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON, USB_DEVICE_ID_KS_SLIMBLADE) },
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index cbf4da4..25d01cd 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -2,6 +2,7 @@
* HID driver for Huion devices not fully compliant with HID standard
*
* Copyright (c) 2013 Martin Rusko
+ * Copyright (c) 2014 Nikolai Kondrashov
*/
/*
@@ -19,11 +20,11 @@
#include "hid-ids.h"
-/* Original Huion 580 report descriptor size */
-#define HUION_580_RDESC_ORIG_SIZE 177
+/* Original tablet report descriptor size */
+#define HUION_TABLET_RDESC_ORIG_SIZE 177
-/* Fixed Huion 580 report descriptor */
-static __u8 huion_580_rdesc_fixed[] = {
+/* Fixed tablet report descriptor */
+static __u8 huion_tablet_rdesc_fixed[] = {
0x05, 0x0D, /* Usage Page (Digitizer), */
0x09, 0x02, /* Usage (Pen), */
0xA1, 0x01, /* Collection (Application), */
@@ -72,10 +73,10 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
switch (hdev->product) {
- case USB_DEVICE_ID_HUION_580:
- if (*rsize == HUION_580_RDESC_ORIG_SIZE) {
- rdesc = huion_580_rdesc_fixed;
- *rsize = sizeof(huion_580_rdesc_fixed);
+ case USB_DEVICE_ID_HUION_TABLET:
+ if (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
+ rdesc = huion_tablet_rdesc_fixed;
+ *rsize = sizeof(huion_tablet_rdesc_fixed);
}
break;
}
@@ -108,11 +109,11 @@ static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret;
struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
- /* Ignore interfaces 1 (mouse) and 2 (keyboard) for Huion 580 tablet,
+ /* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
* as they are not used
*/
switch (id->product) {
- case USB_DEVICE_ID_HUION_580:
+ case USB_DEVICE_ID_HUION_TABLET:
if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
return -ENODEV;
break;
@@ -131,7 +132,7 @@ static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
switch (id->product) {
- case USB_DEVICE_ID_HUION_580:
+ case USB_DEVICE_ID_HUION_TABLET:
ret = huion_tablet_enable(hdev);
if (ret) {
hid_err(hdev, "tablet enabling failed\n");
@@ -158,7 +159,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
}
static const struct hid_device_id huion_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_580) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
{ }
};
MODULE_DEVICE_TABLE(hid, huion_devices);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 48b66bb..fe2f163 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -448,7 +448,7 @@
#define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
#define USB_VENDOR_ID_HUION 0x256c
-#define USB_DEVICE_ID_HUION_580 0x006e
+#define USB_DEVICE_ID_HUION_TABLET 0x006e
#define USB_VENDOR_ID_IDEACOM 0x1cb6
#define USB_DEVICE_ID_IDEACOM_IDC6650 0x6650
--
2.0.1
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 16:32:23
|
Don't ignore non pen-reporting interfaces as they may be used by some
models reusing the same product ID.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-huion.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 25d01cd..46c425b 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -107,17 +107,6 @@ static int huion_tablet_enable(struct hid_device *hdev)
static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
int ret;
- struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-
- /* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
- * as they are not used
- */
- switch (id->product) {
- case USB_DEVICE_ID_HUION_TABLET:
- if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
- return -ENODEV;
- break;
- }
ret = hid_parse(hdev);
if (ret) {
@@ -151,8 +140,13 @@ err:
static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *data, int size)
{
- /* If this is a pen input report then invert the in-range bit */
- if (report->type == HID_INPUT_REPORT && report->id == 0x07 && size >= 2)
+ 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;
--
2.0.1
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 16:32:25
|
Add handling of Huion tablets with UC-Logic vendor ID, but the same
product ID to hid-huion driver. A Huion H580 was seen using it.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-huion.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 6c811c1..438b54e 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -243,6 +243,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
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);
--
2.0.1
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 16:32:26
|
Switch to generating tablet pen report descriptor from a template and
parameters retrieved from string descriptor 0x64.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-huion.c | 245 +++++++++++++++++++++++++++++++++---------------
1 file changed, 167 insertions(+), 78 deletions(-)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 46c425b..6c811c1 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -16,67 +16,89 @@
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/usb.h>
+#include <asm/unaligned.h>
#include "usbhid/usbhid.h"
#include "hid-ids.h"
-/* Original tablet report descriptor size */
-#define HUION_TABLET_RDESC_ORIG_SIZE 177
-
-/* Fixed tablet report descriptor */
-static __u8 huion_tablet_rdesc_fixed[] = {
- 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), */
- 0x46, 0x40, 0x1F, /* Physical Maximum (8000), */
- 0x26, 0x00, 0x7D, /* Logical Maximum (32000), */
- 0x81, 0x02, /* Input (Variable), */
- 0x09, 0x31, /* Usage (Y), */
- 0x46, 0x88, 0x13, /* Physical Maximum (5000), */
- 0x26, 0x20, 0x4E, /* Logical Maximum (20000), */
- 0x81, 0x02, /* Input (Variable), */
- 0xB4, /* Pop, */
- 0x09, 0x30, /* Usage (Tip Pressure), */
- 0x26, 0xFF, 0x07, /* Logical Maximum (2047), */
- 0x81, 0x02, /* Input (Variable), */
- 0xC0, /* End Collection, */
- 0xC0 /* End Collection */
+/* 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 */
+};
+
+/* 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 (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
- rdesc = huion_tablet_rdesc_fixed;
- *rsize = sizeof(huion_tablet_rdesc_fixed);
+ if (drvdata->rdesc != NULL) {
+ rdesc = drvdata->rdesc;
+ *rsize = drvdata->rsize;
}
break;
}
@@ -84,57 +106,124 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
}
/**
- * Enable fully-functional tablet mode by reading special string
- * descriptor.
+ * Enable fully-functional tablet mode and determine device parameters.
*
* @hdev: HID device
- *
- * The specific string descriptor and data were discovered by sniffing
- * the Windows driver traffic.
*/
static int huion_tablet_enable(struct hid_device *hdev)
{
int rc;
- char buf[22];
+ struct usb_device *usb_dev = hid_to_usb_dev(hdev);
+ struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 buf[6];
- rc = usb_string(hid_to_usb_dev(hdev), 0x64, buf, sizeof(buf));
- if (rc < 0)
- return rc;
+ /*
+ * 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.
+ */
+ 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, sizeof(buf),
+ USB_CTRL_GET_TIMEOUT);
+ if (rc == -EPIPE)
+ hid_warn(hdev, "device parameters not found\n");
+ else if (rc < 0)
+ hid_warn(hdev, "failed to get device parameters: %d\n", rc);
+ else if (rc != sizeof(buf))
+ hid_warn(hdev, "invalid device parameters\n");
+ else {
+ s32 params[HUION_PH_ID_NUM];
+ s32 resolution;
+ __u8 *p;
+ s32 v;
+
+ /* Extract device parameters */
+ params[HUION_PH_ID_X_LM] = le16_to_cpu(buf[1]);
+ params[HUION_PH_ID_Y_LM] = le16_to_cpu(buf[2]);
+ params[HUION_PH_ID_PRESSURE_LM] = le16_to_cpu(buf[4]);
+ resolution = le16_to_cpu(buf[5]);
+ 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");
+ return -ENOMEM;
+ }
+ 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++;
+ }
+ }
+ }
return 0;
}
static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
- int ret;
-
- ret = hid_parse(hdev);
- if (ret) {
- hid_err(hdev, "parse failed\n");
- goto err;
- }
+ int rc;
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct huion_drvdata *drvdata;
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret) {
- hid_err(hdev, "hw start failed\n");
- goto err;
+ /* 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:
- ret = huion_tablet_enable(hdev);
- if (ret) {
- hid_err(hdev, "tablet enabling failed\n");
- goto enabling_err;
+ /* 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;
-enabling_err:
- hid_hw_stop(hdev);
-err:
- return ret;
}
static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
--
2.0.1
|
|
From: Benjamin T. <ben...@gm...> - 2014-07-28 15:33:45
|
On Wed, Jul 23, 2014 at 12:31 PM, Nikolai Kondrashov <sp...@gm...> wrote: > Hi everyone, > > This is the second version of the patch set adding support for more Huion > tablets [1]. > > This version has the "hid: huion: Invert in-range on specific product" patch > removed as requested by Benjamin Tissoires. > > Tested with Huion H610N. > > Nick > > [PATCH 1/4] hid: huion: Use "tablet" instead of specific model > [PATCH 2/4] hid: huion: Don't ignore other interfaces > [PATCH 3/4] hid: huion: Switch to generating report descriptor > [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID > > drivers/hid/hid-core.c | 2 +- > drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++---------------- > drivers/hid/hid-ids.h | 2 +- > 3 files changed, 177 insertions(+), 92 deletions(-) > > [1] http://thread.gmane.org/gmane.linux.kernel.input/37187 Just so that the info does not get lost, the v2 of the patch series is still: Reviewed-by: Benjamin Tissoires <ben...@re...> Cheers, Benjamin |
|
From: Jiri K. <jk...@su...> - 2014-07-29 09:22:10
|
On Mon, 28 Jul 2014, Benjamin Tissoires wrote: > > This is the second version of the patch set adding support for more Huion > > tablets [1]. > > > > This version has the "hid: huion: Invert in-range on specific product" patch > > removed as requested by Benjamin Tissoires. > > > > Tested with Huion H610N. > > > > Nick > > > > [PATCH 1/4] hid: huion: Use "tablet" instead of specific model > > [PATCH 2/4] hid: huion: Don't ignore other interfaces > > [PATCH 3/4] hid: huion: Switch to generating report descriptor > > [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID > > > > drivers/hid/hid-core.c | 2 +- > > drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++---------------- > > drivers/hid/hid-ids.h | 2 +- > > 3 files changed, 177 insertions(+), 92 deletions(-) > > > > [1] http://thread.gmane.org/gmane.linux.kernel.input/37187 > > Just so that the info does not get lost, the v2 of the patch series is still: > > Reviewed-by: Benjamin Tissoires <ben...@re...> Queuing for 3.17, thanks. -- Jiri Kosina SUSE Labs |
|
From: Nikolai K. <sp...@gm...> - 2014-07-29 12:50:23
|
Fix sparse warnings in hid-huion.c by using correct buffer type for
retrieved string descriptor.
The warnings in question were:
drivers/hid/hid-huion.c:144:44: sparse: cast to restricted __le16
drivers/hid/hid-huion.c:145:44: sparse: cast to restricted __le16
drivers/hid/hid-huion.c:146:51: sparse: cast to restricted __le16
drivers/hid/hid-huion.c:147:30: sparse: cast to restricted __le16
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
Hi Jiri, Benjamin,
Thanks for the review. There is one minor problem, though. I didn't run a
sparse check and only just got a message with "sparse" warnings. This patch
fixes them.
If you wouldn't like adding a tiny patch like this, I can send another version
of the original patches. Otherwise, please feel free to just fixup my "Switch
to generating report descriptor" commit with this one.
I'll try to remember to run sparse next time before submitting the patches.
Thank you.
Nick
drivers/hid/hid-huion.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 438b54e..60f44cd 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -115,7 +115,7 @@ 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);
- u16 buf[6];
+ __le16 buf[6];
/*
* Read string descriptor containing tablet parameters. The specific
--
2.0.1
|
|
From: Jiri K. <jk...@su...> - 2014-07-29 13:06:42
|
On Tue, 29 Jul 2014, Nikolai Kondrashov wrote: > Fix sparse warnings in hid-huion.c by using correct buffer type for > retrieved string descriptor. > > The warnings in question were: > > drivers/hid/hid-huion.c:144:44: sparse: cast to restricted __le16 > drivers/hid/hid-huion.c:145:44: sparse: cast to restricted __le16 > drivers/hid/hid-huion.c:146:51: sparse: cast to restricted __le16 > drivers/hid/hid-huion.c:147:30: sparse: cast to restricted __le16 Interestingly enough, my sparse doesn't seem to report this problem (running sparse is part of my machinery before pushing anything out). > Signed-off-by: Nikolai Kondrashov <sp...@gm...> > --- > > Hi Jiri, Benjamin, > > Thanks for the review. There is one minor problem, though. I didn't run a > sparse check and only just got a message with "sparse" warnings. This patch > fixes them. > > If you wouldn't like adding a tiny patch like this, I can send another version > of the original patches. Otherwise, please feel free to just fixup my "Switch > to generating report descriptor" commit with this one. Yup, I'll be applying this on top of your previous one. Thanks. -- Jiri Kosina SUSE Labs |
|
From: Nikolai K. <sp...@gm...> - 2014-07-29 13:24:25
|
On 07/29/2014 04:06 PM, Jiri Kosina wrote: > On Tue, 29 Jul 2014, Nikolai Kondrashov wrote: > >> Fix sparse warnings in hid-huion.c by using correct buffer type for >> retrieved string descriptor. >> >> The warnings in question were: >> >> drivers/hid/hid-huion.c:144:44: sparse: cast to restricted __le16 >> drivers/hid/hid-huion.c:145:44: sparse: cast to restricted __le16 >> drivers/hid/hid-huion.c:146:51: sparse: cast to restricted __le16 >> drivers/hid/hid-huion.c:147:30: sparse: cast to restricted __le16 > > Interestingly enough, my sparse doesn't seem to report this problem > (running sparse is part of my machinery before pushing anything out). The above only appears with CF=-D__CHECK_ENDIAN__. >> If you wouldn't like adding a tiny patch like this, I can send another version >> of the original patches. Otherwise, please feel free to just fixup my "Switch >> to generating report descriptor" commit with this one. > > Yup, I'll be applying this on top of your previous one. Thanks! Nick |
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 12:43:31
|
Don't ignore non pen-reporting interfaces as they may be used by some
models reusing the same product ID.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-huion.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index db24472..e5f1e22 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -107,17 +107,6 @@ static int huion_tablet_enable(struct hid_device *hdev)
static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
int ret;
- struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-
- /* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
- * as they are not used
- */
- switch (id->product) {
- case USB_DEVICE_ID_HUION_TABLET:
- if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
- return -ENODEV;
- break;
- }
ret = hid_parse(hdev);
if (ret) {
@@ -151,10 +140,13 @@ err:
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);
+
switch (hdev->product) {
case USB_DEVICE_ID_HUION_TABLET:
/* If this is a pen input report */
- if (report->type == HID_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;
--
2.0.1
|
|
From: Benjamin T. <ben...@gm...> - 2014-07-23 14:43:54
|
On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <sp...@gm...> wrote:
> Don't ignore non pen-reporting interfaces as they may be used by some
> models reusing the same product ID.
>
> Signed-off-by: Nikolai Kondrashov <sp...@gm...>
> ---
Reviewed-by: Benjamin Tissoires <ben...@re...>
> drivers/hid/hid-huion.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
> index db24472..e5f1e22 100644
> --- a/drivers/hid/hid-huion.c
> +++ b/drivers/hid/hid-huion.c
> @@ -107,17 +107,6 @@ static int huion_tablet_enable(struct hid_device *hdev)
> static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
> {
> int ret;
> - struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> -
> - /* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
> - * as they are not used
> - */
> - switch (id->product) {
> - case USB_DEVICE_ID_HUION_TABLET:
> - if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
> - return -ENODEV;
> - break;
> - }
>
> ret = hid_parse(hdev);
> if (ret) {
> @@ -151,10 +140,13 @@ err:
> 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);
> +
> switch (hdev->product) {
> case USB_DEVICE_ID_HUION_TABLET:
> /* If this is a pen input report */
> - if (report->type == HID_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;
> --
> 2.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to maj...@vg...
> More majordomo info at http://vger.kernel.org/majordomo-info.html
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 12:43:32
|
Add handling of Huion tablets with UC-Logic vendor ID, but the same
product ID to hid-huion driver. A Huion H580 was seen using it.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-huion.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index e6f5338..1006c2a 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -247,6 +247,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
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);
--
2.0.1
|
|
From: Benjamin T. <ben...@gm...> - 2014-07-23 14:44:07
|
On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <sp...@gm...> wrote:
> Add handling of Huion tablets with UC-Logic vendor ID, but the same
> product ID to hid-huion driver. A Huion H580 was seen using it.
>
> Signed-off-by: Nikolai Kondrashov <sp...@gm...>
> ---
Reviewed-by: Benjamin Tissoires <ben...@re...>
> drivers/hid/hid-huion.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
> index e6f5338..1006c2a 100644
> --- a/drivers/hid/hid-huion.c
> +++ b/drivers/hid/hid-huion.c
> @@ -247,6 +247,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
>
> 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);
> --
> 2.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to maj...@vg...
> More majordomo info at http://vger.kernel.org/majordomo-info.html
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 12:43:34
|
Switch to generating tablet pen report descriptor from a template and
parameters retrieved from string descriptor 0x64.
Signed-off-by: Nikolai Kondrashov <sp...@gm...>
---
drivers/hid/hid-huion.c | 245 +++++++++++++++++++++++++++++++++---------------
1 file changed, 167 insertions(+), 78 deletions(-)
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index e5f1e22..e6f5338 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -16,67 +16,89 @@
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/usb.h>
+#include <asm/unaligned.h>
#include "usbhid/usbhid.h"
#include "hid-ids.h"
-/* Original tablet report descriptor size */
-#define HUION_TABLET_RDESC_ORIG_SIZE 177
-
-/* Fixed tablet report descriptor */
-static __u8 huion_tablet_rdesc_fixed[] = {
- 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), */
- 0x46, 0x40, 0x1F, /* Physical Maximum (8000), */
- 0x26, 0x00, 0x7D, /* Logical Maximum (32000), */
- 0x81, 0x02, /* Input (Variable), */
- 0x09, 0x31, /* Usage (Y), */
- 0x46, 0x88, 0x13, /* Physical Maximum (5000), */
- 0x26, 0x20, 0x4E, /* Logical Maximum (20000), */
- 0x81, 0x02, /* Input (Variable), */
- 0xB4, /* Pop, */
- 0x09, 0x30, /* Usage (Tip Pressure), */
- 0x26, 0xFF, 0x07, /* Logical Maximum (2047), */
- 0x81, 0x02, /* Input (Variable), */
- 0xC0, /* End Collection, */
- 0xC0 /* End Collection */
+/* 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 */
+};
+
+/* 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 (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
- rdesc = huion_tablet_rdesc_fixed;
- *rsize = sizeof(huion_tablet_rdesc_fixed);
+ if (drvdata->rdesc != NULL) {
+ rdesc = drvdata->rdesc;
+ *rsize = drvdata->rsize;
}
break;
}
@@ -84,57 +106,124 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
}
/**
- * Enable fully-functional tablet mode by reading special string
- * descriptor.
+ * Enable fully-functional tablet mode and determine device parameters.
*
* @hdev: HID device
- *
- * The specific string descriptor and data were discovered by sniffing
- * the Windows driver traffic.
*/
static int huion_tablet_enable(struct hid_device *hdev)
{
int rc;
- char buf[22];
+ struct usb_device *usb_dev = hid_to_usb_dev(hdev);
+ struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 buf[6];
- rc = usb_string(hid_to_usb_dev(hdev), 0x64, buf, sizeof(buf));
- if (rc < 0)
- return rc;
+ /*
+ * 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.
+ */
+ 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, sizeof(buf),
+ USB_CTRL_GET_TIMEOUT);
+ if (rc == -EPIPE)
+ hid_warn(hdev, "device parameters not found\n");
+ else if (rc < 0)
+ hid_warn(hdev, "failed to get device parameters: %d\n", rc);
+ else if (rc != sizeof(buf))
+ hid_warn(hdev, "invalid device parameters\n");
+ else {
+ s32 params[HUION_PH_ID_NUM];
+ s32 resolution;
+ __u8 *p;
+ s32 v;
+
+ /* Extract device parameters */
+ params[HUION_PH_ID_X_LM] = le16_to_cpu(buf[1]);
+ params[HUION_PH_ID_Y_LM] = le16_to_cpu(buf[2]);
+ params[HUION_PH_ID_PRESSURE_LM] = le16_to_cpu(buf[4]);
+ resolution = le16_to_cpu(buf[5]);
+ 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");
+ return -ENOMEM;
+ }
+ 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++;
+ }
+ }
+ }
return 0;
}
static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
- int ret;
-
- ret = hid_parse(hdev);
- if (ret) {
- hid_err(hdev, "parse failed\n");
- goto err;
- }
+ int rc;
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct huion_drvdata *drvdata;
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret) {
- hid_err(hdev, "hw start failed\n");
- goto err;
+ /* 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:
- ret = huion_tablet_enable(hdev);
- if (ret) {
- hid_err(hdev, "tablet enabling failed\n");
- goto enabling_err;
+ /* 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;
-enabling_err:
- hid_hw_stop(hdev);
-err:
- return ret;
}
static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
--
2.0.1
|
|
From: Benjamin T. <ben...@gm...> - 2014-07-23 14:43:07
|
On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <sp...@gm...> wrote:
> Switch to generating tablet pen report descriptor from a template and
> parameters retrieved from string descriptor 0x64.
>
> Signed-off-by: Nikolai Kondrashov <sp...@gm...>
> ---
This is a nice solution.
I still have few remarks/questions inlined:
> drivers/hid/hid-huion.c | 245 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 167 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
> index e5f1e22..e6f5338 100644
> --- a/drivers/hid/hid-huion.c
> +++ b/drivers/hid/hid-huion.c
> @@ -16,67 +16,89 @@
> #include <linux/hid.h>
> #include <linux/module.h>
> #include <linux/usb.h>
> +#include <asm/unaligned.h>
> #include "usbhid/usbhid.h"
>
> #include "hid-ids.h"
>
> -/* Original tablet report descriptor size */
> -#define HUION_TABLET_RDESC_ORIG_SIZE 177
> -
> -/* Fixed tablet report descriptor */
> -static __u8 huion_tablet_rdesc_fixed[] = {
> - 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), */
> - 0x46, 0x40, 0x1F, /* Physical Maximum (8000), */
> - 0x26, 0x00, 0x7D, /* Logical Maximum (32000), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0x88, 0x13, /* Physical Maximum (5000), */
> - 0x26, 0x20, 0x4E, /* Logical Maximum (20000), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xB4, /* Pop, */
> - 0x09, 0x30, /* Usage (Tip Pressure), */
> - 0x26, 0xFF, 0x07, /* Logical Maximum (2047), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xC0, /* End Collection, */
> - 0xC0 /* End Collection */
> +/* 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 */
> +};
> +
> +/* 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 (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
> - rdesc = huion_tablet_rdesc_fixed;
> - *rsize = sizeof(huion_tablet_rdesc_fixed);
> + if (drvdata->rdesc != NULL) {
> + rdesc = drvdata->rdesc;
> + *rsize = drvdata->rsize;
> }
> break;
> }
> @@ -84,57 +106,124 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> }
>
> /**
> - * Enable fully-functional tablet mode by reading special string
> - * descriptor.
> + * Enable fully-functional tablet mode and determine device parameters.
> *
> * @hdev: HID device
> - *
> - * The specific string descriptor and data were discovered by sniffing
> - * the Windows driver traffic.
> */
> static int huion_tablet_enable(struct hid_device *hdev)
> {
> int rc;
> - char buf[22];
> + struct usb_device *usb_dev = hid_to_usb_dev(hdev);
> + struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
> + u16 buf[6];
>
> - rc = usb_string(hid_to_usb_dev(hdev), 0x64, buf, sizeof(buf));
> - if (rc < 0)
> - return rc;
> + /*
> + * 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.
> + */
> + 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, sizeof(buf),
> + USB_CTRL_GET_TIMEOUT);
Just out of curiosity, you are replacing usb_string() by
usb_control_msg(). They both seem to do the same, so why bother
changing the call?
> + if (rc == -EPIPE)
> + hid_warn(hdev, "device parameters not found\n");
> + else if (rc < 0)
> + hid_warn(hdev, "failed to get device parameters: %d\n", rc);
> + else if (rc != sizeof(buf))
> + hid_warn(hdev, "invalid device parameters\n");
> + else {
> + s32 params[HUION_PH_ID_NUM];
> + s32 resolution;
> + __u8 *p;
> + s32 v;
> +
> + /* Extract device parameters */
> + params[HUION_PH_ID_X_LM] = le16_to_cpu(buf[1]);
> + params[HUION_PH_ID_Y_LM] = le16_to_cpu(buf[2]);
> + params[HUION_PH_ID_PRESSURE_LM] = le16_to_cpu(buf[4]);
> + resolution = le16_to_cpu(buf[5]);
> + if (resolution == 0) {
> + params[HUION_PH_ID_X_PM] = 0;
> + params[HUION_PH_ID_Y_PM] = 0;
This is not good (not your code I mean). I know OEMs tend to not care
about resolution, but Wayland will for the tablet protocol. The idea
will be to report the coordinate in mm so that we can have a constant
reporting across vendors/products.
Did you ever fall in this case? and if so, isn't there any way of
retrieving the actual resolution or an approximation?
> + } 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");
> + return -ENOMEM;
> + }
> + 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++;
> + }
> + }
> + }
>
> return 0;
> }
>
> static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
> {
> - int ret;
> -
> - ret = hid_parse(hdev);
> - if (ret) {
> - hid_err(hdev, "parse failed\n");
> - goto err;
> - }
> + int rc;
> + struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> + struct huion_drvdata *drvdata;
>
> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> - if (ret) {
> - hid_err(hdev, "hw start failed\n");
> - goto err;
> + /* 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:
> - ret = huion_tablet_enable(hdev);
> - if (ret) {
> - hid_err(hdev, "tablet enabling failed\n");
> - goto enabling_err;
> + /* 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;
> -enabling_err:
> - hid_hw_stop(hdev);
> -err:
> - return ret;
> }
>
> static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
> --
> 2.0.1
>
The rest looks fair enough, so even with my questions:
Reviewed-by: Benjamin Tissoires <ben...@re...>
Cheers,
Benjamin
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 15:00:07
|
On 07/23/2014 05:42 PM, Benjamin Tissoires wrote:
> On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <sp...@gm...> wrote:
>> Switch to generating tablet pen report descriptor from a template and
>> parameters retrieved from string descriptor 0x64.
>>
>> Signed-off-by: Nikolai Kondrashov <sp...@gm...>
>> ---
>
> This is a nice solution.
Thanks, Benjamin :)
>> - rc = usb_string(hid_to_usb_dev(hdev), 0x64, buf, sizeof(buf));
>> - if (rc < 0)
>> - return rc;
>> + /*
>> + * 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.
>> + */
>> + 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, sizeof(buf),
>> + USB_CTRL_GET_TIMEOUT);
>
> Just out of curiosity, you are replacing usb_string() by
> usb_control_msg(). They both seem to do the same, so why bother
> changing the call?
Well, actually they don't seem to do the same and the main difference is that
usb_string attempts to convert the data from UTF-16LE to UTF-8, which would be
undesirable for the binary contents we expect there.
>> + resolution = le16_to_cpu(buf[5]);
>> + if (resolution == 0) {
>> + params[HUION_PH_ID_X_PM] = 0;
>> + params[HUION_PH_ID_Y_PM] = 0;
>
> This is not good (not your code I mean). I know OEMs tend to not care
> about resolution, but Wayland will for the tablet protocol. The idea
> will be to report the coordinate in mm so that we can have a constant
> reporting across vendors/products.
>
> Did you ever fall in this case? and if so, isn't there any way of
> retrieving the actual resolution or an approximation?
Thankfully not. This is merely a protection against division-by-zero
exception in case they start doing that.
BTW, it's nice that Wayland tries to actually use the resolution.
> The rest looks fair enough, so even with my questions:
> Reviewed-by: Benjamin Tissoires <ben...@re...>
Thanks for the review, Benjamin!
Will send a new version soon.
Sincerely,
Nick
|
|
From: Nikolai K. <sp...@gm...> - 2014-07-23 13:39:53
|
On 07/23/2014 03:42 PM, Nikolai Kondrashov wrote: > The Huion tablets seem to all use a single product ID and the iProduct strings > are inconsistent. So there isn't much to identify the models with so far, > although the magic string descriptor containing tablet parameters has some > internal model name which might be used to identify them in the future. A minor correction for the record: the internal model is contained in a string descriptor separate from the one carrying the device parameters. See https://github.com/DIGImend/huion-tools for details. Nick |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X