Re: [PATCH 2/4] HID: viewsonic: Support PD1011 signature pad

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux