Re: [PATCH] Driver for Bosto 14WA graphics tablet

2015-10-05 Thread Benjamin Tissoires
Hi Leslie,

thanks for your submission.

As a foreword, I must say that you want to review your sending
process. Your email client stripped all the spaces/tabs and we can not
accept such a patch. I would encourage you to directly use the command
"git send-email" to send patches.

Little bit more inlined:

On Sun, Oct 4, 2015 at 8:51 AM, Leslie Viljoen  wrote:
> Add support for the Bosto 14WA graphics tablet. This is originally based
> off the hanwang.c tablet driver since the chip is similar.
>
> I added what I think are the right entries to quirks to make HID ignore
> the tablet - so that this driver can load. I'm still working on getting a
> proper kernel dev VM so I've not tested that part. But the change is
> trivial.
>
> checkpatch.pl said I may need to edit MAINTAINERS because of the
> new file but I don't see anything about that in the SubmittingPatches
> doc. Do I need to do that?

Ideally you do. This is mainly to receive patches from others when
they are sending patches against your driver. This way, you can be
sure not to break your current device. On a day to day basis, we do
not update it as often as we should, and this is a little bit of a
shame :)

>
> I'm a novice, apologies for any mistakes.
>
> Signed-off-by: Leslie Viljoen 
>
> ---
>  drivers/hid/hid-ids.h |   1 +
>  drivers/hid/usbhid/hid-quirks.c   |   2 +
>  drivers/input/tablet/bosto_14wa.c | 555 
> ++

Is there any reasons not to use a hid driver directly?

>From what I can read of the code, it looks like your device is
directly sending the correct events, you just need a special way of
parsing them.

There are several advantages not to write a full usb driver: you only
focus on the parsing of the events, you do not need to handle
suspend/resume, you can use your driver on older kernels with some
good looking udev rules and a sideload of your driver.

drivers/hid/hid-primax.c is a driver with just a raw_event processing,
and you might want to use it as a basis.

Cheers,
Benjamin

>  3 files changed, 558 insertions(+)
>  create mode 100644 drivers/input/tablet/bosto_14wa.c
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index f769208..77ec24a 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -450,6 +450,7 @@
>  #define USB_VENDOR_ID_HANWANG 0x0b57
>  #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
>  #define USB_DEVICE_ID_HANWANG_TABLET_LAST 0x8fff
> +#define USB_DEVICE_ID_BOSTO14WA_2 0x9018
>
>  #define USB_VENDOR_ID_HANVON 0x20b3
>  #define USB_DEVICE_ID_HANVON_MULTITOUCH 0x0a18
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 1dff8f0..840b5d7 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -132,6 +132,8 @@ static const struct hid_blacklist {
>
>   { USB_VENDOR_ID_PI_ENGINEERING,
> USB_DEVICE_ID_PI_ENGINEERING_VEC_USB_FOOTPEDAL,
> HID_QUIRK_HIDINPUT_FORCE },
>
> + { USB_VENDOR_ID_HANWANG, USB_DEVICE_ID_BOSTO14WA_2, HID_QUIRK_IGNORE },
> +
>   { USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_MULTI_TOUCH,
> HID_QUIRK_MULTI_INPUT },
>   { USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_WIRELESS,
> HID_QUIRK_MULTI_INPUT },
>   { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD,
> HID_QUIRK_NO_INIT_REPORTS },
> diff --git a/drivers/input/tablet/bosto_14wa.c
> b/drivers/input/tablet/bosto_14wa.c
> new file mode 100644
> index 000..64f1d71
> --- /dev/null
> +++ b/drivers/input/tablet/bosto_14wa.c
> @@ -0,0 +1,555 @@
> +/*
> + *  USB Bosto tablet support
> + *
> + *  Original Copyright (c) 2010 Xing Wei 
> + *
> + */
> +
> +/*
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRIVER_DESC "USB Bosto(2nd Gen) tablet driver"
> +#define DRIVER_LICENSE  "GPL"
> +
> +MODULE_AUTHOR("Xing Wei ");
> +MODULE_AUTHOR("Aidan Walton ");
> +MODULE_AUTHOR("Leslie Viljoen ");
> +MODULE_AUTHOR("Tomasz Flis ");
> +
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_LICENSE(DRIVER_LICENSE);
> +
> +#define USB_VENDOR_ID_HANWANG 0x0b57
> +#define USB_DEVICE_ID_BOSTO14WA_2 0x9018
> +
> +#define BOSTO_TABLET_INT_CLASS 0x0003
> +#define BOSTO_TABLET_INT_SUB_CLASS 0x0001
> +#define 

Re: [PATCH] Driver for Bosto 14WA graphics tablet

2015-10-05 Thread Leslie Viljoen
On Mon, Oct 5, 2015 at 10:19 PM, Benjamin Tissoires
 wrote:
> accept such a patch. I would encourage you to directly use the command
> "git send-email" to send patches.

Great, I didn't realise this existed.

> Ideally you do. This is mainly to receive patches from others when
> they are sending patches against your driver. This way, you can be
> sure not to break your current device. On a day to day basis, we do
> not update it as often as we should, and this is a little bit of a
> shame :)

Ok, I will update it.

> drivers/hid/hid-primax.c is a driver with just a raw_event processing,
> and you might want to use it as a basis.

Thanks for the feedback, I'll take a look at that! So far I'd just been
extrapolating from the Hanwang driver and was not aware of other
possibilities!
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html