Thread: [DIGImend-devel] PT-1001 digitizer
Brought to you by:
spb_nick
Menu
▾
▴
digimend-devel
|
From: Yann V. <yan...@or...> - 2015-01-20 14:08:59
|
I have a couple of PT-1001 tablets, also known as " Digi Tablet" over USB.
Mine happen to be branded Leogics, but many other brands occur, including none.
The USB vendor is 0x099a Zippy Technology Corp.
It presents three interfaces: one keyboard and two pointers. The purpose of
the second pointer device is somewhat unclear as it does not seem to produce
events, but its descriptors do describe it as a stylus with only three buttons
plus presence.
With recent Linux kernels these produce no events and a couple of warnings:
[ 6719.887522] hid-generic 0003:099A:2620.001C: input,hidraw3: USB HID v1.10 Keyboard [ Digi Tablet] on usb-0000:00:14.0-7/input0
[ 6719.997108] hid-generic 0003:099A:2620.001D: unknown main item tag 0x0
[ 6719.997114] hid-generic 0003:099A:2620.001D: unbalanced collection at end of report description
[ 6719.997121] hid-generic: probe of 0003:099A:2620.001D failed with error -22
I've tracked that problem down; Linux requests precisely the exact buffer and
transfer size for the report descriptors, and this device has an off by one
bug where it doesn't send the last byte. In the particular descriptor for the
events it does send, that last byte is a C0 = End Collection, so the structure
breaks and Linux ignores that interface (it has three). USB transfer dumps
show Windows requests 64 bytes more than the descriptor size, which explains
why it does not trigger this bug.
Possible workarounds include patching in that extra byte or requesting more
(both tested and work). A proper fix would probably include not trying to
parse more descriptor data than the device actually returned. I do suggest
requesting more than the descriptor length by default, since this bug may
be present in other devices (possibly as a misinterpretation of USB's use
of a full frame to indicate more data is available). My own test was simply
to add 1 to rsize and dev_rsize in kmalloc and hid_get_class_descriptor
calls within drivers/hid/usbhid/hid-core.c, but that doesn't check how much
was actually received.
That's not the end of the story, however. Even with proper parsing of the
descriptors it seems the tablet only has one interface that reports a stylus,
and it's not the one sending events. This causes the Xorg driver to deduce it
is a touchpad, not a pen tablet, with all the resulting defects (relative
mode, ignoring events when the nib isn't pressed). It also reports a rather
mystifying set of buttons (from xinput list-props, but do come from kernel,
as confirmed using input-events):
Button Labels (276): "Button Left" (145), "Button Middle" (146),
"Button Right" (147), "Button Wheel Up" (148), "Button Wheel Down" (149),
"Button Horiz Wheel Left" (150), "Button Horiz Wheel Right" (151),
"Button Side" (265), "Button Extra" (266), "Button Forward" (714),
"Button Back" (715), "Button Task" (716), "Button Unknown" (264),
"Button Unknown" (264), "Button Unknown" (264), "Button Unknown" (264)
The second pointer device reported, which does not produce events, has an
equally useless button map:
"Button 0" (630), "Button Unknown" (264), "Button Unknown" (264),
"Button Wheel Up" (148), "Button Wheel Down" (149)
The actual buttons (nib and two side buttons on the stylus) are those
reported as Forward, Back and Task. Remapping them with xinput set-button-map
and setting absolute mode makes it usable but with the serious issue of not
moving the pointer if the nib isn't pressed (the pressure level works in
inkscape). I did not find a way to tell Xorg's evdev driver that the device
is a tablet.
Among the firmware controlled soft buttons (tappable with the stylus) on
the tablet itself is a mouse/stylus switch control. The other side fields
produce other events, mostly keyboard combinations (common zoom hotkeys,
save, open, copy, paste etc) and scroll events. Using this toggle to set
mouse mode causes the first group of buttons to be used, so does map to
mouse buttons by default, but the pointer motion is ignored by X (since
it does not report pressure, this might be an artifact of its touchpad
misinterpretation). The keyboard events are sent on interface 0.
So, on interface 1, we have a bunch of possible reports:
1 5 button relative XY, sent in mouse mode (boot protocol compatible?)
2 scroll wheel? duplicated in descriptor. usages Wheel and AC Pan
3 Unknown use, looks like keyboard reports
5 multimedia buttons
9 tablet mode stylus events
As it happens, it looks like it sticks to report 9 and keyboard events
on interface 0, switching to report 1 in mouse mode.
And the kernel has dutifully collected all those possible buttons in one
device, placing the tablet ones at number 10-12. The hint that this is
a pen tablet is in interface 2, which maps to another device. It could
be that Windows associates the two, maybe by bInterfaceProtocol=3?
The failing heuristic in Xorg's evdev driver is:
if ((libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_X) &&
libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_Y))) {
xf86IDrvMsg(pInfo, X_PROBED, "Found x and y absolute axes\n");
if (libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_TOOL_PEN) ||
libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_STYLUS) ||
libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_STYLUS2))
{
xf86IDrvMsg(pInfo, X_PROBED, "Found absolute tablet.\n");
pEvdev->flags |= EVDEV_TABLET;
if (!pEvdev->num_buttons)
{
pEvdev->num_buttons = 7; /* LMR + scroll wheels */
pEvdev->flags |= EVDEV_BUTTON_EVENTS;
}
} else if (libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_PRESSURE) ||
libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_TOUCH)) {
if (has_lmr || libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_TOOL_FINGER)) {
xf86IDrvMsg(pInfo, X_PROBED, "Found absolute touchpad.\n");
pEvdev->flags |= EVDEV_TOUCHPAD;
} else {
xf86IDrvMsg(pInfo, X_PROBED, "Found absolute touchscreen\n");
pEvdev->flags |= EVDEV_TOUCHSCREEN;
pEvdev->flags |= EVDEV_BUTTON_EVENTS;
}
} else if (!(libevdev_has_event_code(pEvdev->dev, EV_REL, REL_X) &&
libevdev_has_event_code(pEvdev->dev, EV_REL, REL_Y)) && has_lmr) {
/* some touchscreens use BTN_LEFT rather than BTN_TOUCH */
xf86IDrvMsg(pInfo, X_PROBED, "Found absolute touchscreen\n");
pEvdev->flags |= EVDEV_TOUCHSCREEN;
pEvdev->flags |= EVDEV_BUTTON_EVENTS;
}
} else {
#ifdef MULTITOUCH
if (!libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_MT_POSITION_X) ||
!libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_MT_POSITION_Y))
#endif
EvdevForceXY(pInfo, Absolute);
}
For interface 1 this code finds absolute axis, including pressure, but no
button marked stylus (it is marked as mouse button 1, but mislabeled as Back
by the kernel somehow), and so it decides on a touchpad. This single error
causes it to set relative mode and ignore any motion without pressure.
So, opinions? How should these flaws be patched? |
|
From: Nikolai K. <sp...@gm...> - 2015-01-25 14:41:14
|
Hi Yann,
On 01/20/2015 03:46 PM, Yann Vernier wrote:
> I have a couple of PT-1001 tablets, also known as " Digi Tablet" over USB.
> Mine happen to be branded Leogics, but many other brands occur, including none.
> The USB vendor is 0x099a Zippy Technology Corp.
>
> It presents three interfaces: one keyboard and two pointers. The purpose of
> the second pointer device is somewhat unclear as it does not seem to produce
> events, but its descriptors do describe it as a stylus with only three buttons
> plus presence.
The second pointer interface is probably for a mouse (or "puck" in the old
parlance), which is supported by the hardware, but is not supplied with this
product. OTOH, they support "mouse" mode as is and the tablet working area is
very small, although the hardware might support bigger ones.
> With recent Linux kernels these produce no events and a couple of warnings:
> [ 6719.887522] hid-generic 0003:099A:2620.001C: input,hidraw3: USB HID v1.10 Keyboard [ Digi Tablet] on usb-0000:00:14.0-7/input0
> [ 6719.997108] hid-generic 0003:099A:2620.001D: unknown main item tag 0x0
> [ 6719.997114] hid-generic 0003:099A:2620.001D: unbalanced collection at end of report description
> [ 6719.997121] hid-generic: probe of 0003:099A:2620.001D failed with error -22
>
> I've tracked that problem down; Linux requests precisely the exact buffer and
> transfer size for the report descriptors, and this device has an off by one
> bug where it doesn't send the last byte. In the particular descriptor for the
> events it does send, that last byte is a C0 = End Collection, so the structure
> breaks and Linux ignores that interface (it has three). USB transfer dumps
> show Windows requests 64 bytes more than the descriptor size, which explains
> why it does not trigger this bug.
Wow, that is quite an ugly bug and neat research!
> Possible workarounds include patching in that extra byte or requesting more
> (both tested and work). A proper fix would probably include not trying to
> parse more descriptor data than the device actually returned.
Well, such fix still wouldn't work with devices which would get a more
important part of their report descriptor lost. Then, making the parser handle
all the possible failures it introduces would make it too complicated.
> I do suggest requesting more than the descriptor length by default, since
> this bug may be present in other devices (possibly as a misinterpretation of
> USB's use of a full frame to indicate more data is available). My own test
> was simply to add 1 to rsize and dev_rsize in kmalloc and
> hid_get_class_descriptor calls within drivers/hid/usbhid/hid-core.c, but
> that doesn't check how much was actually received.
This would be one controversial solution. How much more would we need to
request? Would this encourage more sloppy HID descriptors in the future? When
should we stop requesting more and more?
I would say we simply replace the device descriptor with a rewritten
descriptor as is done for many other devices already. See hid-uclogic.c for
example.
> That's not the end of the story, however. Even with proper parsing of the
> descriptors it seems the tablet only has one interface that reports a stylus,
> and it's not the one sending events. This causes the Xorg driver to deduce it
> is a touchpad, not a pen tablet, with all the resulting defects (relative
> mode, ignoring events when the nib isn't pressed). It also reports a rather
> mystifying set of buttons (from xinput list-props, but do come from kernel,
> as confirmed using input-events):
> Button Labels (276): "Button Left" (145), "Button Middle" (146),
> "Button Right" (147), "Button Wheel Up" (148), "Button Wheel Down" (149),
> "Button Horiz Wheel Left" (150), "Button Horiz Wheel Right" (151),
> "Button Side" (265), "Button Extra" (266), "Button Forward" (714),
> "Button Back" (715), "Button Task" (716), "Button Unknown" (264),
> "Button Unknown" (264), "Button Unknown" (264), "Button Unknown" (264)
> The second pointer device reported, which does not produce events, has an
> equally useless button map:
> "Button 0" (630), "Button Unknown" (264), "Button Unknown" (264),
> "Button Wheel Up" (148), "Button Wheel Down" (149)
>
> The actual buttons (nib and two side buttons on the stylus) are those
> reported as Forward, Back and Task. Remapping them with xinput set-button-map
> and setting absolute mode makes it usable but with the serious issue of not
> moving the pointer if the nib isn't pressed (the pressure level works in
> inkscape). I did not find a way to tell Xorg's evdev driver that the device
> is a tablet.
Windows report descriptor parsing works quite differently from Linux and such
mapping issues happen often.
We can avoid this problem altogether by simply using appropriate field
usages in the rewritten report descriptor.
> Among the firmware controlled soft buttons (tappable with the stylus) on
> the tablet itself is a mouse/stylus switch control. The other side fields
> produce other events, mostly keyboard combinations (common zoom hotkeys,
> save, open, copy, paste etc) and scroll events. Using this toggle to set
> mouse mode causes the first group of buttons to be used, so does map to
> mouse buttons by default, but the pointer motion is ignored by X (since
> it does not report pressure, this might be an artifact of its touchpad
> misinterpretation). The keyboard events are sent on interface 0.
>
> So, on interface 1, we have a bunch of possible reports:
> 1 5 button relative XY, sent in mouse mode (boot protocol compatible?)
> 2 scroll wheel? duplicated in descriptor. usages Wheel and AC Pan
> 3 Unknown use, looks like keyboard reports
> 5 multimedia buttons
> 9 tablet mode stylus events
> As it happens, it looks like it sticks to report 9 and keyboard events
> on interface 0, switching to report 1 in mouse mode.
Wow! This is the first tablet I see that is trying to actually convert
various working area events into something else in *hardware*, i.e. on the
device side. All others do this in the driver.
> And the kernel has dutifully collected all those possible buttons in one
> device, placing the tablet ones at number 10-12. The hint that this is
> a pen tablet is in interface 2, which maps to another device. It could
> be that Windows associates the two, maybe by bInterfaceProtocol=3?
>
> The failing heuristic in Xorg's evdev driver is:
> if ((libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_X) &&
> libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_Y))) {
> xf86IDrvMsg(pInfo, X_PROBED, "Found x and y absolute axes\n");
> if (libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_TOOL_PEN) ||
> libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_STYLUS) ||
> libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_STYLUS2))
> {
> xf86IDrvMsg(pInfo, X_PROBED, "Found absolute tablet.\n");
> pEvdev->flags |= EVDEV_TABLET;
> if (!pEvdev->num_buttons)
> {
> pEvdev->num_buttons = 7; /* LMR + scroll wheels */
> pEvdev->flags |= EVDEV_BUTTON_EVENTS;
> }
> } else if (libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_PRESSURE) ||
> libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_TOUCH)) {
> if (has_lmr || libevdev_has_event_code(pEvdev->dev, EV_KEY, BTN_TOOL_FINGER)) {
> xf86IDrvMsg(pInfo, X_PROBED, "Found absolute touchpad.\n");
> pEvdev->flags |= EVDEV_TOUCHPAD;
> } else {
> xf86IDrvMsg(pInfo, X_PROBED, "Found absolute touchscreen\n");
> pEvdev->flags |= EVDEV_TOUCHSCREEN;
> pEvdev->flags |= EVDEV_BUTTON_EVENTS;
> }
> } else if (!(libevdev_has_event_code(pEvdev->dev, EV_REL, REL_X) &&
> libevdev_has_event_code(pEvdev->dev, EV_REL, REL_Y)) && has_lmr) {
> /* some touchscreens use BTN_LEFT rather than BTN_TOUCH */
> xf86IDrvMsg(pInfo, X_PROBED, "Found absolute touchscreen\n");
> pEvdev->flags |= EVDEV_TOUCHSCREEN;
> pEvdev->flags |= EVDEV_BUTTON_EVENTS;
> }
> } else {
> #ifdef MULTITOUCH
> if (!libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_MT_POSITION_X) ||
> !libevdev_has_event_code(pEvdev->dev, EV_ABS, ABS_MT_POSITION_Y))
> #endif
> EvdevForceXY(pInfo, Absolute);
> }
>
> For interface 1 this code finds absolute axis, including pressure, but no
> button marked stylus (it is marked as mouse button 1, but mislabeled as Back
> by the kernel somehow), and so it decides on a touchpad. This single error
> causes it to set relative mode and ignore any motion without pressure.
>
>
> So, opinions? How should these flaws be patched?
This X.org heuristic is pretty hairy, yes, but we can still cater to it by
using appropriate report descriptor(s).
So, I think the path to a solution here should be making a separate HID driver
for this tablet and having a new set of report descriptors written, replacing
the original ones. Perhaps with some bit tweaking of the input reports as
well.
You can take hid-uclogic.c for boilerplate, as the most basic HID driver at
the moment doing similar things. If you know how report descriptors work you
can try making one yourself. I can suggest using my hidrd-convert [1] tool for
authoring, although there are a few others. You can find some examples of
rewritten descriptor XML sources in our tablets repo [2] - search for
"fixed*.xml".
Otherwise you can map the bits of all the reports on all the interfaces and I
can try writing them for you, but doing it yourself might be faster and more
fun :)
Thank you for thorough research and description of the problem!
Nick
[1] https://github.com/DIGImend/hidrd
[2] https://github.com/DIGImend/tablets
|
|
From: Nikolai K. <sp...@gm...> - 2015-01-25 14:44:34
|
On 01/25/2015 04:41 PM, Nikolai Kondrashov wrote: > So, I think the path to a solution here should be making a separate HID driver > for this tablet and having a new set of report descriptors written, replacing > the original ones. Perhaps with some bit tweaking of the input reports as > well. > > You can take hid-uclogic.c for boilerplate, as the most basic HID driver at > the moment doing similar things. Oh, and doing it within digimend-kernel-drivers package might be easier than on the whole kernel tree. Nick [1] https://github.com/DIGImend/digimend-kernel-drivers |
|
From: Nikolai K. <sp...@gm...> - 2015-01-26 08:33:30
|
Hi Yann, Could you please keep the maillist in the CC? Thank you. On 01/26/2015 01:01 AM, Yann Vernier wrote: > On Sun, 25 Jan 2015 16:41:04 +0200 > Nikolai Kondrashov <sp...@gm...> wrote: >> On 01/20/2015 03:46 PM, Yann Vernier wrote: >>> Possible workarounds include patching in that extra byte or requesting more >>> (both tested and work). A proper fix would probably include not trying to >>> parse more descriptor data than the device actually returned. >> >> Well, such fix still wouldn't work with devices which would get a more >> important part of their report descriptor lost. Then, making the parser handle >> all the possible failures it introduces would make it too complicated. > > Yes, attempting to guess the missing data is a bad idea. The current behaviour > of not even noticing that it is missing isn't really much better, however. > However, if I just make sure to request a larger size I get the correct amount. Hmm, from your description it seems to me that Linux notices it is missing and actively drops the interface with a bad descriptor, doesn't it? Or did I understand you wrong? >>> I do suggest requesting more than the descriptor length by default, since >>> this bug may be present in other devices (possibly as a misinterpretation of >>> USB's use of a full frame to indicate more data is available). My own test >>> was simply to add 1 to rsize and dev_rsize in kmalloc and >>> hid_get_class_descriptor calls within drivers/hid/usbhid/hid-core.c, but >>> that doesn't check how much was actually received. >> >> This would be one controversial solution. How much more would we need to >> request? Would this encourage more sloppy HID descriptors in the future? When >> should we stop requesting more and more? > > The descriptor isn't actually corrupt, only the transfer is incomplete. > The descriptor is 317 bytes. > Linux requests 317 bytes, and receives 316 bytes because of the device bug. > Linux does not check the size received and therefore not only fails in > parsing the descriptor but reports a different problem (failing to parse a > byte that was never received in the first place). > Windows requests 317+64 bytes, and receives 317 bytes. > If I make Linux request 317+1 bytes, I get 317 bytes, forming the complete > descriptor. > > The main reason I believe we could safely request a larger size from most > HID devices is that Windows apparently already does. Then again, I would > not be terribly surprised if a modern console used that behaviour to > trigger weirdness (we've seen them abuse USB before). An alternative that > could reasonably work is just attempting rsize+1 if we get an incomplete > descriptor on the first try. The advantage is that it would only affect > devices that were displaying bad behaviour in the first place. Sorry, I might have misunderstood you. I'll need to read the Linux USB code to talk about this. >> I would say we simply replace the device descriptor with a rewritten >> descriptor as is done for many other devices already. See hid-uclogic.c for >> example. > > That would allow us to clean it up for better describing the device (such as > it having a stylus), as you noted below. I'll set about doing so tomorrow, > I think. Great! >>> That's not the end of the story, however. Even with proper parsing of the >>> descriptors it seems the tablet only has one interface that reports a stylus, >>> and it's not the one sending events. This causes the Xorg driver to deduce it >>> is a touchpad, not a pen tablet, with all the resulting defects (relative >>> mode, ignoring events when the nib isn't pressed). It also reports a rather >>> mystifying set of buttons (from xinput list-props, but do come from kernel, >>> as confirmed using input-events): >>> Button Labels (276): "Button Left" (145), "Button Middle" (146), >>> "Button Right" (147), "Button Wheel Up" (148), "Button Wheel Down" (149), >>> "Button Horiz Wheel Left" (150), "Button Horiz Wheel Right" (151), >>> "Button Side" (265), "Button Extra" (266), "Button Forward" (714), >>> "Button Back" (715), "Button Task" (716), "Button Unknown" (264), >>> "Button Unknown" (264), "Button Unknown" (264), "Button Unknown" (264) >>> The second pointer device reported, which does not produce events, has an >>> equally useless button map: >>> "Button 0" (630), "Button Unknown" (264), "Button Unknown" (264), >>> "Button Wheel Up" (148), "Button Wheel Down" (149) >>> >>> The actual buttons (nib and two side buttons on the stylus) are those >>> reported as Forward, Back and Task. Remapping them with xinput set-button-map >>> and setting absolute mode makes it usable but with the serious issue of not >>> moving the pointer if the nib isn't pressed (the pressure level works in >>> inkscape). I did not find a way to tell Xorg's evdev driver that the device >>> is a tablet. >> >> Windows report descriptor parsing works quite differently from Linux and such >> mapping issues happen often. > > I suspect there's actually a parsing bug in Linux here, as the kernel values > barely have any similarities to the descriptors themselves. The first five on > interface 1 match (being the boot protocol mouse buttons), as do the axis, > but none of the other buttons do - interface 2 doesn't even have five > buttons (it has four). I might try to figure out what the kernel HID parser > did later. I think it is intentional, although presents a Windows-compatibility problem. Basically, IIRC, if Linux encounters a field with usage which should be mapped to the same event as a field previously seen in the same report descriptor, it assigns this field a higher event number. This might be limited to some event types. The HID_QUIRK_MULTI_INPUT quirk is used to make the driver create separate event devices for different report IDs to avoid this. >> Wow! This is the first tablet I see that is trying to actually convert >> various working area events into something else in *hardware*, i.e. on the >> device side. All others do this in the driver. > > I expect it's a firmware thing. Sure. > It does peculiar things including translating three virtual sliders into > different types of events (mouse scroll by emulated mouse buttons, volume > and zoom via keyboard keys) and plenty of regions send key combinations such > as Ctrl+C rather than a HID keycode with assigned meaning. Well, sending particular, Windows-oriented key combinations (at least for real on-the-frame buttons) is common among graphics tablets. > The PCB is marked 155-D, and has a Polostar controller marked TB92E and > A941K0420G1 (in the one I opened). The only other IC was an opamp. It might > have a command to report raw tablet data, like the Huion tablets, which would > make sense of interface 2. Finding it would be trickier since there is no > driver; it's clearly designed to operate with Windows' generic drivers. > http://www.polostar-tech.com/ appears to be the correct company. Thanks, I might take a look at them later. I think it's actually not a bad thing of them to try to make it work without a driver. >> This X.org heuristic is pretty hairy, yes, but we can still cater to it by >> using appropriate report descriptor(s). >> >> So, I think the path to a solution here should be making a separate HID driver >> for this tablet and having a new set of report descriptors written, replacing >> the original ones. Perhaps with some bit tweaking of the input reports as >> well. > > The formats described seem to work just fine; it just has a few more mystery > reports than I've observed in use. I'll try overriding the button usage codes. Alright. >> You can take hid-uclogic.c for boilerplate, as the most basic HID driver at >> the moment doing similar things. If you know how report descriptors work you >> can try making one yourself. I can suggest using my hidrd-convert [1] tool for >> authoring, although there are a few others. You can find some examples of >> rewritten descriptor XML sources in our tablets repo [2] - search for >> "fixed*.xml". > > The descriptors I attached to my first mail were decoded with hidrd-convert. Ah, sorry, forgot to look at the attachments in a hurry. > I should be able to alter them sensibly with the help of the public HID specs. Good. Don't hesitate to write, if something is not obvious there. Perhaps I'll be able to help. Nick |
|
From: Yann V. <yan...@or...> - 2015-01-28 10:43:06
Attachments:
hid-polostar.c
|
On Mon, 26 Jan 2015 10:33:16 +0200 Nikolai Kondrashov <sp...@gm...> wrote: > Hi Yann, > > Could you please keep the maillist in the CC? Thank you. Certainly. I made a mistake when hitting reply, for which I apologize. It was not my intent to exclude the list. > On 01/26/2015 01:01 AM, Yann Vernier wrote: > > On Sun, 25 Jan 2015 16:41:04 +0200 > > Nikolai Kondrashov <sp...@gm...> wrote: > >> On 01/20/2015 03:46 PM, Yann Vernier wrote: > >>> Possible workarounds include patching in that extra byte or requesting more > >>> (both tested and work). A proper fix would probably include not trying to > >>> parse more descriptor data than the device actually returned. > >> > >> Well, such fix still wouldn't work with devices which would get a more > >> important part of their report descriptor lost. Then, making the parser handle > >> all the possible failures it introduces would make it too complicated. > > > > Yes, attempting to guess the missing data is a bad idea. The current behaviour > > of not even noticing that it is missing isn't really much better, however. > > However, if I just make sure to request a larger size I get the correct amount. > > Hmm, from your description it seems to me that Linux notices it is missing and > actively drops the interface with a bad descriptor, doesn't it? Or did I > understand you wrong? It only drops it after failing to parse uninitialized data. drivers/hid/usbhid/hid-core.c usbhid_parse and hid_post_reset both call hid_get_class_descriptor, which does check the size, but gives up after four tries. Even so, having received some data it returns the incomplete descriptor, i.e. instead of error (negative) it returns rsize-1 (as the last byte is missing). It never attempted a larger read, so it did not work around the bug in the PT-1001. In usbhid_parse (and hid_post_reset), the return value is only checked for negative, not for wrong size, so the error is lost at this point. This subtle bug changes the meaning of the received descriptor from incomplete (truncated) to corrupted (padded, in this case with zero, but kmalloc does not guarantee that). Since the error was lost, Linux proceeds to hid_parse_report. As the last byte was never received, it has an erroneous value, and that's when the parsing fails. Linux attempted to parse a byte that did not come from the device. Had the descriptor been different up to that point, the corruption could have gone unnoticed, producing arbitrary results. Linux only detected a problem because a C0 was required. > >>> I do suggest requesting more than the descriptor length by default, since > >>> this bug may be present in other devices (possibly as a misinterpretation of > >>> USB's use of a full frame to indicate more data is available). My own test > >>> was simply to add 1 to rsize and dev_rsize in kmalloc and > >>> hid_get_class_descriptor calls within drivers/hid/usbhid/hid-core.c, but > >>> that doesn't check how much was actually received. > >> > >> This would be one controversial solution. How much more would we need to > >> request? Would this encourage more sloppy HID descriptors in the future? When > >> should we stop requesting more and more? > > > > The descriptor isn't actually corrupt, only the transfer is incomplete. > > The descriptor is 317 bytes. > > Linux requests 317 bytes, and receives 316 bytes because of the device bug. > > Linux does not check the size received and therefore not only fails in > > parsing the descriptor but reports a different problem (failing to parse a > > byte that was never received in the first place). > > Windows requests 317+64 bytes, and receives 317 bytes. > > If I make Linux request 317+1 bytes, I get 317 bytes, forming the complete > > descriptor. > > > > The main reason I believe we could safely request a larger size from most > > HID devices is that Windows apparently already does. Then again, I would > > not be terribly surprised if a modern console used that behaviour to > > trigger weirdness (we've seen them abuse USB before). An alternative that > > could reasonably work is just attempting rsize+1 if we get an incomplete > > descriptor on the first try. The advantage is that it would only affect > > devices that were displaying bad behaviour in the first place. > > Sorry, I might have misunderstood you. I'll need to read the Linux USB code to > talk about this. > >>> That's not the end of the story, however. Even with proper parsing of the > >>> descriptors it seems the tablet only has one interface that reports a stylus, > >>> and it's not the one sending events. This causes the Xorg driver to deduce it > >>> is a touchpad, not a pen tablet, with all the resulting defects (relative > >>> mode, ignoring events when the nib isn't pressed). It also reports a rather > >>> mystifying set of buttons (from xinput list-props, but do come from kernel, > >>> as confirmed using input-events): > >>> Button Labels (276): "Button Left" (145), "Button Middle" (146), > >>> "Button Right" (147), "Button Wheel Up" (148), "Button Wheel Down" (149), > >>> "Button Horiz Wheel Left" (150), "Button Horiz Wheel Right" (151), > >>> "Button Side" (265), "Button Extra" (266), "Button Forward" (714), > >>> "Button Back" (715), "Button Task" (716), "Button Unknown" (264), > >>> "Button Unknown" (264), "Button Unknown" (264), "Button Unknown" (264) > >>> The second pointer device reported, which does not produce events, has an > >>> equally useless button map: > >>> "Button 0" (630), "Button Unknown" (264), "Button Unknown" (264), > >>> "Button Wheel Up" (148), "Button Wheel Down" (149) > >>> > >>> The actual buttons (nib and two side buttons on the stylus) are those > >>> reported as Forward, Back and Task. Remapping them with xinput set-button-map > >>> and setting absolute mode makes it usable but with the serious issue of not > >>> moving the pointer if the nib isn't pressed (the pressure level works in > >>> inkscape). I did not find a way to tell Xorg's evdev driver that the device > >>> is a tablet. > >> > >> Windows report descriptor parsing works quite differently from Linux and such > >> mapping issues happen often. > > > > I suspect there's actually a parsing bug in Linux here, as the kernel values > > barely have any similarities to the descriptors themselves. The first five on > > interface 1 match (being the boot protocol mouse buttons), as do the axis, > > but none of the other buttons do - interface 2 doesn't even have five > > buttons (it has four). I might try to figure out what the kernel HID parser > > did later. > > I think it is intentional, although presents a Windows-compatibility problem. > Basically, IIRC, if Linux encounters a field with usage which should be mapped > to the same event as a field previously seen in the same report descriptor, it > assigns this field a higher event number. This might be limited to some event > types. The HID_QUIRK_MULTI_INPUT quirk is used to make the driver create > separate event devices for different report IDs to avoid this. Interesting. MULTI_INPUT might be appropriate to separate mouse and tablet mode in the PT-1001, as they produce different reports. The strange part, comparing to the report descriptors, is that the button meanings generally don't match up at all, even on the second interface where all buttons have usages and none collide. The anti-collission support one might expect would allocate different generic buttons rather than six copies of "unknown" sprinkled among a rather random selection. I have attached an initial draft of a pt1001 descriptor patch driver, but it still has some issues: For some reason, hid-generic still takes priority when choosing driver. I have to unbind hid-generic to use this driver. The button usages still don't show through, and worse, the stylus tip and second barrel (reported as eraser) button both show as button 1. Using input-events seems to explain this as them being reported as touch, stylus and button 0; the expected set was stylus, barrel and eraser. With this button setup xinput mapping cannot separate them. multi_input did help in that both tablet and mouse mode work, and the modified descriptor (with digitizer rather than pointer) makes tablet mode function correctly except for the button SNAFU. I will probably pick this little project up again after FOSDEM. |
|
From: Nikolai K. <sp...@gm...> - 2015-01-31 10:35:39
|
Hi Yann, On 01/28/2015 12:42 PM, Yann Vernier wrote: > On Mon, 26 Jan 2015 10:33:16 +0200 > Nikolai Kondrashov <sp...@gm...> wrote: >> Could you please keep the maillist in the CC? Thank you. > > Certainly. I made a mistake when hitting reply, for which I apologize. > It was not my intent to exclude the list. Sure, no problem :) >> Hmm, from your description it seems to me that Linux notices it is missing and >> actively drops the interface with a bad descriptor, doesn't it? Or did I >> understand you wrong? > > It only drops it after failing to parse uninitialized data. > > drivers/hid/usbhid/hid-core.c usbhid_parse and hid_post_reset both call > hid_get_class_descriptor, which does check the size, but gives up after > four tries. Even so, having received some data it returns the incomplete > descriptor, i.e. instead of error (negative) it returns rsize-1 (as the > last byte is missing). It never attempted a larger read, so it did not > work around the bug in the PT-1001. > > In usbhid_parse (and hid_post_reset), the return value is only checked > for negative, not for wrong size, so the error is lost at this point. > This subtle bug changes the meaning of the received descriptor from > incomplete (truncated) to corrupted (padded, in this case with zero, > but kmalloc does not guarantee that). > > Since the error was lost, Linux proceeds to hid_parse_report. As the last > byte was never received, it has an erroneous value, and that's when the > parsing fails. Linux attempted to parse a byte that did not come from the > device. Had the descriptor been different up to that point, the corruption > could have gone unnoticed, producing arbitrary results. Linux only detected > a problem because a C0 was required. Thank you for a detailed explanation. Yes, it seems the error handling needs to be improved here. >> I think it is intentional, although presents a Windows-compatibility problem. >> Basically, IIRC, if Linux encounters a field with usage which should be mapped >> to the same event as a field previously seen in the same report descriptor, it >> assigns this field a higher event number. This might be limited to some event >> types. The HID_QUIRK_MULTI_INPUT quirk is used to make the driver create >> separate event devices for different report IDs to avoid this. > > Interesting. MULTI_INPUT might be appropriate to separate mouse and tablet > mode in the PT-1001, as they produce different reports. Yes, it will help there. > The strange part, comparing to the report descriptors, is that the button > meanings generally don't match up at all, even on the second interface where > all buttons have usages and none collide. The anti-collission support one > might expect would allocate different generic buttons rather than six copies > of "unknown" sprinkled among a rather random selection. Yes, it's quite hairy. > I have attached an initial draft of a pt1001 descriptor patch driver, but > it still has some issues: > > For some reason, hid-generic still takes priority when choosing driver. > I have to unbind hid-generic to use this driver. Yes, the generic driver takes over everything it doesn't know about and the official solution for out-of-tree drivers is to rebind. For that reason digimend-kernel-drivers installs the rebinding script and udev rules. See hid-rebind.rules, and hid-rebind. > The button usages still don't show through, and worse, the stylus tip and > second barrel (reported as eraser) button both show as button 1. Using > input-events seems to explain this as them being reported as touch, > stylus and button 0; the expected set was stylus, barrel and eraser. > With this button setup xinput mapping cannot separate them. I'm not sure why, but Eraser usage (0x45) doesn't seem to be mapped explicitly and goes to the "unknown" label in hidinput_configure_usage(), probably being mapped to BTN_0 in map_key() from there. > multi_input did help in that both tablet and mouse mode work, and the > modified descriptor (with digitizer rather than pointer) makes tablet > mode function correctly except for the button SNAFU. > > I will probably pick this little project up again after FOSDEM. Cool, you're going to FOSDEM, I'm envious :) I'll be going to devconf.cz next week myself, so will likely reply even slower than usual. The code looks fine to me, but I have a few comments on the report descriptor. I think it is better to omit report ID's, features and fields that are not used by the driver, for clarity. Physical extents are useless without units and probably don't even apply to some usages below, like resolution multiplier, so it's better to remove them. However, adding the units, unit exponent and physical extents to the pen coordinates is useful. The result will be visible in "evtest" output and likely used by Wayland. I'm not sure how repeated report ID items in a single collection even work. Also duplicate report IDs sound fishy. All-in-all, this is some really weird report descriptor and would be difficult to reason about. I would start from scratch, taking one of the fixed report descriptors from other tablets and adjusting it to the bit fields this one reports. > /* I expect patching in these values (from interface 2) > would get Xorg to use tablet mode: */ > 0x09, 0x42, /* Usage (Tip Switch), */ > 0x09, 0x44, /* Usage (Barrel Switch), */ > 0x09, 0x45, /* Usage (Eraser), */ /* Really on the barrel too */ I suggest to use "Tablet Pick" here. I added it to the kernel and use it for the second barrel button on all of the tablets. It will map to the third button. > /* The original usages should have mapped to mouse buttons, but Linux > happily reports them as buttons 10-12. */ > /* 0x05, 0x0f, / * Usage Page (Button), */ > /* 0x19, 0x01, / * Usage Minimum (01h), */ > /* 0x29, 0x03, / * Usage Maximum (03h), */ > /* End of Stylus button usage */ I think having six usages and only three reports wouldn't work. > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x95, 0x03, /* Report Count (3), */ > 0x75, 0x01, /* Report Size (1), */ > 0x81, 0x02, /* Input (Variable), */ > 0x95, 0x05, /* Report Count (5), */ > 0x81, 0x01, /* Input (Constant), */ > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x30, /* Usage (X), */ > 0x09, 0x31, /* Usage (Y), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x26, 0x00, 0x10, /* Logical Maximum (4096), */ > 0x35, 0x00, /* Physical Minimum (0), */ > 0x46, 0x00, 0x10, /* Physical Maximum (4096), */ Is the actual physical drawing area square? Nick |
|
From: Yann V. <yan...@or...> - 2015-02-19 16:19:32
Attachments:
hid-polostar.c
|
On Sat, 31 Jan 2015 12:35:28 +0200 Nikolai Kondrashov <sp...@gm...> wrote: > The code looks fine to me, but I have a few comments on the report descriptor. > > I think it is better to omit report ID's, features and fields that are not > used by the driver, for clarity. > > Physical extents are useless without units and probably don't even apply to > some usages below, like resolution multiplier, so it's better to remove them. > However, adding the units, unit exponent and physical extents to the pen > coordinates is useful. The result will be visible in "evtest" output and > likely used by Wayland. Okay. > I'm not sure how repeated report ID items in a single collection even work. > Also duplicate report IDs sound fishy. All-in-all, this is some really weird > report descriptor and would be difficult to reason about. I would start from > scratch, taking one of the fixed report descriptors from other tablets and > adjusting it to the bit fields this one reports. I think it's some sort of indirect description, applying resolution multipliers to the preexisting report 1, so as not to confuse the boot protocol interpretation. However, I have not gotten the device to send the multiplier reports, and Linux seems to happily ignore them. > > /* I expect patching in these values (from interface 2) > > would get Xorg to use tablet mode: */ > > 0x09, 0x42, /* Usage (Tip Switch), */ > > 0x09, 0x44, /* Usage (Barrel Switch), */ > > 0x09, 0x45, /* Usage (Eraser), */ /* Really on the barrel too */ > > I suggest to use "Tablet Pick" here. I added it to the kernel and use it for > the second barrel button on all of the tablets. It will map to the third > button. Certainly. Since this is a patch driver for Linux anyway, convention there takes priority. I don't know if other tablets also abuse eraser like this. > > /* The original usages should have mapped to mouse buttons, but Linux > > happily reports them as buttons 10-12. */ > > /* 0x05, 0x0f, / * Usage Page (Button), */ > > /* 0x19, 0x01, / * Usage Minimum (01h), */ > > /* 0x29, 0x03, / * Usage Maximum (03h), */ > > /* End of Stylus button usage */ > > I think having six usages and only three reports wouldn't work. Those were simply commented out versions of what the device originally sent. > > 0x15, 0x00, /* Logical Minimum (0), */ > > 0x25, 0x01, /* Logical Maximum (1), */ > > 0x95, 0x03, /* Report Count (3), */ > > 0x75, 0x01, /* Report Size (1), */ > > 0x81, 0x02, /* Input (Variable), */ > > 0x95, 0x05, /* Report Count (5), */ > > 0x81, 0x01, /* Input (Constant), */ > > 0x05, 0x01, /* Usage Page (Desktop), */ > > 0x09, 0x30, /* Usage (X), */ > > 0x09, 0x31, /* Usage (Y), */ > > 0x15, 0x00, /* Logical Minimum (0), */ > > 0x26, 0x00, 0x10, /* Logical Maximum (4096), */ > > 0x35, 0x00, /* Physical Minimum (0), */ > > 0x46, 0x00, 0x10, /* Physical Maximum (4096), */ > > Is the actual physical drawing area square? No. I believe it's meant to be 16:10 proportions. A quick measurement by the printed border shows: X: inner 103.65mm, outer 105mm Y: inner 65.6mm, outer 66.45mm This gives an aspect ratio somewhere from 1.56 to 1.60. Measuring the firmware limits would be a bit harder (for instance, it doesn't measure the tip but the coil position). I guess 105.6*66mm would be a reasonable approximation. I have tried to put this data into the attached descriptor. It would also be a little tricky to measure the unit of pressure, so I took the push/pop example from one of the uc-logic descriptors. The attached version uses pick, and thus reports (using input-events) stylus for the side button closer to the tip and stylus2 for the one further away. It was tested to work smoothly in inkscape and krita. Since it overrides the descriptor it doesn't care about the readout bugs, so it works on its own (but still requires the unbinding of hid-generic from the relevant interface). http://www.nybytt.se/zeniq-pen-tablet/ is a clear example of one of these tablets. Mine happen to have the Leogics brand instead. |
|
From: Yann V. <yan...@or...> - 2015-02-19 16:47:46
|
> The attached version uses pick, and thus reports (using input-events) stylus > for the side button closer to the tip and stylus2 for the one further away. Upon a moment more testing, I think those two usages might be better swapped, for consistency with the mouse mode. Or perhaps the mouse mode should have the swap for consistency with other tablets. Right now the buttons swap meaning between mouse and stylus modes. |
|
From: Nikolai K. <sp...@gm...> - 2015-02-22 12:23:15
|
On 02/19/2015 06:41 PM, Yann Vernier wrote: >> The attached version uses pick, and thus reports (using input-events) stylus >> for the side button closer to the tip and stylus2 for the one further away. > > Upon a moment more testing, I think those two usages might be better swapped, > for consistency with the mouse mode. Or perhaps the mouse mode should have the > swap for consistency with other tablets. Right now the buttons swap meaning > between mouse and stylus modes. I would prefer the latter. I.e. have the lower side button report middle click and the upper one report right click to match other tablets. Nick |
|
From: Nikolai K. <sp...@gm...> - 2015-02-22 12:22:03
|
On 02/19/2015 06:19 PM, Yann Vernier wrote: > On Sat, 31 Jan 2015 12:35:28 +0200 > Nikolai Kondrashov <sp...@gm...> wrote: >> I'm not sure how repeated report ID items in a single collection even work. >> Also duplicate report IDs sound fishy. All-in-all, this is some really weird >> report descriptor and would be difficult to reason about. I would start from >> scratch, taking one of the fixed report descriptors from other tablets and >> adjusting it to the bit fields this one reports. > > I think it's some sort of indirect description, applying resolution multipliers > to the preexisting report 1, so as not to confuse the boot protocol interpretation. I have my doubts about this. First, boot protocol doesn't involve parsing the report descriptor. Then, the device is unlikely to actually need a resolution multiplier. What for? > However, I have not gotten the device to send the multiplier reports, and Linux > seems to happily ignore them. That's probably just some imagination/engineering gone wild and never implemented. >>> /* I expect patching in these values (from interface 2) >>> would get Xorg to use tablet mode: */ >>> 0x09, 0x42, /* Usage (Tip Switch), */ >>> 0x09, 0x44, /* Usage (Barrel Switch), */ >>> 0x09, 0x45, /* Usage (Eraser), */ /* Really on the barrel too */ >> >> I suggest to use "Tablet Pick" here. I added it to the kernel and use it for >> the second barrel button on all of the tablets. It will map to the third >> button. > > Certainly. Since this is a patch driver for Linux anyway, convention there takes > priority. I don't know if other tablets also abuse eraser like this. Yes, quite a lot of them do that. >>> /* The original usages should have mapped to mouse buttons, but Linux >>> happily reports them as buttons 10-12. */ >>> /* 0x05, 0x0f, / * Usage Page (Button), */ >>> /* 0x19, 0x01, / * Usage Minimum (01h), */ >>> /* 0x29, 0x03, / * Usage Maximum (03h), */ >>> /* End of Stylus button usage */ >> >> I think having six usages and only three reports wouldn't work. > > Those were simply commented out versions of what the device originally sent. Ah, sorry, didn't notice that. >>> 0x15, 0x00, /* Logical Minimum (0), */ >>> 0x25, 0x01, /* Logical Maximum (1), */ >>> 0x95, 0x03, /* Report Count (3), */ >>> 0x75, 0x01, /* Report Size (1), */ >>> 0x81, 0x02, /* Input (Variable), */ >>> 0x95, 0x05, /* Report Count (5), */ >>> 0x81, 0x01, /* Input (Constant), */ >>> 0x05, 0x01, /* Usage Page (Desktop), */ >>> 0x09, 0x30, /* Usage (X), */ >>> 0x09, 0x31, /* Usage (Y), */ >>> 0x15, 0x00, /* Logical Minimum (0), */ >>> 0x26, 0x00, 0x10, /* Logical Maximum (4096), */ >>> 0x35, 0x00, /* Physical Minimum (0), */ >>> 0x46, 0x00, 0x10, /* Physical Maximum (4096), */ >> >> Is the actual physical drawing area square? > > No. I believe it's meant to be 16:10 proportions. > A quick measurement by the printed border shows: > X: inner 103.65mm, outer 105mm > Y: inner 65.6mm, outer 66.45mm > This gives an aspect ratio somewhere from 1.56 to 1.60. Measuring the firmware limits > would be a bit harder (for instance, it doesn't measure the tip but the coil position). > I guess 105.6*66mm would be a reasonable approximation. This would be awfully low resolution and most likely scaled by firmware for compatibility. The oldest tablet I have (UC-Logic WP8060U) has higher resolution, I think. There's very little chance a non-square tablet would report equal coordinate range for both axes in hardware. There might be a way to get it to report real ranges over USB. I would try poking at those features in the report descriptor. There should have been a driver for this tablet doing that. Perhaps we can find it and take a look at the USB traffic. We can postpone it for later, though. > I have tried to put this data into the attached descriptor. BTW, the original descriptor for interface 2 seems to have the correct range and physical extents. You can take the extents from there at least, which seem to be 4x2.5 inches. > It would also be a little tricky to measure the unit of pressure, so I took the > push/pop example from one of the uc-logic descriptors. Sure. It would also be mostly pointless. I don't think any manufacturer really calibrates them. There is no kernel support for communicating the resolution and people just use pressure curves to adjust the behavior as necessary anyway. > The attached version uses pick, and thus reports (using input-events) stylus > for the side button closer to the tip and stylus2 for the one further away. > > It was tested to work smoothly in inkscape and krita. Since it overrides the > descriptor it doesn't care about the readout bugs, so it works on its own (but > still requires the unbinding of hid-generic from the relevant interface). Great! Yes, you need rebinding for out-of-tree drivers anyway. That's just the way kernel works. It won't be necessary if it gets merged into the kernel. Speaking of which, I think you need to make the report descriptor as simple as possible to make it easier to get into the kernel. Strip it down to the absolute minimum required to work. Remove unused report IDs and collections. > http://www.nybytt.se/zeniq-pen-tablet/ is a clear example of one of these tablets. > Mine happen to have the Leogics brand instead. Ah, another rebranding. I have some comments to the code below. > /* > * HID driver for Polostar devices not fully compliant with HID standard > * > * Copyright (c) 2010 Nikolai Kondrashov There's no need to put my copyright on here. > * Copyright (c) 2015 Yann Vernier > */ > > /* > * This program is free software; you can redistribute it and/or modify it > * under the terms of the GNU General Public License as published by the Free > * Software Foundation; either version 2 of the License, or (at your option) > * any later version. > */ > > #include <linux/device.h> > #include <linux/hid.h> > #include <linux/module.h> > #include <linux/usb.h> > > #include "hid-ids.h" > /* Known as Zippy Technology Corp. in usbutils */ > #define USB_VENDOR_ID_POLOSTAR 0x099a > #define USB_DEVICE_ID_POLOSTAR_TABLET_PT1001 0x2620 > > /* Size of the original descriptor of PT-1001 tablets */ > #define PT1001_RDESC_ORIG_SIZE 317 > > /* Fixed PT1001 report descriptor */ > static __u8 pt1001_rdesc_fixed[] = { > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x02, /* Usage (Mouse), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x05, 0x09, /* Usage Page (Button), */ > /* Report ID 1 is used for mouse mode and scroll */ > 0x85, 0x01, /* Report ID (1), */ > 0x19, 0x01, /* Usage Minimum (01h), */ > 0x29, 0x05, /* Usage Maximum (05h), */ > 0x95, 0x05, /* Report Count (5), */ > 0x75, 0x01, /* Report Size (1), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x81, 0x02, /* Input (Variable), */ > 0x95, 0x03, /* Report Count (3), */ > 0x81, 0x01, /* Input (Constant), */ > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x01, /* Usage (Pointer), */ > 0xA1, 0x00, /* Collection (Physical), */ > 0x09, 0x30, /* Usage (X), */ > 0x09, 0x31, /* Usage (Y), */ > 0x95, 0x02, /* Report Count (2), */ > 0x75, 0x10, /* Report Size (16), */ > 0x16, 0x01, 0x80, /* Logical Minimum (-32767), */ > 0x26, 0xFF, 0x7F, /* Logical Maximum (32767), */ > 0x81, 0x06, /* Input (Variable, Relative), */ > 0xC0, /* End Collection, */ > /* The multipliers do not seem to occur as reports. */ > /* They seem to relate to vertical and horisontal scroll. */ > 0xA1, 0x02, /* Collection (Logical), */ > 0x85, 0x02, /* Report ID (2), */ > 0x09, 0x48, /* Usage (Resolution Multiplier), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x95, 0x01, /* Report Count (1), */ > 0x75, 0x02, /* Report Size (2), */ > 0xB1, 0x02, /* Feature (Variable), */ > 0x85, 0x01, /* Report ID (1), */ > 0x09, 0x38, /* Usage (Wheel), */ > 0x15, 0x81, /* Logical Minimum (-127), */ > 0x25, 0x7F, /* Logical Maximum (127), */ > 0x75, 0x08, /* Report Size (8), */ > 0x95, 0x01, /* Report Count (1), */ > 0x81, 0x06, /* Input (Variable, Relative), */ > 0xC0, /* End Collection, */ > 0xA1, 0x02, /* Collection (Logical), */ > 0x85, 0x02, /* Report ID (2), */ > 0x09, 0x48, /* Usage (Resolution Multiplier), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x75, 0x02, /* Report Size (2), */ > 0xB1, 0x02, /* Feature (Variable), */ > 0x75, 0x04, /* Report Size (4), */ > 0xB1, 0x01, /* Feature (Constant), */ > 0x85, 0x01, /* Report ID (1), */ > 0x05, 0x0C, /* Usage Page (Consumer), */ > 0x0A, 0x38, 0x02, /* Usage (AC Pan), */ > 0x15, 0x81, /* Logical Minimum (-127), */ > 0x25, 0x7F, /* Logical Maximum (127), */ > 0x75, 0x08, /* Report Size (8), */ > 0x95, 0x01, /* Report Count (1), */ > 0x81, 0x06, /* Input (Variable, Relative), */ > 0xC0, /* End Collection, */ > 0xC0, /* End Collection, */ You can replace the multiplier fields with constant fields, if they're unused. Also get rid of nested collections and repeated report IDs, if you can. > /* Report 3 looks like the raw protocol. Unknown how to enable. */ > 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ > 0x09, 0x01, /* Usage (01h), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x03, /* Report ID (3), */ > 0x19, 0x01, /* Usage Minimum (01h), */ > 0x29, 0x01, /* Usage Maximum (01h), */ > 0x95, 0x07, /* Report Count (7), */ > 0x81, 0x02, /* Input (Variable), */ > 0x19, 0x01, /* Usage Minimum (01h), */ > 0x29, 0x01, /* Usage Maximum (01h), */ > 0xB1, 0x02, /* Feature (Variable), */ > 0x09, 0x02, /* Usage (02h), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x26, 0xFF, 0x00, /* Logical Maximum (255), */ > 0x75, 0x08, /* Report Size (8), */ > 0x95, 0x01, /* Report Count (1), */ > 0x91, 0x02, /* Output (Variable), */ > 0xC0, /* End Collection, */ It's better to remove it, if it's unused. You can always add it back if you figure out a way to make it work. > /* Report ID 5 is used for some periphery buttons */ > 0x05, 0x0C, /* Usage Page (Consumer), */ > 0x09, 0x01, /* Usage (Consumer Control), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x05, /* Report ID (5), */ > 0x95, 0x01, /* Report Count (1), */ > 0x75, 0x08, /* Report Size (8), */ > 0x81, 0x01, /* Input (Constant), */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x75, 0x01, /* Report Size (1), */ > 0x95, 0x12, /* Report Count (18), */ > 0x0A, 0x83, 0x01, /* Usage (AL Consumer Control Config), */ > 0x0A, 0x8A, 0x01, /* Usage (AL Email Reader), */ > 0x0A, 0x92, 0x01, /* Usage (AL Calculator), */ > 0x0A, 0x94, 0x01, /* Usage (AL Local Machine Brwsr), */ > 0x0A, 0x21, 0x02, /* Usage (AC Search), */ > 0x0A, 0x23, 0x02, /* Usage (AC Home), */ > 0x0A, 0x24, 0x02, /* Usage (AC Back), */ > 0x0A, 0x25, 0x02, /* Usage (AC Forward), */ > 0x0A, 0x26, 0x02, /* Usage (AC Stop), */ > 0x0A, 0x27, 0x02, /* Usage (AC Refresh), */ > 0x0A, 0x2A, 0x02, /* Usage (AC Bookmarks), */ > 0x09, 0xB5, /* Usage (Scan Next Track), */ > 0x09, 0xB6, /* Usage (Scan Previous Track), */ > 0x09, 0xB7, /* Usage (Stop), */ > 0x09, 0xCD, /* Usage (Play Pause), */ > 0x09, 0xE2, /* Usage (Mute), */ > 0x09, 0xE9, /* Usage (Volume Inc), */ > 0x09, 0xEA, /* Usage (Volume Dec), */ > 0x81, 0x62, /* Input (Variable, No Preferred, Null State), */ > 0x95, 0x06, /* Report Count (6), */ > 0x75, 0x01, /* Report Size (1), */ > 0x81, 0x03, /* Input (Constant, Variable), */ > 0xC0, /* End Collection, */ > > /* Report 9 is the primary digitizer report. */ > 0x05, 0x0D, /* Usage Page (Digitizer), */ > 0x09, 0x01, /* Usage (Digitizer), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x09, /* Report ID (9), */ > 0x09, 0x20, /* Usage (Stylus), */ > 0xA1, 0x00, /* Collection (Physical), */ > > 0x09, 0x42, /* Usage (Tip Switch), */ > 0x09, 0x44, /* Usage (Barrel Switch), */ > 0x09, 0x46, /* Usage (Tablet Pick), */ /* Really on the barrel too */ > 0x15, 0x00, /* Logical Minimum (0), */ > 0x25, 0x01, /* Logical Maximum (1), */ > 0x95, 0x03, /* Report Count (3), */ > 0x75, 0x01, /* Report Size (1), */ > 0x81, 0x02, /* Input (Variable), */ > > 0x95, 0x05, /* Report Count (5), */ > 0x81, 0x01, /* Input (Constant), */ > > 0x05, 0x01, /* Usage Page (Desktop), */ If you put this usage page inside the push/pop pair, you won't need to restore the digitizer page explicitly later. > 0x15, 0x00, /* Logical Minimum (0), */ > 0x26, 0x00, 0x10, /* Logical Maximum (4096), */ > 0x75, 0x10, /* Report Size (16), */ > 0x95, 0x01, /* Report Count (1), */ > > 0xa4, /* Push */ > > 0x55, 0xfe, /* unit exponent -2: cm -> 0.1mm */ > 0x65, 0x11, /* SI linear centimeters */ > > 0x09, 0x30, /* Usage (X), */ > 0x35, 0x00, /* Physical Minimum (0), */ > 0x46, 0x20, 0x04, /* Physical Maximum (1056), */ > 0x81, 0x02, /* Input (Variable), */ > > 0x09, 0x31, /* Usage (Y), */ > 0x35, 0x00, /* Physical Minimum (0), */ There is no need to repeat physical minimum, but it might be easier to read this way. > 0x46, 0x94, 0x02, /* Physical Maximum (660), */ > 0x81, 0x02, /* Input (Variable), */ > > 0xb4, /* Pop (restore no unit) */ > > 0x05, 0x0D, /* Usage Page (Digitizer), */ > 0x09, 0x30, /* Usage (Tip Pressure), */ > 0x26, 0xFF, 0x03, /* Logical Maximum (1023), */ > 0x95, 0x01, /* Report Count (1), */ There's no need to repeat report count either. > 0x81, 0x02, /* Input (Variable), */ > 0xC0, /* End Collection, */ > 0xC0, /* End Collection */ > }; > > static __u8 *polostar_report_fixup(struct hid_device *hdev, __u8 *rdesc, > unsigned int *rsize) > { > struct usb_interface *iface = to_usb_interface(hdev->dev.parent); > __u8 iface_num = iface->cur_altsetting->desc.bInterfaceNumber; > > switch (hdev->product) { > case USB_DEVICE_ID_POLOSTAR_TABLET_PT1001: > if (iface_num==1 && *rsize == PT1001_RDESC_ORIG_SIZE) { There should be spaces around "==" according to the Linux Kernel Coding Style. > rdesc = pt1001_rdesc_fixed; > *rsize = sizeof(pt1001_rdesc_fixed); > } > break; > } > > return rdesc; > } > > static int polostar_probe(struct hid_device *hdev, const struct hid_device_id *id) > { > int rc; > > hdev->quirks |= id->driver_data; I would disable interface #2 here, if we can't make it work, with something like this: struct usb_interface *intf = to_usb_interface(hdev->dev.parent); if (intf->cur_altsetting->desc.bInterfaceNumber == 2) return -ENODEV; > rc = hid_parse(hdev); > if (rc) { > hid_err(hdev, "parse failed\n"); > return rc; > } > > rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT); > if (rc) { > hid_err(hdev, "hw start failed\n"); > return rc; > } > > return 0; > } > > static const struct hid_device_id polostar_devices[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_POLOSTAR, > USB_DEVICE_ID_POLOSTAR_TABLET_PT1001), > .driver_data = HID_QUIRK_MULTI_INPUT }, > { } > }; > MODULE_DEVICE_TABLE(hid, polostar_devices); > > static struct hid_driver polostar_driver = { > .name = "polostar", > .id_table = polostar_devices, > .probe = polostar_probe, > .report_fixup = polostar_report_fixup, > }; > module_driver(polostar_driver, hid_register_driver, hid_unregister_driver); > > MODULE_LICENSE("GPL"); > MODULE_VERSION("5"); BTW, can you make pull requests to digimend-kernel-drivers instead of sending files? Thank you. Nick |
|
From: Yann V. <yan...@or...> - 2015-03-07 19:35:29
|
Nikolai Kondrashov <sp...@gm...> wrote: > I would prefer the latter. I.e. have the lower side button report middle click > and the upper one report right click to match other tablets. Okay, doing that. > >> Is the actual physical drawing area square? > > > > No. I believe it's meant to be 16:10 proportions. > > A quick measurement by the printed border shows: > > X: inner 103.65mm, outer 105mm > > Y: inner 65.6mm, outer 66.45mm > > This gives an aspect ratio somewhere from 1.56 to 1.60. Measuring the firmware limits > > would be a bit harder (for instance, it doesn't measure the tip but the coil position). > > I guess 105.6*66mm would be a reasonable approximation. > > This would be awfully low resolution and most likely scaled by firmware for > compatibility. The oldest tablet I have (UC-Logic WP8060U) has higher > resolution, I think. There's very little chance a non-square tablet would > report equal coordinate range for both axes in hardware. There might be a way > to get it to report real ranges over USB. I would try poking at those features > in the report descriptor. There should have been a driver for this tablet > doing that. Perhaps we can find it and take a look at the USB traffic. We can > postpone it for later, though. It is definitely scaled by firmware; the non-square area is reported as a full 12 bit range. This works out to roughly 4096/(105.6/25.4)=985cpi. The soft buttons are outside of this reported area and handled entirely in firmware. Obviously the resolution runs out as this hits 4k monitors; quantisation errors may be notable before that. In mouse mode, it reports a higher resolution at 16 bits, but takes steps of about 7 counts, which works out to roughly 2200cpi. The tablet was designed to work with Windows default drivers, and did not come with any drivers. I have no idea how to request raw data out of it. > BTW, the original descriptor for interface 2 seems to have the correct range > and physical extents. You can take the extents from there at least, which seem > to be 4x2.5 inches. I find the HID encoding of units rather confusing. The descriptor seems to read 4x2.5 English inches, with a mistaken unit of cubic inches. If I have managed to figure out the system of HID correctly, the nibble value says what the exponent is for a unit in that position, so the inches should be 0x31. The only reason I didn't make the same mistake was that centimeters just happened to be written under column 1 in the nearly unreadable table. > Speaking of which, I think you need to make the report descriptor as simple as > possible to make it easier to get into the kernel. Strip it down to the > absolute minimum required to work. Remove unused report IDs and collections. Okay, stripping it down. > You can replace the multiplier fields with constant fields, if they're unused. > Also get rid of nested collections and repeated report IDs, if you can. I was mistaken about the resolution multiplier fields; the tablet really does use one of them for scrolling events. The other looks like it's horisontal scrolling, which there isn't a control for. > I would disable interface #2 here, if we can't make it work, with something > like this: Done. > BTW, can you make pull requests to digimend-kernel-drivers instead of sending > files? I think I may have figured it out. https://github.com/DIGImend/digimend-kernel-drivers/pull/11 |
|
From: Nikolai K. <sp...@gm...> - 2015-03-15 15:17:09
|
Hi Yann, On 03/07/2015 09:35 PM, Yann Vernier wrote: > Nikolai Kondrashov <sp...@gm...> wrote: > >> I would prefer the latter. I.e. have the lower side button report middle click >> and the upper one report right click to match other tablets. > > Okay, doing that. Great! >>>> Is the actual physical drawing area square? >>> >>> No. I believe it's meant to be 16:10 proportions. >>> A quick measurement by the printed border shows: >>> X: inner 103.65mm, outer 105mm >>> Y: inner 65.6mm, outer 66.45mm >>> This gives an aspect ratio somewhere from 1.56 to 1.60. Measuring the firmware limits >>> would be a bit harder (for instance, it doesn't measure the tip but the coil position). >>> I guess 105.6*66mm would be a reasonable approximation. >> >> This would be awfully low resolution and most likely scaled by firmware for >> compatibility. The oldest tablet I have (UC-Logic WP8060U) has higher >> resolution, I think. There's very little chance a non-square tablet would >> report equal coordinate range for both axes in hardware. There might be a way >> to get it to report real ranges over USB. I would try poking at those features >> in the report descriptor. There should have been a driver for this tablet >> doing that. Perhaps we can find it and take a look at the USB traffic. We can >> postpone it for later, though. > > It is definitely scaled by firmware; the non-square area is reported as a full > 12 bit range. This works out to roughly 4096/(105.6/25.4)=985cpi. The soft buttons > are outside of this reported area and handled entirely in firmware. > Obviously the resolution runs out as this hits 4k monitors; quantisation errors > may be notable before that. > > In mouse mode, it reports a higher resolution at 16 bits, but takes steps of > about 7 counts, which works out to roughly 2200cpi. > > The tablet was designed to work with Windows default drivers, and did not come > with any drivers. I have no idea how to request raw data out of it. Well, we'll have to do with that then. >> BTW, the original descriptor for interface 2 seems to have the correct range >> and physical extents. You can take the extents from there at least, which seem >> to be 4x2.5 inches. > > I find the HID encoding of units rather confusing. The descriptor seems > to read 4x2.5 English inches, with a mistaken unit of cubic inches. If I have > managed to figure out the system of HID correctly, the nibble value says what > the exponent is for a unit in that position, so the inches should be 0x31. > The only reason I didn't make the same mistake was that centimeters just > happened to be written under column 1 in the nearly unreadable table. Yes, the unit system is complicated and is not helped by the fact that the authors of the original HID descriptor tool have misinterpreted it, confusing all its users further. >> Speaking of which, I think you need to make the report descriptor as simple as >> possible to make it easier to get into the kernel. Strip it down to the >> absolute minimum required to work. Remove unused report IDs and collections. > > Okay, stripping it down. Thanks! >> You can replace the multiplier fields with constant fields, if they're unused. >> Also get rid of nested collections and repeated report IDs, if you can. > > I was mistaken about the resolution multiplier fields; the tablet really does > use one of them for scrolling events. The other looks like it's horisontal > scrolling, which there isn't a control for. > >> I would disable interface #2 here, if we can't make it work, with something >> like this: > > Done. > > >> BTW, can you make pull requests to digimend-kernel-drivers instead of sending >> files? > > I think I may have figured it out. > https://github.com/DIGImend/digimend-kernel-drivers/pull/11 Thanks, Yann. Reviewed now. Nick |
Thanks for helping keep SourceForge clean.
X