Hi Nick, On Thu, Dec 27, 2018 at 6:17 PM Nikolai Kondrashov <spbnick@xxxxxxxxx> wrote: > > Add support for ViewSonic PD1011 signature (display) pad, which is also > sold by Signotec under a different name. > > Signed-off-by: Nikolai Kondrashov <spbnick@xxxxxxxxx> > --- > drivers/hid/Kconfig | 6 ++ > drivers/hid/Makefile | 1 + > drivers/hid/hid-ids.h | 6 ++ > drivers/hid/hid-quirks.c | 4 ++ > drivers/hid/hid-viewsonic.c | 127 ++++++++++++++++++++++++++++++++++++ > 5 files changed, 144 insertions(+) > create mode 100644 drivers/hid/hid-viewsonic.c > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 41e9935fc584..4e4cacf85cbd 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -412,6 +412,12 @@ config HID_WALTOP > ---help--- > Support for Waltop tablets. > > +config HID_VIEWSONIC > + tristate "ViewSonic/Signotec" > + depends on HID > + help > + Support for ViewSonic/Signotec PD1011 signature pad. > + > config HID_GYRATION > tristate "Gyration remote control" > depends on HID > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index 896a51ce7ce0..a57e1088133a 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -114,6 +114,7 @@ obj-$(CONFIG_HID_LED) += hid-led.o > obj-$(CONFIG_HID_XINMO) += hid-xinmo.o > obj-$(CONFIG_HID_ZEROPLUS) += hid-zpff.o > obj-$(CONFIG_HID_ZYDACRON) += hid-zydacron.o > +obj-$(CONFIG_HID_VIEWSONIC) += hid-viewsonic.o > > wacom-objs := wacom_wac.o wacom_sys.o > obj-$(CONFIG_HID_WACOM) += wacom.o > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 640ba1a37b12..19f62c187e9c 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -1235,4 +1235,10 @@ > #define USB_VENDOR_ID_UGTIZER 0x2179 > #define USB_DEVICE_ID_UGTIZER_TABLET_GP0610 0x0053 > > +#define USB_VENDOR_ID_VIEWSONIC 0x0543 > +#define USB_DEVICE_ID_VIEWSONIC_PD1011 0xe621 > + > +#define USB_VENDOR_ID_SIGNOTEC 0x2133 > +#define USB_DEVICE_ID_SIGNOTEC_VIEWSONIC_PD1011 0x0018 > + > #endif > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c > index 23e7d7eda27c..9641a556822c 100644 > --- a/drivers/hid/hid-quirks.c > +++ b/drivers/hid/hid-quirks.c > @@ -714,6 +714,10 @@ static const struct hid_device_id hid_have_special_driver[] = { > #endif > #if IS_ENABLED(CONFIG_HID_ZYDACRON) > { HID_USB_DEVICE(USB_VENDOR_ID_ZYDACRON, USB_DEVICE_ID_ZYDACRON_REMOTE_CONTROL) }, > +#endif > +#if IS_ENABLED(CONFIG_HID_VIEWSONIC) > + { HID_USB_DEVICE(USB_VENDOR_ID_VIEWSONIC, USB_DEVICE_ID_VIEWSONIC_PD1011) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_SIGNOTEC, USB_DEVICE_ID_SIGNOTEC_VIEWSONIC_PD1011) }, Same comment than in my previous patch, this hunk should be dropped. > #endif > { } > }; > diff --git a/drivers/hid/hid-viewsonic.c b/drivers/hid/hid-viewsonic.c > new file mode 100644 > index 000000000000..d4c5bb5604a8 > --- /dev/null > +++ b/drivers/hid/hid-viewsonic.c > @@ -0,0 +1,127 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * HID driver for ViewSonic devices not fully compliant with HID standard > + * > + * Copyright (c) 2017 Nikolai Kondrashov > + */ > + > +/* > + * 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> Please don't. When replaying the same HID descriptor over uhid, we will get a crash, and this is not something we want. further review later: usb is not even used by the driver, so yes, we should drop it. > + > +#include "hid-ids.h" > + > +/* Size of the original descriptor of PD1011 signature pad */ > +#define PD1011_RDESC_ORIG_SIZE 408 > + > +/* Fixed report descriptor of PD1011 signature pad */ > +static __u8 pd1011_rdesc_fixed[] = { > + 0x05, 0x0D, /* Usage Page (Digitizer), */ > + 0x09, 0x02, /* Usage (Pen), */ > + 0xA1, 0x01, /* Collection (Application), */ > + 0x85, 0x02, /* Report ID (2), */ > + 0x09, 0x20, /* Usage (Stylus), */ > + 0xA0, /* Collection (Physical), */ > + 0x75, 0x10, /* Report Size (16), */ > + 0x95, 0x01, /* Report Count (1), */ > + 0xA4, /* Push, */ > + 0x05, 0x01, /* Usage Page (Desktop), */ > + 0x65, 0x13, /* Unit (Inch), */ > + 0x55, 0xFD, /* Unit Exponent (-3), */ > + 0x34, /* Physical Minimum (0), */ > + 0x09, 0x30, /* Usage (X), */ > + 0x46, 0x5D, 0x21, /* Physical Maximum (8541), */ > + 0x27, 0x80, 0xA9, > + 0x00, 0x00, /* Logical Maximum (43392), */ > + 0x81, 0x02, /* Input (Variable), */ > + 0x09, 0x31, /* Usage (Y), */ > + 0x46, 0xDA, 0x14, /* Physical Maximum (5338), */ > + 0x26, 0xF0, 0x69, /* Logical Maximum (27120), */ > + 0x81, 0x02, /* Input (Variable), */ > + 0xB4, /* Pop, */ > + 0x14, /* Logical Minimum (0), */ > + 0x25, 0x01, /* Logical Maximum (1), */ > + 0x75, 0x01, /* Report Size (1), */ > + 0x95, 0x01, /* Report Count (1), */ > + 0x81, 0x03, /* Input (Constant, Variable), */ > + 0x09, 0x32, /* Usage (In Range), */ > + 0x09, 0x42, /* Usage (Tip Switch), */ > + 0x95, 0x02, /* Report Count (2), */ > + 0x81, 0x02, /* Input (Variable), */ > + 0x95, 0x05, /* Report Count (5), */ > + 0x81, 0x03, /* Input (Constant, Variable), */ > + 0x75, 0x10, /* Report Size (16), */ > + 0x95, 0x01, /* Report Count (1), */ > + 0x09, 0x30, /* Usage (Tip Pressure), */ > + 0x15, 0x05, /* Logical Minimum (5), */ > + 0x26, 0xFF, 0x07, /* Logical Maximum (2047), */ > + 0x81, 0x02, /* Input (Variable), */ > + 0x75, 0x10, /* Report Size (16), */ > + 0x95, 0x01, /* Report Count (1), */ > + 0x81, 0x03, /* Input (Constant, Variable), */ > + 0xC0, /* End Collection, */ > + 0xC0 /* End Collection */ > +}; > + > +static __u8 *viewsonic_report_fixup(struct hid_device *hdev, __u8 *rdesc, > + unsigned int *rsize) > +{ > + switch (hdev->product) { > + case USB_DEVICE_ID_VIEWSONIC_PD1011: > + case USB_DEVICE_ID_SIGNOTEC_VIEWSONIC_PD1011: > + if (*rsize == PD1011_RDESC_ORIG_SIZE) { > + rdesc = pd1011_rdesc_fixed; > + *rsize = sizeof(pd1011_rdesc_fixed); > + } > + break; > + } > + > + return rdesc; > +} > + > +static int viewsonic_probe(struct hid_device *hdev, > + const struct hid_device_id *id) > +{ > + int rc; > + > + 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; unless I am missing something, the .probe() has nothing special and could be dropped. > +} > + > +static const struct hid_device_id viewsonic_devices[] = { > + { HID_USB_DEVICE(USB_VENDOR_ID_VIEWSONIC, > + USB_DEVICE_ID_VIEWSONIC_PD1011) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_SIGNOTEC, > + USB_DEVICE_ID_SIGNOTEC_VIEWSONIC_PD1011) }, > + { } > +}; > +MODULE_DEVICE_TABLE(hid, viewsonic_devices); > + > +static struct hid_driver viewsonic_driver = { > + .name = "viewsonic", > + .id_table = viewsonic_devices, > + .probe = viewsonic_probe, > + .report_fixup = viewsonic_report_fixup, > +}; > +module_hid_driver(viewsonic_driver); > + > +MODULE_LICENSE("GPL"); > -- > 2.19.2 > Cheers, Benjamin