| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150225215130.GA4404@mail.corp.redhat.com>
Date: Wed, 25 Feb 2015 16:51:31 -0500
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Ping Cheng <pinglinux@...il.com>
Cc: josep.sanchez.ferreres@....fib.upc.edu,
ACHARD.Frederic.frederic.achard@...e.fr,
Jiri Kosina <jkosina@...e.cz>,
Jason Gerecke <killertofu@...il.com>,
linux-input <linux-input@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] HID: wacom: add full support of the Wacom Bamboo
PAD
On Feb 25 2015 or thereabouts, Ping Cheng wrote:
> On Wed, Feb 25, 2015 at 8:43 AM, Benjamin Tissoires
> <benjamin.tissoires@...hat.com> wrote:
> > The stylus of this device works just fine out of the box.
> > The touch is seen by default as a mouse with relative events and some
> > gestures.
> > The wireless and the wired version have slightly different firmwares, but
> > the debug mode 2 on the feature 2 is common to the 2 devices. In this mode,
> > all the reports are emitted through the debug interface (pen, raw touch
> > and mouse emulation), so we have to re-route manually the events.
> >
> > We keep the Pen interface as a HID_GENERIC one because it works, and only
> > parse the raw touches while discarding the mouse emulation & gestures.
> >
> > Switching the default in raw mode allows us to have a consistent user
> > experience accross all the multitouch touchpads (and enable the touch part
> > of the devices).
> >
> > Note that the buttons of this devices are reported through the touch
> > interface. There is no 'Pad' interface. It seemed more natural to have
> > the BTN_LEFT and BTN_RIGHT reported with the touch because they are
> > placed under the touch interface and it looks like they belong to the
> > touch part.
> >
> > Tested-by: Josep Sanchez Ferreres <josep.sanchez.ferreres@....fib.upc.edu>
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> > ---
> > drivers/hid/wacom_sys.c | 24 ++++++++++++
> > drivers/hid/wacom_wac.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++
> > drivers/hid/wacom_wac.h | 5 +++
> > 3 files changed, 129 insertions(+)
> >
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index b3c2395..957699f 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -406,6 +406,9 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
> > else if (features->type == WACOM_27QHDT) {
> > return wacom_set_device_mode(hdev, 131, 3, 2);
> > }
> > + else if (features->type == BAMBOO_PAD) {
> > + return wacom_set_device_mode(hdev, 2, 2, 2);
> > + }
> > } else if (features->device_type == BTN_TOOL_PEN) {
> > if (features->type <= BAMBOO_PT && features->type != WIRELESS) {
> > return wacom_set_device_mode(hdev, 2, 2, 2);
> > @@ -1425,6 +1428,21 @@ static int wacom_probe(struct hid_device *hdev,
> > goto fail_allocate_inputs;
> > }
> >
> > + /*
> > + * Bamboo Pad has a generic hid handling for the Pen, and we switch it
> > + * into debug mode for the touch part.
> > + * We ignore the other interfaces.
> > + */
> > + if (features->type == BAMBOO_PAD) {
> > + if (features->pktlen == WACOM_PKGLEN_PENABLED) {
> > + features->type = HID_GENERIC;
> > + } else if ((features->pktlen != WACOM_PKGLEN_BPAD_TOUCH) &&
> > + (features->pktlen != WACOM_PKGLEN_BPAD_TOUCH_USB)) {
> > + error = -ENODEV;
> > + goto fail_shared_data;
> > + }
> > + }
> > +
> > /* set the default size in case we do not get them from hid */
> > wacom_set_default_phy(features);
> >
> > @@ -1459,6 +1477,12 @@ static int wacom_probe(struct hid_device *hdev,
> > features->y_max = 4096;
> > }
> >
> > + /*
> > + * Same thing for Bamboo PAD
> > + */
> > + if (features->type == BAMBOO_PAD)
> > + features->device_type = BTN_TOOL_FINGER;
> > +
> > if (hdev->bus == BUS_BLUETOOTH)
> > features->quirks |= WACOM_QUIRK_BATTERY;
> >
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index 16e8d28..ff693a6 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -1826,6 +1826,87 @@ static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
> > return 0;
> > }
> >
> > +static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
> > + unsigned char *data)
> > +{
> > + unsigned char prefix;
> > +
> > + /*
> > + * We need to reroute the event from the debug interface to the
> > + * pen interface.
> > + * We need to add the report ID to the actual pen report, so we
> > + * temporary overwrite the first byte to prevent having to kzalloc/kfree
> > + * and memcpy the report.
> > + */
> > + prefix = data[0];
> > + data[0] = WACOM_REPORT_BPAD_PEN;
> > +
> > + /*
> > + * actually reroute the event.
> > + * No need to check if wacom->shared->pen is valid, hid_input_report()
> > + * will check for us.
> > + */
> > + hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data,
> > + WACOM_PKGLEN_PENABLED, 1);
> > +
> > + data[0] = prefix;
> > +}
> > +
> > +static int wacom_bamboo_pad_touch_event(struct wacom_wac *wacom,
> > + unsigned char *data)
> > +{
> > + struct input_dev *input = wacom->input;
> > + unsigned char *finger_data, prefix;
> > + unsigned id;
> > + int x, y;
> > + bool valid;
> > +
> > + prefix = data[0];
> > +
> > + for (id = 0; id < wacom->features.touch_max; id++) {
> > + valid = !!(prefix & BIT(id)) &&
> > + !wacom->shared->stylus_in_proximity;
>
> stylus_in_proximity should be updated in wacom_bamboo_pad_pen_event()
> for the above statement to be sensible.
>
Hehe, it is actually. When we receive a pen event, we forward it to the
pen hid interface, which processes it eventually through
wacom_wac_pen_report(), which sets the field :)
> > +
> > + input_mt_slot(input, id);
> > + input_mt_report_slot_state(input, MT_TOOL_FINGER, valid);
> > +
> > + if (!valid)
> > + continue;
> > +
> > + finger_data = data + 1 + id * 3;
> > + x = finger_data[0] | ((finger_data[1] & 0x0f) << 8);
> > + y = (finger_data[2] << 4) | (finger_data[1] >> 4);
> > +
> > + input_report_abs(input, ABS_MT_POSITION_X, x);
> > + input_report_abs(input, ABS_MT_POSITION_Y, y);
> > + }
> > +
> > + input_mt_sync_frame(input);
> > +
> > + input_report_key(input, BTN_LEFT, prefix & 0x40);
> > + input_report_key(input, BTN_RIGHT, prefix & 0x80);
>
> We need to set touch_down here so pen events won't be posted before
> touch is forced up.
Oh, right. I did not noticed it when testing, but that would be better.
>
> Everything else looks good to me.
Thanks!
v3 is on its way (tomorrow actually)
Cheers,
Benjamin
>
> Thanks,
>
> Ping
>
> > +
> > + return 1;
> > +}
> > +
> > +static int wacom_bamboo_pad_irq(struct wacom_wac *wacom, size_t len)
> > +{
> > + unsigned char *data = wacom->data;
> > +
> > + if (!((len == WACOM_PKGLEN_BPAD_TOUCH) ||
> > + (len == WACOM_PKGLEN_BPAD_TOUCH_USB)) ||
> > + (data[0] != WACOM_REPORT_BPAD_TOUCH))
> > + return 0;
> > +
> > + if (data[1] & 0x01)
> > + wacom_bamboo_pad_pen_event(wacom, &data[1]);
> > +
> > + if (data[1] & 0x02)
> > + return wacom_bamboo_pad_touch_event(wacom, &data[9]);
> > +
> > + return 0;
> > +}
> > +
> > static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
> > {
> > unsigned char *data = wacom->data;
> > @@ -1962,6 +2043,10 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> > sync = wacom_bpt_irq(wacom_wac, len);
> > break;
> >
> > + case BAMBOO_PAD:
> > + sync = wacom_bamboo_pad_irq(wacom_wac, len);
> > + break;
> > +
> > case WIRELESS:
> > sync = wacom_wireless_irq(wacom_wac, len);
> > break;
> > @@ -2300,6 +2385,13 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
> > 0, 0);
> > }
> > break;
> > + case BAMBOO_PAD:
> > + __clear_bit(ABS_MISC, input_dev->absbit);
> > + input_mt_init_slots(input_dev, features->touch_max,
> > + INPUT_MT_POINTER);
> > + __set_bit(BTN_LEFT, input_dev->keybit);
> > + __set_bit(BTN_RIGHT, input_dev->keybit);
> > + break;
> > }
> > return 0;
> > }
> > @@ -2953,6 +3045,12 @@ static const struct wacom_features wacom_features_0x30C =
> > { "Wacom ISDv5 30C", .type = WACOM_24HDT, /* Touch */
> > .oVid = USB_VENDOR_ID_WACOM, .oPid = 0x30A, .touch_max = 10,
> > .check_for_hid_type = true, .hid_type = HID_TYPE_USBNONE };
> > +static const struct wacom_features wacom_features_0x318 =
> > + { "Wacom USB Bamboo PAD", 4095, 4095, /* Touch */
> > + .type = BAMBOO_PAD, 35, 48, .touch_max = 4 };
> > +static const struct wacom_features wacom_features_0x319 =
> > + { "Wacom Wireless Bamboo PAD", 4095, 4095, /* Touch */
> > + .type = BAMBOO_PAD, 35, 48, .touch_max = 4 };
> > static const struct wacom_features wacom_features_0x323 =
> > { "Wacom Intuos P M", 21600, 13500, 1023, 31,
> > INTUOSHT, WACOM_INTUOS_RES, WACOM_INTUOS_RES,
> > @@ -3105,6 +3203,8 @@ const struct hid_device_id wacom_ids[] = {
> > { USB_DEVICE_WACOM(0x314) },
> > { USB_DEVICE_WACOM(0x315) },
> > { USB_DEVICE_WACOM(0x317) },
> > + { USB_DEVICE_WACOM(0x318) },
> > + { USB_DEVICE_WACOM(0x319) },
> > { USB_DEVICE_WACOM(0x323) },
> > { USB_DEVICE_WACOM(0x32A) },
> > { USB_DEVICE_WACOM(0x32B) },
> > diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> > index e42efbe..a3d0828 100644
> > --- a/drivers/hid/wacom_wac.h
> > +++ b/drivers/hid/wacom_wac.h
> > @@ -33,6 +33,8 @@
> > #define WACOM_PKGLEN_MTTPC 40
> > #define WACOM_PKGLEN_DTUS 68
> > #define WACOM_PKGLEN_PENABLED 8
> > +#define WACOM_PKGLEN_BPAD_TOUCH 32
> > +#define WACOM_PKGLEN_BPAD_TOUCH_USB 64
> >
> > /* wacom data size per MT contact */
> > #define WACOM_BYTES_PER_MT_PACKET 11
> > @@ -67,6 +69,8 @@
> > #define WACOM_REPORT_24HDT 1
> > #define WACOM_REPORT_WL 128
> > #define WACOM_REPORT_USB 192
> > +#define WACOM_REPORT_BPAD_PEN 3
> > +#define WACOM_REPORT_BPAD_TOUCH 16
> >
> > /* device quirks */
> > #define WACOM_QUIRK_MULTI_INPUT 0x0001
> > @@ -122,6 +126,7 @@ enum {
> > BAMBOO_PT,
> > WACOM_24HDT,
> > WACOM_27QHDT,
> > + BAMBOO_PAD,
> > TABLETPC, /* add new TPC below */
> > TABLETPCE,
> > TABLETPC2FG,
> > --
> > 2.1.0
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists