* [PATCHES] hid: Add support for more Huion tablets
@ 2014-07-23 12:42 Nikolai Kondrashov
2014-07-23 12:42 ` [PATCH 1/5] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov
` (5 more replies)
0 siblings, 6 replies; 24+ messages in thread
From: Nikolai Kondrashov @ 2014-07-23 12:42 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, DIGImend-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/5] hid: huion: Use "tablet" instead of specific model 2014-07-23 12:42 [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov @ 2014-07-23 12:42 ` Nikolai Kondrashov 2014-07-23 14:30 ` Benjamin Tissoires 2014-07-23 12:42 ` [PATCH 2/5] hid: huion: Invert in-range on specific product Nikolai Kondrashov ` (4 subsequent siblings) 5 siblings, 1 reply; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 12:42 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/5] hid: huion: Use "tablet" instead of specific model 2014-07-23 12:42 ` [PATCH 1/5] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov @ 2014-07-23 14:30 ` Benjamin Tissoires 0 siblings, 0 replies; 24+ messages in thread From: Benjamin Tissoires @ 2014-07-23 14:30 UTC (permalink / raw) To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> 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 <spbnick@gmail.com> > --- Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cheers, Benjamin ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/5] hid: huion: Invert in-range on specific product 2014-07-23 12:42 [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov 2014-07-23 12:42 ` [PATCH 1/5] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov @ 2014-07-23 12:42 ` Nikolai Kondrashov 2014-07-23 14:34 ` Benjamin Tissoires 2014-07-23 12:42 ` [PATCH 3/5] hid: huion: Don't ignore other interfaces Nikolai Kondrashov ` (3 subsequent siblings) 5 siblings, 1 reply; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 12:42 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/5] hid: huion: Invert in-range on specific product 2014-07-23 12:42 ` [PATCH 2/5] hid: huion: Invert in-range on specific product Nikolai Kondrashov @ 2014-07-23 14:34 ` Benjamin Tissoires 2014-07-23 14:40 ` Nikolai Kondrashov 0 siblings, 1 reply; 24+ messages in thread From: Benjamin Tissoires @ 2014-07-23 14:34 UTC (permalink / raw) To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> 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 <spbnick@gmail.com> > --- 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/5] hid: huion: Invert in-range on specific product 2014-07-23 14:34 ` Benjamin Tissoires @ 2014-07-23 14:40 ` Nikolai Kondrashov 2014-07-23 16:31 ` [PATCHES v2] Add support for more Huion tablets Nikolai Kondrashov 0 siblings, 1 reply; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 14:40 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, DIGImend-devel On 07/23/2014 05:34 PM, Benjamin Tissoires wrote: > On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> 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 <spbnick@gmail.com> >> --- > > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHES v2] Add support for more Huion tablets 2014-07-23 14:40 ` Nikolai Kondrashov @ 2014-07-23 16:31 ` Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 1/4] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov ` (4 more replies) 0 siblings, 5 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] hid: huion: Use "tablet" instead of specific model 2014-07-23 16:31 ` [PATCHES v2] Add support for more Huion tablets Nikolai Kondrashov @ 2014-07-23 16:31 ` Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 2/4] hid: huion: Don't ignore other interfaces Nikolai Kondrashov ` (3 subsequent siblings) 4 siblings, 0 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] hid: huion: Don't ignore other interfaces 2014-07-23 16:31 ` [PATCHES v2] Add support for more Huion tablets Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 1/4] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov @ 2014-07-23 16:31 ` Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 3/4] hid: huion: Switch to generating report descriptor Nikolai Kondrashov ` (2 subsequent siblings) 4 siblings, 0 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] hid: huion: Switch to generating report descriptor 2014-07-23 16:31 ` [PATCHES v2] Add support for more Huion tablets Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 1/4] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 2/4] hid: huion: Don't ignore other interfaces Nikolai Kondrashov @ 2014-07-23 16:31 ` Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID Nikolai Kondrashov 2014-07-28 15:33 ` [PATCHES v2] Add support for more Huion tablets Benjamin Tissoires 4 siblings, 0 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov Switch to generating tablet pen report descriptor from a template and parameters retrieved from string descriptor 0x64. Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID 2014-07-23 16:31 ` [PATCHES v2] Add support for more Huion tablets Nikolai Kondrashov ` (2 preceding siblings ...) 2014-07-23 16:31 ` [PATCH 3/4] hid: huion: Switch to generating report descriptor Nikolai Kondrashov @ 2014-07-23 16:31 ` Nikolai Kondrashov 2014-07-28 15:33 ` [PATCHES v2] Add support for more Huion tablets Benjamin Tissoires 4 siblings, 0 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCHES v2] Add support for more Huion tablets 2014-07-23 16:31 ` [PATCHES v2] Add support for more Huion tablets Nikolai Kondrashov ` (3 preceding siblings ...) 2014-07-23 16:31 ` [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID Nikolai Kondrashov @ 2014-07-28 15:33 ` Benjamin Tissoires 2014-07-29 9:22 ` Jiri Kosina 4 siblings, 1 reply; 24+ messages in thread From: Benjamin Tissoires @ 2014-07-28 15:33 UTC (permalink / raw) To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel On Wed, Jul 23, 2014 at 12:31 PM, Nikolai Kondrashov <spbnick@gmail.com> 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 <benjamin.tissoires@redhat.com> Cheers, Benjamin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHES v2] Add support for more Huion tablets 2014-07-28 15:33 ` [PATCHES v2] Add support for more Huion tablets Benjamin Tissoires @ 2014-07-29 9:22 ` Jiri Kosina 2014-07-29 12:50 ` [PATCH] hid: huion: Fix sparse warnings Nikolai Kondrashov 0 siblings, 1 reply; 24+ messages in thread From: Jiri Kosina @ 2014-07-29 9:22 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: Nikolai Kondrashov, linux-input, DIGImend-devel 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 <benjamin.tissoires@redhat.com> Queuing for 3.17, thanks. -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] hid: huion: Fix sparse warnings 2014-07-29 9:22 ` Jiri Kosina @ 2014-07-29 12:50 ` Nikolai Kondrashov 2014-07-29 13:06 ` Jiri Kosina 0 siblings, 1 reply; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-29 12:50 UTC (permalink / raw) To: Jiri Kosina Cc: linux-input, DIGImend-devel, Benjamin Tissoires, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] hid: huion: Fix sparse warnings 2014-07-29 12:50 ` [PATCH] hid: huion: Fix sparse warnings Nikolai Kondrashov @ 2014-07-29 13:06 ` Jiri Kosina 2014-07-29 13:24 ` Nikolai Kondrashov 0 siblings, 1 reply; 24+ messages in thread From: Jiri Kosina @ 2014-07-29 13:06 UTC (permalink / raw) To: Nikolai Kondrashov; +Cc: linux-input, DIGImend-devel, Benjamin Tissoires 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 <spbnick@gmail.com> > --- > > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] hid: huion: Fix sparse warnings 2014-07-29 13:06 ` Jiri Kosina @ 2014-07-29 13:24 ` Nikolai Kondrashov 0 siblings, 0 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-29 13:24 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Benjamin Tissoires 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/5] hid: huion: Don't ignore other interfaces 2014-07-23 12:42 [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov 2014-07-23 12:42 ` [PATCH 1/5] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov 2014-07-23 12:42 ` [PATCH 2/5] hid: huion: Invert in-range on specific product Nikolai Kondrashov @ 2014-07-23 12:42 ` Nikolai Kondrashov 2014-07-23 14:43 ` Benjamin Tissoires 2014-07-23 12:42 ` [PATCH 4/5] hid: huion: Switch to generating report descriptor Nikolai Kondrashov ` (2 subsequent siblings) 5 siblings, 1 reply; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 12:42 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/5] hid: huion: Don't ignore other interfaces 2014-07-23 12:42 ` [PATCH 3/5] hid: huion: Don't ignore other interfaces Nikolai Kondrashov @ 2014-07-23 14:43 ` Benjamin Tissoires 0 siblings, 0 replies; 24+ messages in thread From: Benjamin Tissoires @ 2014-07-23 14:43 UTC (permalink / raw) To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> 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 <spbnick@gmail.com> > --- Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/5] hid: huion: Switch to generating report descriptor 2014-07-23 12:42 [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov ` (2 preceding siblings ...) 2014-07-23 12:42 ` [PATCH 3/5] hid: huion: Don't ignore other interfaces Nikolai Kondrashov @ 2014-07-23 12:42 ` Nikolai Kondrashov 2014-07-23 14:42 ` Benjamin Tissoires 2014-07-23 12:42 ` [PATCH 5/5] hid: huion: Handle tablets with UC-Logic vendor ID Nikolai Kondrashov 2014-07-23 13:39 ` [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov 5 siblings, 1 reply; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 12:42 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov Switch to generating tablet pen report descriptor from a template and parameters retrieved from string descriptor 0x64. Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 4/5] hid: huion: Switch to generating report descriptor 2014-07-23 12:42 ` [PATCH 4/5] hid: huion: Switch to generating report descriptor Nikolai Kondrashov @ 2014-07-23 14:42 ` Benjamin Tissoires 2014-07-23 14:59 ` Nikolai Kondrashov 0 siblings, 1 reply; 24+ messages in thread From: Benjamin Tissoires @ 2014-07-23 14:42 UTC (permalink / raw) To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote: > Switch to generating tablet pen report descriptor from a template and > parameters retrieved from string descriptor 0x64. > > Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com> > --- 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 <benjamin.tissoires@redhat.com> Cheers, Benjamin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/5] hid: huion: Switch to generating report descriptor 2014-07-23 14:42 ` Benjamin Tissoires @ 2014-07-23 14:59 ` Nikolai Kondrashov 0 siblings, 0 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 14:59 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, DIGImend-devel On 07/23/2014 05:42 PM, Benjamin Tissoires wrote: > On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote: >> Switch to generating tablet pen report descriptor from a template and >> parameters retrieved from string descriptor 0x64. >> >> Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com> >> --- > > 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 <benjamin.tissoires@redhat.com> Thanks for the review, Benjamin! Will send a new version soon. Sincerely, Nick ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/5] hid: huion: Handle tablets with UC-Logic vendor ID 2014-07-23 12:42 [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov ` (3 preceding siblings ...) 2014-07-23 12:42 ` [PATCH 4/5] hid: huion: Switch to generating report descriptor Nikolai Kondrashov @ 2014-07-23 12:42 ` Nikolai Kondrashov 2014-07-23 14:43 ` Benjamin Tissoires 2014-07-23 13:39 ` [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov 5 siblings, 1 reply; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 12:42 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov 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 <spbnick@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 5/5] hid: huion: Handle tablets with UC-Logic vendor ID 2014-07-23 12:42 ` [PATCH 5/5] hid: huion: Handle tablets with UC-Logic vendor ID Nikolai Kondrashov @ 2014-07-23 14:43 ` Benjamin Tissoires 0 siblings, 0 replies; 24+ messages in thread From: Benjamin Tissoires @ 2014-07-23 14:43 UTC (permalink / raw) To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> 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 <spbnick@gmail.com> > --- Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHES] hid: Add support for more Huion tablets 2014-07-23 12:42 [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov ` (4 preceding siblings ...) 2014-07-23 12:42 ` [PATCH 5/5] hid: huion: Handle tablets with UC-Logic vendor ID Nikolai Kondrashov @ 2014-07-23 13:39 ` Nikolai Kondrashov 5 siblings, 0 replies; 24+ messages in thread From: Nikolai Kondrashov @ 2014-07-23 13:39 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, DIGImend-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-07-29 13:24 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-23 12:42 [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov 2014-07-23 12:42 ` [PATCH 1/5] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov 2014-07-23 14:30 ` Benjamin Tissoires 2014-07-23 12:42 ` [PATCH 2/5] hid: huion: Invert in-range on specific product Nikolai Kondrashov 2014-07-23 14:34 ` Benjamin Tissoires 2014-07-23 14:40 ` Nikolai Kondrashov 2014-07-23 16:31 ` [PATCHES v2] Add support for more Huion tablets Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 1/4] hid: huion: Use "tablet" instead of specific model Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 2/4] hid: huion: Don't ignore other interfaces Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 3/4] hid: huion: Switch to generating report descriptor Nikolai Kondrashov 2014-07-23 16:31 ` [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID Nikolai Kondrashov 2014-07-28 15:33 ` [PATCHES v2] Add support for more Huion tablets Benjamin Tissoires 2014-07-29 9:22 ` Jiri Kosina 2014-07-29 12:50 ` [PATCH] hid: huion: Fix sparse warnings Nikolai Kondrashov 2014-07-29 13:06 ` Jiri Kosina 2014-07-29 13:24 ` Nikolai Kondrashov 2014-07-23 12:42 ` [PATCH 3/5] hid: huion: Don't ignore other interfaces Nikolai Kondrashov 2014-07-23 14:43 ` Benjamin Tissoires 2014-07-23 12:42 ` [PATCH 4/5] hid: huion: Switch to generating report descriptor Nikolai Kondrashov 2014-07-23 14:42 ` Benjamin Tissoires 2014-07-23 14:59 ` Nikolai Kondrashov 2014-07-23 12:42 ` [PATCH 5/5] hid: huion: Handle tablets with UC-Logic vendor ID Nikolai Kondrashov 2014-07-23 14:43 ` Benjamin Tissoires 2014-07-23 13:39 ` [PATCHES] hid: Add support for more Huion tablets Nikolai Kondrashov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).