Thread: [DIGImend-devel] [PATCH 1/2] uclogic: make input_mapping independent of usb
Brought to you by:
spb_nick
|
From: Benjamin T. <ben...@re...> - 2015-03-13 21:28:41
|
No need to retrieve the USB handle in input_mapping() when we already do that in probe. It also allows to use the quirk without having to add the product ID matching. Sent upstream as: https://patchwork.kernel.org/patch/6006271/ --- hid-uclogic.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/hid-uclogic.c b/hid-uclogic.c index 51333fa..88c0914 100644 --- a/hid-uclogic.c +++ b/hid-uclogic.c @@ -627,6 +627,7 @@ struct uclogic_drvdata { __u8 *rdesc; unsigned int rsize; bool invert_pen_inrange; + bool ignore_pen_interface; }; static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc, @@ -719,16 +720,12 @@ static int uclogic_input_mapping(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) { - struct usb_interface *intf; - - if (hdev->product == USB_DEVICE_ID_HUION_TABLET) { - intf = to_usb_interface(hdev->dev.parent); + struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev); - /* discard the unused pen interface */ - if ((intf->cur_altsetting->desc.bInterfaceNumber != 0) && - (field->application == HID_DG_PEN)) - return -1; - } + /* discard the unused pen interface */ + if ((drvdata->ignore_pen_interface) && + (field->application == HID_DG_PEN)) + return -1; /* let hid-core decide what to do */ return 0; @@ -910,6 +907,8 @@ static int uclogic_probe(struct hid_device *hdev, return rc; } drvdata->invert_pen_inrange = true; + } else { + drvdata->ignore_pen_interface = true; } break; case USB_DEVICE_ID_UCLOGIC_TABLET_TWHA60: -- 2.3.1 |
|
From: Benjamin T. <ben...@re...> - 2015-03-13 21:28:41
|
Based on a patch from: Nikolai Kondrashov <Nik...@re...>
Enable abstract keyboard mode for Huion tablets, which makes them report
frame buttons using the pen interface and report ID. Divert these
reports to a virtual report ID describing them.
This makes the tablet compatible with xf86-input-wacom and libinput,
but stops the frame buttons from reporting keyboard events.
---
Sorry it took so long to actually propose this patch. I am fixing at the moment
gnome-control-center, gnome-settings-daemon and libwacom to be able to handle
also the generic devices like the Huion ones.
I need to go through all the Huion known device and see what they answer when
querying the string 0x7b, but I believe checking if "HK On" is returned should
allow us to prevent activating a pad interface if the device does not support
it.
I am quite tempted to send this one upstream directly too. So please, test it
and report if there are obvious problems.
Cheers,
Benjamin
hid-uclogic.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 98 insertions(+), 5 deletions(-)
diff --git a/hid-uclogic.c b/hid-uclogic.c
index 88c0914..ea57d9f 100644
--- a/hid-uclogic.c
+++ b/hid-uclogic.c
@@ -613,6 +613,27 @@ static const __u8 uclogic_tablet_rdesc_template[] = {
0xC0 /* End Collection */
};
+/* Fixed report descriptor template */
+static const __u8 uclogic_buttonpad_rdesc_template[] = {
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x09, 0x07, /* Usage (Keypad), */
+ 0xA1, 0x01, /* Collection (Application), */
+ 0x85, 0xF7, /* Report ID (247), */
+ 0x05, 0x0D, /* Usage Page (Digitizers), */
+ 0x09, 0x39, /* Usage (Tablet Function Keys), */
+ 0xA0, /* Collection (Physical), */
+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x75, 0x01, /* Report Size (1), */
+ 0x95, 0x18, /* Report Count (24), */
+ 0x81, 0x03, /* Input (Constant, Variable), */
+ 0x19, 0x01, /* Usage Minimum (01h), */
+ 0x29, 0x08, /* Usage Maximum (08h), */
+ 0x95, 0x08, /* Report Count (8), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0xC0, /* End Collection */
+ 0xC0 /* End Collection */
+};
+
/* Parameter indices */
enum uclogic_prm {
UCLOGIC_PRM_X_LM = 1,
@@ -628,6 +649,7 @@ struct uclogic_drvdata {
unsigned int rsize;
bool invert_pen_inrange;
bool ignore_pen_interface;
+ bool has_virtual_pad_interface;
};
static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -874,6 +896,70 @@ cleanup:
return rc;
}
+/**
+ * Enable actual button mode.
+ *
+ * @hdev: HID device
+ */
+static int uclogic_button_enable(struct hid_device *hdev)
+{
+ int rc;
+ struct usb_device *usb_dev = hid_to_usb_dev(hdev);
+ struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
+ char *str_buf;
+ size_t str_len = 16;
+ unsigned char *rdesc;
+ size_t rdesc_len;
+
+ str_buf = kzalloc(str_len, GFP_KERNEL);
+ if (str_buf == NULL) {
+ hid_err(hdev, "failed to allocate string buffer\n");
+ rc = -ENOMEM;
+ goto cleanup;
+ }
+
+ /* Enable abstract keyboard mode */
+ rc = usb_string(usb_dev, 0x7b, str_buf, str_len);
+ if (rc == -EPIPE) {
+ hid_info(hdev, "button mode setting not found\n");
+ rc = 0;
+ goto cleanup;
+ } else if (rc < 0) {
+ hid_err(hdev, "failed to enable abstract keyboard\n");
+ goto cleanup;
+ } else if (strncmp(str_buf, "HK On", rc)) {
+ hid_info(hdev, "invalid answer when requesting buttons: '%s'\n",
+ str_buf);
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+ /* Re-allocate fixed report descriptor */
+ rdesc_len = drvdata->rsize + sizeof(uclogic_buttonpad_rdesc_template);
+ rdesc = devm_kzalloc(&hdev->dev, rdesc_len, GFP_KERNEL);
+ if (!rdesc) {
+ rc = -ENOMEM;
+ goto cleanup;
+ }
+
+ memcpy(rdesc, drvdata->rdesc, drvdata->rsize);
+
+ /* Append the buttonpad descriptor */
+ memcpy(rdesc + drvdata->rsize, uclogic_buttonpad_rdesc_template,
+ sizeof(uclogic_buttonpad_rdesc_template));
+
+ /* clean up old rdesc and use the new one */
+ drvdata->rsize = rdesc_len;
+ devm_kfree(&hdev->dev, drvdata->rdesc);
+ drvdata->rdesc = rdesc;
+
+ rc = 0;
+
+cleanup:
+ kfree(str_buf);
+ return rc;
+}
+
static int uclogic_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -907,6 +993,9 @@ static int uclogic_probe(struct hid_device *hdev,
return rc;
}
drvdata->invert_pen_inrange = true;
+
+ rc = uclogic_button_enable(hdev);
+ drvdata->has_virtual_pad_interface = !rc;
} else {
drvdata->ignore_pen_interface = true;
}
@@ -947,12 +1036,16 @@ static int uclogic_raw_event(struct hid_device *hdev, struct hid_report *report,
{
struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
- if ((drvdata->invert_pen_inrange) &&
- (report->type == HID_INPUT_REPORT) &&
+ if ((report->type == HID_INPUT_REPORT) &&
(report->id == UCLOGIC_PEN_REPORT_ID) &&
- (size >= 2))
- /* Invert the in-range bit */
- data[1] ^= 0x40;
+ (size >= 2)) {
+ if (drvdata->has_virtual_pad_interface && (data[1] & 0x20))
+ /* Change to virtual frame button report ID */
+ data[0] = 0xf7;
+ else if (drvdata->invert_pen_inrange)
+ /* Invert the in-range bit */
+ data[1] ^= 0x40;
+ }
return 0;
}
--
2.3.1
|
|
From: Nikolai K. <sp...@gm...> - 2015-03-15 15:29:10
|
On 03/13/2015 11:28 PM, Benjamin Tissoires wrote: > No need to retrieve the USB handle in input_mapping() when we already > do that in probe. It also allows to use the quirk without having to > add the product ID matching. > > Sent upstream as: https://patchwork.kernel.org/patch/6006271/ Thanks, Benjamin. I'll wait for your answer to my comment upstream. Nick |
|
From: Nikolai K. <sp...@gm...> - 2015-03-15 16:02:46
|
On 03/13/2015 11:28 PM, Benjamin Tissoires wrote:
> Based on a patch from: Nikolai Kondrashov <Nik...@re...>
>
> Enable abstract keyboard mode for Huion tablets, which makes them report
> frame buttons using the pen interface and report ID. Divert these
> reports to a virtual report ID describing them.
>
> This makes the tablet compatible with xf86-input-wacom and libinput,
> but stops the frame buttons from reporting keyboard events.
> ---
>
> Sorry it took so long to actually propose this patch. I am fixing at the moment
> gnome-control-center, gnome-settings-daemon and libwacom to be able to handle
> also the generic devices like the Huion ones.
This is excellent news which we'll be welcome by all users, I'm sure.
> I need to go through all the Huion known device and see what they answer when
> querying the string 0x7b, but I believe checking if "HK On" is returned should
> allow us to prevent activating a pad interface if the device does not support
> it.
We can add requesting that string to huion-tools:
https://github.com/DIGImend/huion-tools
And then ask people to run it and report results.
I should probably rename that to uclogic-tools, BTW.
> I am quite tempted to send this one upstream directly too. So please, test it
> and report if there are obvious problems.
I understand :) Once we get this merged and I clean up the general state and
update the README.md, I'll make a release and people will test it.
Nick
> Cheers,
> Benjamin
>
> hid-uclogic.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 98 insertions(+), 5 deletions(-)
>
> diff --git a/hid-uclogic.c b/hid-uclogic.c
> index 88c0914..ea57d9f 100644
> --- a/hid-uclogic.c
> +++ b/hid-uclogic.c
> @@ -613,6 +613,27 @@ static const __u8 uclogic_tablet_rdesc_template[] = {
> 0xC0 /* End Collection */
> };
>
> +/* Fixed report descriptor template */
> +static const __u8 uclogic_buttonpad_rdesc_template[] = {
This isn't a template, but just a descriptor. I think it is better to drop
"template" from here.
> + 0x05, 0x01, /* Usage Page (Desktop), */
> + 0x09, 0x07, /* Usage (Keypad), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0xF7, /* Report ID (247), */
> + 0x05, 0x0D, /* Usage Page (Digitizers), */
> + 0x09, 0x39, /* Usage (Tablet Function Keys), */
> + 0xA0, /* Collection (Physical), */
> + 0x05, 0x09, /* Usage Page (Button), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x18, /* Report Count (24), */
> + 0x81, 0x03, /* Input (Constant, Variable), */
> + 0x19, 0x01, /* Usage Minimum (01h), */
> + 0x29, 0x08, /* Usage Maximum (08h), */
> + 0x95, 0x08, /* Report Count (8), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0xC0, /* End Collection */
> + 0xC0 /* End Collection */
> +};
> +
> /* Parameter indices */
> enum uclogic_prm {
> UCLOGIC_PRM_X_LM = 1,
> @@ -628,6 +649,7 @@ struct uclogic_drvdata {
> unsigned int rsize;
> bool invert_pen_inrange;
> bool ignore_pen_interface;
> + bool has_virtual_pad_interface;
> };
>
> static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> @@ -874,6 +896,70 @@ cleanup:
> return rc;
> }
>
> +/**
> + * Enable actual button mode.
> + *
> + * @hdev: HID device
> + */
> +static int uclogic_button_enable(struct hid_device *hdev)
> +{
> + int rc;
> + struct usb_device *usb_dev = hid_to_usb_dev(hdev);
> + struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
> + char *str_buf;
> + size_t str_len = 16;
> + unsigned char *rdesc;
> + size_t rdesc_len;
> +
> + str_buf = kzalloc(str_len, GFP_KERNEL);
> + if (str_buf == NULL) {
> + hid_err(hdev, "failed to allocate string buffer\n");
> + rc = -ENOMEM;
> + goto cleanup;
> + }
> +
> + /* Enable abstract keyboard mode */
> + rc = usb_string(usb_dev, 0x7b, str_buf, str_len);
> + if (rc == -EPIPE) {
> + hid_info(hdev, "button mode setting not found\n");
> + rc = 0;
> + goto cleanup;
> + } else if (rc < 0) {
> + hid_err(hdev, "failed to enable abstract keyboard\n");
> + goto cleanup;
> + } else if (strncmp(str_buf, "HK On", rc)) {
> + hid_info(hdev, "invalid answer when requesting buttons: '%s'\n",
> + str_buf);
> + rc = -EINVAL;
> + goto cleanup;
> + }
> +
> + /* Re-allocate fixed report descriptor */
> + rdesc_len = drvdata->rsize + sizeof(uclogic_buttonpad_rdesc_template);
> + rdesc = devm_kzalloc(&hdev->dev, rdesc_len, GFP_KERNEL);
> + if (!rdesc) {
> + rc = -ENOMEM;
> + goto cleanup;
> + }
> +
> + memcpy(rdesc, drvdata->rdesc, drvdata->rsize);
> +
> + /* Append the buttonpad descriptor */
> + memcpy(rdesc + drvdata->rsize, uclogic_buttonpad_rdesc_template,
> + sizeof(uclogic_buttonpad_rdesc_template));
> +
> + /* clean up old rdesc and use the new one */
> + drvdata->rsize = rdesc_len;
> + devm_kfree(&hdev->dev, drvdata->rdesc);
> + drvdata->rdesc = rdesc;
> +
> + rc = 0;
> +
> +cleanup:
> + kfree(str_buf);
> + return rc;
> +}
> +
> static int uclogic_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> {
> @@ -907,6 +993,9 @@ static int uclogic_probe(struct hid_device *hdev,
> return rc;
> }
> drvdata->invert_pen_inrange = true;
> +
> + rc = uclogic_button_enable(hdev);
> + drvdata->has_virtual_pad_interface = !rc;
> } else {
> drvdata->ignore_pen_interface = true;
> }
I think the button enabling should also work on TWHA60 v3 handled in the
"case" just below. We can move it above and let it fall down into this when
bNumInterfaces == 3. I.e.:
case USB_DEVICE_ID_UCLOGIC_TABLET_TWHA60:
/*
* If it is not the pen interface of the three-interface
* version, which is known to respond to initialization.
*/
if (udev->config->desc.bNumInterfaces != 3)
break;
case USB_DEVICE_ID_HUION_TABLET:
case USB_DEVICE_ID_YIYNOVA_TABLET:
Another nice thing to do here is to somehow disable the keyboard interface
(#2), but I'm not sure how to do that best.
> @@ -947,12 +1036,16 @@ static int uclogic_raw_event(struct hid_device *hdev, struct hid_report *report,
> {
> struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
>
> - if ((drvdata->invert_pen_inrange) &&
> - (report->type == HID_INPUT_REPORT) &&
> + if ((report->type == HID_INPUT_REPORT) &&
> (report->id == UCLOGIC_PEN_REPORT_ID) &&
> - (size >= 2))
> - /* Invert the in-range bit */
> - data[1] ^= 0x40;
> + (size >= 2)) {
> + if (drvdata->has_virtual_pad_interface && (data[1] & 0x20))
> + /* Change to virtual frame button report ID */
> + data[0] = 0xf7;
> + else if (drvdata->invert_pen_inrange)
> + /* Invert the in-range bit */
> + data[1] ^= 0x40;
> + }
>
> return 0;
> }
This is neat.
Thanks, Benjamin!
Nick
|
|
From: Benjamin T. <bti...@re...> - 2015-03-15 17:34:12
|
----- Original Message ----- > From: "Nikolai Kondrashov" <sp...@gm...> > To: "Benjamin Tissoires" <ben...@re...>, "DIGImend-devel" <DIG...@li...> > Sent: Sunday, March 15, 2015 12:02:41 PM > Subject: Re: [DIGImend-devel] [PATCH 2/2] huion: Switch to reporting abstract button events > > On 03/13/2015 11:28 PM, Benjamin Tissoires wrote: > > Based on a patch from: Nikolai Kondrashov <Nik...@re...> > > > > Enable abstract keyboard mode for Huion tablets, which makes them report > > frame buttons using the pen interface and report ID. Divert these > > reports to a virtual report ID describing them. > > > > This makes the tablet compatible with xf86-input-wacom and libinput, > > but stops the frame buttons from reporting keyboard events. > > --- > > > > Sorry it took so long to actually propose this patch. I am fixing at the > > moment > > gnome-control-center, gnome-settings-daemon and libwacom to be able to > > handle > > also the generic devices like the Huion ones. > > This is excellent news which we'll be welcome by all users, I'm sure. Yeah. It turns out that there was a little bit of work to do to be able to support these tablets. Most of the problem came from the fact that the tablets don't have an eraser and gnome-control-center rejected such tablets. I think I'll make an announce here once I got the bugs fixed, but I think it should be safe to assume that gnome 3.16 will be able to handle the Huion tablets seamlessly. And I'll make sure Fedora 22 get all the patches. We will need to upgrade libwacom too, and I already started adding the Huion H610 Pro (with the button layout) to it: http://sourceforge.net/p/linuxwacom/mailman/linuxwacom-devel/thread/1426281693-12291-3-git-send-email-benjamin.tissoires%40redhat.com/#msg33595219 We will then have to add the various tablets we want to have a nice Button mapping gui to libwacom. If the entry is not in libwacom, the button mapping won't be configurable by the user, but the tablet will show up in g-c-c. > > > I need to go through all the Huion known device and see what they answer > > when > > querying the string 0x7b, but I believe checking if "HK On" is returned > > should > > allow us to prevent activating a pad interface if the device does not > > support > > it. > > We can add requesting that string to huion-tools: > > https://github.com/DIGImend/huion-tools > > And then ask people to run it and report results. > > I should probably rename that to uclogic-tools, BTW. > > > I am quite tempted to send this one upstream directly too. So please, test > > it > > and report if there are obvious problems. > > I understand :) Once we get this merged and I clean up the general state and > update the README.md, I'll make a release and people will test it. > > Nick > > > Cheers, > > Benjamin > > > > hid-uclogic.c | 103 > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 98 insertions(+), 5 deletions(-) > > > > diff --git a/hid-uclogic.c b/hid-uclogic.c > > index 88c0914..ea57d9f 100644 > > --- a/hid-uclogic.c > > +++ b/hid-uclogic.c > > @@ -613,6 +613,27 @@ static const __u8 uclogic_tablet_rdesc_template[] = { > > 0xC0 /* End Collection */ > > }; > > > > +/* Fixed report descriptor template */ > > +static const __u8 uclogic_buttonpad_rdesc_template[] = { > > This isn't a template, but just a descriptor. I think it is better to drop > "template" from here. Sure, will fix. > > > + 0x05, 0x01, /* Usage Page (Desktop), */ > > + 0x09, 0x07, /* Usage (Keypad), */ > > + 0xA1, 0x01, /* Collection (Application), */ > > + 0x85, 0xF7, /* Report ID (247), */ > > + 0x05, 0x0D, /* Usage Page (Digitizers), */ > > + 0x09, 0x39, /* Usage (Tablet Function Keys), */ > > + 0xA0, /* Collection (Physical), */ > > + 0x05, 0x09, /* Usage Page (Button), */ > > + 0x75, 0x01, /* Report Size (1), */ > > + 0x95, 0x18, /* Report Count (24), */ > > + 0x81, 0x03, /* Input (Constant, Variable), */ > > + 0x19, 0x01, /* Usage Minimum (01h), */ > > + 0x29, 0x08, /* Usage Maximum (08h), */ > > + 0x95, 0x08, /* Report Count (8), */ > > + 0x81, 0x02, /* Input (Variable), */ > > + 0xC0, /* End Collection */ > > + 0xC0 /* End Collection */ > > +}; > > + > > /* Parameter indices */ > > enum uclogic_prm { > > UCLOGIC_PRM_X_LM = 1, > > @@ -628,6 +649,7 @@ struct uclogic_drvdata { > > unsigned int rsize; > > bool invert_pen_inrange; > > bool ignore_pen_interface; > > + bool has_virtual_pad_interface; > > }; > > > > static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc, > > @@ -874,6 +896,70 @@ cleanup: > > return rc; > > } > > > > +/** > > + * Enable actual button mode. > > + * > > + * @hdev: HID device > > + */ > > +static int uclogic_button_enable(struct hid_device *hdev) > > +{ > > + int rc; > > + struct usb_device *usb_dev = hid_to_usb_dev(hdev); > > + struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev); > > + char *str_buf; > > + size_t str_len = 16; > > + unsigned char *rdesc; > > + size_t rdesc_len; > > + > > + str_buf = kzalloc(str_len, GFP_KERNEL); > > + if (str_buf == NULL) { > > + hid_err(hdev, "failed to allocate string buffer\n"); > > + rc = -ENOMEM; > > + goto cleanup; > > + } > > + > > + /* Enable abstract keyboard mode */ > > + rc = usb_string(usb_dev, 0x7b, str_buf, str_len); > > + if (rc == -EPIPE) { > > + hid_info(hdev, "button mode setting not found\n"); > > + rc = 0; > > + goto cleanup; > > + } else if (rc < 0) { > > + hid_err(hdev, "failed to enable abstract keyboard\n"); > > + goto cleanup; > > + } else if (strncmp(str_buf, "HK On", rc)) { > > + hid_info(hdev, "invalid answer when requesting buttons: '%s'\n", > > + str_buf); > > + rc = -EINVAL; > > + goto cleanup; > > + } > > + > > + /* Re-allocate fixed report descriptor */ > > + rdesc_len = drvdata->rsize + sizeof(uclogic_buttonpad_rdesc_template); > > + rdesc = devm_kzalloc(&hdev->dev, rdesc_len, GFP_KERNEL); > > + if (!rdesc) { > > + rc = -ENOMEM; > > + goto cleanup; > > + } > > + > > + memcpy(rdesc, drvdata->rdesc, drvdata->rsize); > > + > > + /* Append the buttonpad descriptor */ > > + memcpy(rdesc + drvdata->rsize, uclogic_buttonpad_rdesc_template, > > + sizeof(uclogic_buttonpad_rdesc_template)); > > + > > + /* clean up old rdesc and use the new one */ > > + drvdata->rsize = rdesc_len; > > + devm_kfree(&hdev->dev, drvdata->rdesc); > > + drvdata->rdesc = rdesc; > > + > > + rc = 0; > > + > > +cleanup: > > + kfree(str_buf); > > + return rc; > > +} > > + > > static int uclogic_probe(struct hid_device *hdev, > > const struct hid_device_id *id) > > { > > @@ -907,6 +993,9 @@ static int uclogic_probe(struct hid_device *hdev, > > return rc; > > } > > drvdata->invert_pen_inrange = true; > > + > > + rc = uclogic_button_enable(hdev); > > + drvdata->has_virtual_pad_interface = !rc; > > } else { > > drvdata->ignore_pen_interface = true; > > } > > I think the button enabling should also work on TWHA60 v3 handled in the > "case" just below. We can move it above and let it fall down into this when > bNumInterfaces == 3. I.e.: > > case USB_DEVICE_ID_UCLOGIC_TABLET_TWHA60: > /* > * If it is not the pen interface of the three-interface > * version, which is known to respond to initialization. > */ > if (udev->config->desc.bNumInterfaces != 3) > break; > > case USB_DEVICE_ID_HUION_TABLET: > case USB_DEVICE_ID_YIYNOVA_TABLET: True. But upstream doesn't have support of the TWHA60 v3. So I'd prefer keep the patch in that way, and then add an extra commit for TWHA60 v3. Then the final support of TWHA60 v3 can be merged upstream too. > > Another nice thing to do here is to somehow disable the keyboard interface > (#2), but I'm not sure how to do that best. Not sure either. With the proper names, we have a lot of extra input nodes, but the user can easily ignore them seeing that one is "Keyboard". If Huion did not reused the same VID/PID for every tablets, things would have been easier :( > > > > @@ -947,12 +1036,16 @@ static int uclogic_raw_event(struct hid_device > > *hdev, struct hid_report *report, > > { > > struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev); > > > > - if ((drvdata->invert_pen_inrange) && > > - (report->type == HID_INPUT_REPORT) && > > + if ((report->type == HID_INPUT_REPORT) && > > (report->id == UCLOGIC_PEN_REPORT_ID) && > > - (size >= 2)) > > - /* Invert the in-range bit */ > > - data[1] ^= 0x40; > > + (size >= 2)) { > > + if (drvdata->has_virtual_pad_interface && (data[1] & 0x20)) > > + /* Change to virtual frame button report ID */ > > + data[0] = 0xf7; > > + else if (drvdata->invert_pen_inrange) > > + /* Invert the in-range bit */ > > + data[1] ^= 0x40; > > + } > > > > return 0; > > } > > This is neat. > > Thanks, Benjamin! > You are welcome. Cheers, Benjamin |
|
From: Nikolai K. <sp...@gm...> - 2015-03-15 16:54:24
|
On 03/15/2015 06:02 PM, Nikolai Kondrashov wrote: > On 03/13/2015 11:28 PM, Benjamin Tissoires wrote: >> Based on a patch from: Nikolai Kondrashov <Nik...@re...> >> >> Enable abstract keyboard mode for Huion tablets, which makes them report >> frame buttons using the pen interface and report ID. Divert these >> reports to a virtual report ID describing them. >> >> This makes the tablet compatible with xf86-input-wacom and libinput, >> but stops the frame buttons from reporting keyboard events. >> --- >> >> Sorry it took so long to actually propose this patch. I am fixing at the moment >> gnome-control-center, gnome-settings-daemon and libwacom to be able to handle >> also the generic devices like the Huion ones. > > This is excellent news which we'll be welcome by all users, I'm sure. > >> I need to go through all the Huion known device and see what they answer when >> querying the string 0x7b, but I believe checking if "HK On" is returned should >> allow us to prevent activating a pad interface if the device does not support >> it. > > We can add requesting that string to huion-tools: > > https://github.com/DIGImend/huion-tools Added. > And then ask people to run it and report results. Can make a release, build packages and send the word out to people, if you think it's useful. Nick |
|
From: Benjamin T. <bti...@re...> - 2015-03-15 17:18:13
|
----- Original Message ----- > From: "Nikolai Kondrashov" <sp...@gm...> > To: "Benjamin Tissoires" <ben...@re...>, "DIGImend-devel" <DIG...@li...> > Sent: Sunday, March 15, 2015 12:54:19 PM > Subject: Re: [DIGImend-devel] [PATCH 2/2] huion: Switch to reporting abstract button events > > On 03/15/2015 06:02 PM, Nikolai Kondrashov wrote: > > On 03/13/2015 11:28 PM, Benjamin Tissoires wrote: > >> Based on a patch from: Nikolai Kondrashov <Nik...@re...> > >> > >> Enable abstract keyboard mode for Huion tablets, which makes them report > >> frame buttons using the pen interface and report ID. Divert these > >> reports to a virtual report ID describing them. > >> > >> This makes the tablet compatible with xf86-input-wacom and libinput, > >> but stops the frame buttons from reporting keyboard events. > >> --- > >> > >> Sorry it took so long to actually propose this patch. I am fixing at the > >> moment > >> gnome-control-center, gnome-settings-daemon and libwacom to be able to > >> handle > >> also the generic devices like the Huion ones. > > > > This is excellent news which we'll be welcome by all users, I'm sure. > > > >> I need to go through all the Huion known device and see what they answer > >> when > >> querying the string 0x7b, but I believe checking if "HK On" is returned > >> should > >> allow us to prevent activating a pad interface if the device does not > >> support > >> it. > > > > We can add requesting that string to huion-tools: > > > > https://github.com/DIGImend/huion-tools > > Added. > > > And then ask people to run it and report results. > > Can make a release, build packages and send the word out to people, if you > think it's useful. > I think it will definitively be useful. I just made a quick test, and I think you should also warn users that they need to unplug/replug the tablet after running the tool. It can switch the tablet in the button reporting mode and kill the current keyboard interface (and messed up the pen interface too). Cheers, Benjamin |
|
From: Nikolai K. <sp...@gm...> - 2015-03-15 20:07:23
|
On 03/15/2015 07:18 PM, Benjamin Tissoires wrote: > > > > > ----- Original Message ----- >> From: "Nikolai Kondrashov" <sp...@gm...> >> To: "Benjamin Tissoires" <ben...@re...>, "DIGImend-devel" <DIG...@li...> >> Sent: Sunday, March 15, 2015 12:54:19 PM >> Subject: Re: [DIGImend-devel] [PATCH 2/2] huion: Switch to reporting abstract button events >> >> On 03/15/2015 06:02 PM, Nikolai Kondrashov wrote: >>> On 03/13/2015 11:28 PM, Benjamin Tissoires wrote: >>>> Based on a patch from: Nikolai Kondrashov <Nik...@re...> >>>> >>>> Enable abstract keyboard mode for Huion tablets, which makes them report >>>> frame buttons using the pen interface and report ID. Divert these >>>> reports to a virtual report ID describing them. >>>> >>>> This makes the tablet compatible with xf86-input-wacom and libinput, >>>> but stops the frame buttons from reporting keyboard events. >>>> --- >>>> >>>> Sorry it took so long to actually propose this patch. I am fixing at the >>>> moment >>>> gnome-control-center, gnome-settings-daemon and libwacom to be able to >>>> handle >>>> also the generic devices like the Huion ones. >>> >>> This is excellent news which we'll be welcome by all users, I'm sure. >>> >>>> I need to go through all the Huion known device and see what they answer >>>> when >>>> querying the string 0x7b, but I believe checking if "HK On" is returned >>>> should >>>> allow us to prevent activating a pad interface if the device does not >>>> support >>>> it. >>> >>> We can add requesting that string to huion-tools: >>> >>> https://github.com/DIGImend/huion-tools >> >> Added. >> >>> And then ask people to run it and report results. >> >> Can make a release, build packages and send the word out to people, if you >> think it's useful. >> > > I think it will definitively be useful. > I just made a quick test, and I think you should also warn users that they > need to unplug/replug the tablet after running the tool. It can switch the > tablet in the button reporting mode and kill the current keyboard interface > (and messed up the pen interface too). I added a note on this, thanks Benjamin. I've quickly thrown together a release: https://github.com/DIGImend/uclogic-tools/releases/tag/v4 I'll be sending out a message to the list and a few people to get the dumps, tomorrow. Nick |
|
From: Benjamin T. <ben...@re...> - 2015-03-15 18:34:29
|
No need to retrieve the USB handle in input_mapping() when we already do that in probe. It also allows to use the quirk without having to add the product ID matching. Sent upstream as: https://patchwork.kernel.org/patch/6013671/ --- changes in v2: - renamed ignore_pen_interface into ignore_pen_usage - updated commit message hid-uclogic.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/hid-uclogic.c b/hid-uclogic.c index 51333fa..4b4a190 100644 --- a/hid-uclogic.c +++ b/hid-uclogic.c @@ -627,6 +627,7 @@ struct uclogic_drvdata { __u8 *rdesc; unsigned int rsize; bool invert_pen_inrange; + bool ignore_pen_usage; }; static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc, @@ -719,16 +720,12 @@ static int uclogic_input_mapping(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) { - struct usb_interface *intf; - - if (hdev->product == USB_DEVICE_ID_HUION_TABLET) { - intf = to_usb_interface(hdev->dev.parent); + struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev); - /* discard the unused pen interface */ - if ((intf->cur_altsetting->desc.bInterfaceNumber != 0) && - (field->application == HID_DG_PEN)) - return -1; - } + /* discard the unused pen interface */ + if ((drvdata->ignore_pen_usage) && + (field->application == HID_DG_PEN)) + return -1; /* let hid-core decide what to do */ return 0; @@ -910,6 +907,8 @@ static int uclogic_probe(struct hid_device *hdev, return rc; } drvdata->invert_pen_inrange = true; + } else { + drvdata->ignore_pen_usage = true; } break; case USB_DEVICE_ID_UCLOGIC_TABLET_TWHA60: -- 2.3.1 |
|
From: Benjamin T. <ben...@re...> - 2015-03-15 18:34:29
|
Based on a patch from: Nikolai Kondrashov <Nik...@re...>
Enable abstract keyboard mode for Huion tablets, which makes them report
frame buttons using the pen interface and report ID. Divert these
reports to a virtual report ID describing them.
This makes the tablet compatible with xf86-input-wacom and libinput,
but stops the frame buttons from reporting keyboard events.
---
changes in v2:
- renamed uclogic_buttonpad_rdesc_template into uclogic_buttonpad_rdesc
- amended comment above uclogic_buttonpad_rdesc
hid-uclogic.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 98 insertions(+), 5 deletions(-)
diff --git a/hid-uclogic.c b/hid-uclogic.c
index 4b4a190..0080444 100644
--- a/hid-uclogic.c
+++ b/hid-uclogic.c
@@ -613,6 +613,27 @@ static const __u8 uclogic_tablet_rdesc_template[] = {
0xC0 /* End Collection */
};
+/* Fixed virtual pad report descriptor */
+static const __u8 uclogic_buttonpad_rdesc[] = {
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x09, 0x07, /* Usage (Keypad), */
+ 0xA1, 0x01, /* Collection (Application), */
+ 0x85, 0xF7, /* Report ID (247), */
+ 0x05, 0x0D, /* Usage Page (Digitizers), */
+ 0x09, 0x39, /* Usage (Tablet Function Keys), */
+ 0xA0, /* Collection (Physical), */
+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x75, 0x01, /* Report Size (1), */
+ 0x95, 0x18, /* Report Count (24), */
+ 0x81, 0x03, /* Input (Constant, Variable), */
+ 0x19, 0x01, /* Usage Minimum (01h), */
+ 0x29, 0x08, /* Usage Maximum (08h), */
+ 0x95, 0x08, /* Report Count (8), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0xC0, /* End Collection */
+ 0xC0 /* End Collection */
+};
+
/* Parameter indices */
enum uclogic_prm {
UCLOGIC_PRM_X_LM = 1,
@@ -628,6 +649,7 @@ struct uclogic_drvdata {
unsigned int rsize;
bool invert_pen_inrange;
bool ignore_pen_usage;
+ bool has_virtual_pad_interface;
};
static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -874,6 +896,70 @@ cleanup:
return rc;
}
+/**
+ * Enable actual button mode.
+ *
+ * @hdev: HID device
+ */
+static int uclogic_button_enable(struct hid_device *hdev)
+{
+ int rc;
+ struct usb_device *usb_dev = hid_to_usb_dev(hdev);
+ struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
+ char *str_buf;
+ size_t str_len = 16;
+ unsigned char *rdesc;
+ size_t rdesc_len;
+
+ str_buf = kzalloc(str_len, GFP_KERNEL);
+ if (str_buf == NULL) {
+ hid_err(hdev, "failed to allocate string buffer\n");
+ rc = -ENOMEM;
+ goto cleanup;
+ }
+
+ /* Enable abstract keyboard mode */
+ rc = usb_string(usb_dev, 0x7b, str_buf, str_len);
+ if (rc == -EPIPE) {
+ hid_info(hdev, "button mode setting not found\n");
+ rc = 0;
+ goto cleanup;
+ } else if (rc < 0) {
+ hid_err(hdev, "failed to enable abstract keyboard\n");
+ goto cleanup;
+ } else if (strncmp(str_buf, "HK On", rc)) {
+ hid_info(hdev, "invalid answer when requesting buttons: '%s'\n",
+ str_buf);
+ rc = -EINVAL;
+ goto cleanup;
+ }
+
+ /* Re-allocate fixed report descriptor */
+ rdesc_len = drvdata->rsize + sizeof(uclogic_buttonpad_rdesc);
+ rdesc = devm_kzalloc(&hdev->dev, rdesc_len, GFP_KERNEL);
+ if (!rdesc) {
+ rc = -ENOMEM;
+ goto cleanup;
+ }
+
+ memcpy(rdesc, drvdata->rdesc, drvdata->rsize);
+
+ /* Append the buttonpad descriptor */
+ memcpy(rdesc + drvdata->rsize, uclogic_buttonpad_rdesc,
+ sizeof(uclogic_buttonpad_rdesc));
+
+ /* clean up old rdesc and use the new one */
+ drvdata->rsize = rdesc_len;
+ devm_kfree(&hdev->dev, drvdata->rdesc);
+ drvdata->rdesc = rdesc;
+
+ rc = 0;
+
+cleanup:
+ kfree(str_buf);
+ return rc;
+}
+
static int uclogic_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -907,6 +993,9 @@ static int uclogic_probe(struct hid_device *hdev,
return rc;
}
drvdata->invert_pen_inrange = true;
+
+ rc = uclogic_button_enable(hdev);
+ drvdata->has_virtual_pad_interface = !rc;
} else {
drvdata->ignore_pen_usage = true;
}
@@ -947,12 +1036,16 @@ static int uclogic_raw_event(struct hid_device *hdev, struct hid_report *report,
{
struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
- if ((drvdata->invert_pen_inrange) &&
- (report->type == HID_INPUT_REPORT) &&
+ if ((report->type == HID_INPUT_REPORT) &&
(report->id == UCLOGIC_PEN_REPORT_ID) &&
- (size >= 2))
- /* Invert the in-range bit */
- data[1] ^= 0x40;
+ (size >= 2)) {
+ if (drvdata->has_virtual_pad_interface && (data[1] & 0x20))
+ /* Change to virtual frame button report ID */
+ data[0] = 0xf7;
+ else if (drvdata->invert_pen_inrange)
+ /* Invert the in-range bit */
+ data[1] ^= 0x40;
+ }
return 0;
}
--
2.3.1
|
|
From: Nikolai K. <sp...@gm...> - 2015-03-16 19:45:09
|
On 03/15/2015 08:34 PM, Benjamin Tissoires wrote: > Based on a patch from: Nikolai Kondrashov <Nik...@re...> > > Enable abstract keyboard mode for Huion tablets, which makes them report > frame buttons using the pen interface and report ID. Divert these > reports to a virtual report ID describing them. > > This makes the tablet compatible with xf86-input-wacom and libinput, > but stops the frame buttons from reporting keyboard events. > --- > > changes in v2: > - renamed uclogic_buttonpad_rdesc_template into uclogic_buttonpad_rdesc > - amended comment above uclogic_buttonpad_rdesc Looks fine now. Only changed the tag in the subject to "uclogic", and merged. Thanks, Benjamin! Nick |