[Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Phil Dennis-Jordan]

 1. Set bInterfaceProtocol to 0x00 for usb-tablet. This should be non-zero for boot protocol devices only, which the usb-tablet is not.
 2. Set the usb-tablet's usage to "mouse" in the report descriptor.

The boot protocol of 0x02 specifically confused OS X/macOS' HID driver stack, causing it to generate additional bogus HID events with relative motion in addition to the tablet's absolute coordinate events.

Absolute pointing devices with HID Report Descriptor usage of 0x01 (pointing) are treated by the macOS HID driver as analog sticks, and absolute coordinates are not directly translated to absolute mouse cursor positions. Changing it to 0x02 (mouse) fixes the problem, and does not have any adverse effect in other operating systems and windowing systems. (VMWare does the same thing.)

Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
---

Changelog
=========

v2 -> v3:
 * v2 Thread was "[PATCH v2 0/2] hw/usb/dev-hid: Make usb-tablet work with OS X/macOS guests"
 * Changing the report descriptor to 0x02 (mouse) does not seem to have an adverse effect on any guests, so just change it for everyone instead of adding an option.
 * The 2 patches are now tiny, so combine them back into one.

v1 -> v2:
 * v1 Thread was "[PATCH] hw/usb/dev-hid: add a Mac guest compatibility option to usb-tablet"
 * Always apply the boot protocol (bInterfaceProtocol) change to usb-tablet, not just when the Mac compatibility option is active. The original value of 0x02 was determined to be incorrect according to the spec anyway.
 * As the boot protocol change is permanent, separate interface and device descriptor constants for the Mac/non-Mac variants of the tablet are no longer required, and have been removed.


 hw/usb/dev-hid.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 24d05f7..dda0bf0 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -144,7 +144,7 @@ static const USBDescIface desc_iface_tablet = {
     .bInterfaceNumber              = 0,
     .bNumEndpoints                 = 1,
     .bInterfaceClass               = USB_CLASS_HID,
-    .bInterfaceProtocol            = 0x02,
+    .bInterfaceProtocol            = 0x00,
     .ndesc                         = 1,
     .descs = (USBDescOther[]) {
         {
@@ -174,7 +174,7 @@ static const USBDescIface desc_iface_tablet2 = {
     .bInterfaceNumber              = 0,
     .bNumEndpoints                 = 1,
     .bInterfaceClass               = USB_CLASS_HID,
-    .bInterfaceProtocol            = 0x02,
+    .bInterfaceProtocol            = 0x00,
     .ndesc                         = 1,
     .descs = (USBDescOther[]) {
         {
@@ -487,7 +487,7 @@ static const uint8_t qemu_mouse_hid_report_descriptor[] = {
 
 static const uint8_t qemu_tablet_hid_report_descriptor[] = {
     0x05, 0x01,		/* Usage Page (Generic Desktop) */
-    0x09, 0x01,		/* Usage (Pointer) */
+    0x09, 0x02,		/* Usage (Mouse) */
     0xa1, 0x01,		/* Collection (Application) */
     0x09, 0x01,		/*   Usage (Pointer) */
     0xa1, 0x00,		/*   Collection (Physical) */
-- 
2.3.2 (Apple Git-55)

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet
Message-id: 1485365075-32702-1-git-send-email-phil@philjordan.eu

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1485297322-10595-1-git-send-email-mst@redhat.com -> patchew/1485297322-10595-1-git-send-email-mst@redhat.com
 * [new tag]         patchew/1485365075-32702-1-git-send-email-phil@philjordan.eu -> patchew/1485365075-32702-1-git-send-email-phil@philjordan.eu
 - [tag update]      patchew/20170124110151.937-1-berrange@redhat.com -> patchew/20170124110151.937-1-berrange@redhat.com
 - [tag update]      patchew/20170125121710.13561-1-cornelia.huck@de.ibm.com -> patchew/20170125121710.13561-1-cornelia.huck@de.ibm.com
Switched to a new branch 'test'
bdba2e5 hw/usb/dev-hid: Improve guest compatibility of usb-tablet

=== OUTPUT BEGIN ===
Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of usb-tablet...
ERROR: code indent should never use tabs
#43: FILE: hw/usb/dev-hid.c:490:
+    0x09, 0x02,^I^I/* Usage (Mouse) */$

total: 1 errors, 0 warnings, 24 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Phil Dennis-Jordan]

On 25 January 2017 at 18:27,  <no-reply@patchew.org> wrote:
> Your series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet
> Message-id: 1485365075-32702-1-git-send-email-phil@philjordan.eu
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of usb-tablet...
> ERROR: code indent should never use tabs
> #43: FILE: hw/usb/dev-hid.c:490:
> +    0x09, 0x02,^I^I/* Usage (Mouse) */$

Interestingly, the surrounding array initialisation already uses tabs,
so replacing them with spaces on only the line I edited seems wrong as
it'll mis-render in editors configured with a different tab width.
Please let me know if I need to take action on this issue, and if so
what to do. (I can add a whitespace-only patch to fix the surrounding
area, for example. Coding guidelines suggest this might not be
desirable though.)

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Fam Zheng]

On Wed, 01/25 18:36, Phil Dennis-Jordan wrote:
> On 25 January 2017 at 18:27,  <no-reply@patchew.org> wrote:
> > Your series seems to have some coding style problems. See output below for
> > more information:
> >
> > Type: series
> > Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet
> > Message-id: 1485365075-32702-1-git-send-email-phil@philjordan.eu
> >
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of usb-tablet...
> > ERROR: code indent should never use tabs
> > #43: FILE: hw/usb/dev-hid.c:490:
> > +    0x09, 0x02,^I^I/* Usage (Mouse) */$
> 
> Interestingly, the surrounding array initialisation already uses tabs,
> so replacing them with spaces on only the line I edited seems wrong as
> it'll mis-render in editors configured with a different tab width.
> Please let me know if I need to take action on this issue, and if so
> what to do. (I can add a whitespace-only patch to fix the surrounding
> area, for example. Coding guidelines suggest this might not be
> desirable though.)

File level tab consistency should override checkpatch.pl in this case, and as
you said whitespace patches are usually not advisable. Let's just ignore the
complain.

Fam

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Markus Armbruster]

Fam Zheng <famz@redhat.com> writes:

> On Wed, 01/25 18:36, Phil Dennis-Jordan wrote:
>> On 25 January 2017 at 18:27,  <no-reply@patchew.org> wrote:
>> > Your series seems to have some coding style problems. See output below for
>> > more information:
>> >
>> > Type: series
>> > Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet
>> > Message-id: 1485365075-32702-1-git-send-email-phil@philjordan.eu
>> >
>> > === OUTPUT BEGIN ===
>> > Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of usb-tablet...
>> > ERROR: code indent should never use tabs
>> > #43: FILE: hw/usb/dev-hid.c:490:
>> > +    0x09, 0x02,^I^I/* Usage (Mouse) */$
>> 
>> Interestingly, the surrounding array initialisation already uses tabs,
>> so replacing them with spaces on only the line I edited seems wrong as
>> it'll mis-render in editors configured with a different tab width.
>> Please let me know if I need to take action on this issue, and if so
>> what to do. (I can add a whitespace-only patch to fix the surrounding
>> area, for example. Coding guidelines suggest this might not be
>> desirable though.)
>
> File level tab consistency should override checkpatch.pl in this case, and as
> you said whitespace patches are usually not advisable. Let's just ignore the
> complain.

Ignoring is okay here.  Expanding the tabs would also be okay, and we
routinely do that when we touch lines.

With QEMU sources, you can have any tab width, as long as it is eight.

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Fam Zheng]

On Thu, 01/26 07:29, Markus Armbruster wrote:
> With QEMU sources, you can have any tab width, as long as it is eight.

That is very permissive!

Fam

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Gerd Hoffmann]

On Mi, 2017-01-25 at 18:36 +0100, Phil Dennis-Jordan wrote:
> On 25 January 2017 at 18:27,  <no-reply@patchew.org> wrote:
> > Your series seems to have some coding style problems. See output below for
> > more information:
> >
> > Type: series
> > Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet
> > Message-id: 1485365075-32702-1-git-send-email-phil@philjordan.eu
> >
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of usb-tablet...
> > ERROR: code indent should never use tabs
> > #43: FILE: hw/usb/dev-hid.c:490:
> > +    0x09, 0x02,^I^I/* Usage (Mouse) */$
> 
> Interestingly, the surrounding array initialisation already uses tabs,
> so replacing them with spaces on only the line I edited seems wrong as
> it'll mis-render in editors configured with a different tab width.
> Please let me know if I need to take action on this issue, and if so
> what to do. (I can add a whitespace-only patch to fix the surrounding
> area, for example. Coding guidelines suggest this might not be
> desirable though.)

I'll happily take the patch as-is.

thanks,
  Gerd