| Message ID | 1485365075-32702-1-git-send-email-phil@philjordan.eu |
|---|---|
| State | New |
| Headers | show |
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
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.)
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
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.
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
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
========= 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) */
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