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
