Re: [PATCH 1/2] hid-multitouch: work on default classes and renamingof classes

From: Henrik Rydberg
Date: Fri Jan 28 2011 - 12:19:04 EST


Hi Benjamin,

> The safest quirk for a device (the one that works out of the box for
> most of them) is the MT_QUIRK_NOT_SEEN_MEANS_UP. Indeed, it does not
> make any assumption on the device. When adding a new device, we can
> easily test it against MT_CLS_DEFAULT, and then optimize it with other
> quirks: that's why no device use MT_CLS_DEFAULT right now.
>
> This patch introduce also MT_CLS_DUAL_DEFAULT which has the same purpose
> than MT_CLS_DEFAULT, but for dual touch panels.

But it is used anywhere?

> Finally, the patch renames MT_CLS_DUAL1 to MT_CLS_DUAL_INRANGE_CONTACTID
> and MT_CLS_DUAL2 to MT_CLS_DUAL_INRANGE_CONTACTNUMBER for better
> readability.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxx>
> ---

The patch description and the content lacks a certain
distinctness. Please single out janitory actions into a separate
patch, or, if possible, skip it altogether.

> drivers/hid/hid-multitouch.c | 25 +++++++++++++++----------
> 1 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 07d3183..ffe2a76 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -24,6 +24,7 @@
>
>
> MODULE_AUTHOR("Stephane Chatty <chatty@xxxxxxx>");
> +MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx>");

Adding this is fine, based on your prior contributions. Perhaps it
should be motivated as such in a separate patch instead.

> MODULE_DESCRIPTION("HID multitouch panels");
> MODULE_LICENSE("GPL");
>
> @@ -65,10 +66,11 @@ struct mt_class {
> };
>
> /* classes of device behavior */
> -#define MT_CLS_DEFAULT 1
> -#define MT_CLS_DUAL1 2
> -#define MT_CLS_DUAL2 3
> -#define MT_CLS_CYPRESS 4
> +#define MT_CLS_DEFAULT 1
> +#define MT_CLS_DUAL_DEFAULT 2
> +#define MT_CLS_DUAL_INRANGE_CONTACTID 3
> +#define MT_CLS_DUAL_INRANGE_CONTACTNUMBER 4
> +#define MT_CLS_CYPRESS 10

There is no need to change the numbering for unchanged names.

>
> /*
> * these device-dependent functions determine what slot corresponds
> @@ -104,13 +106,16 @@ static int find_slot_from_contactid(struct mt_device *td)
>
> struct mt_class mt_classes[] = {
> { .name = MT_CLS_DEFAULT,
> - .quirks = MT_QUIRK_VALID_IS_INRANGE,
> + .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP,
> .maxcontacts = 10 },
> - { .name = MT_CLS_DUAL1,
> + { .name = MT_CLS_DUAL_DEFAULT,
> + .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP,
> + .maxcontacts = 2 },
> + { .name = MT_CLS_DUAL_INRANGE_CONTACTID,
> .quirks = MT_QUIRK_VALID_IS_INRANGE |
> MT_QUIRK_SLOT_IS_CONTACTID,
> .maxcontacts = 2 },
> - { .name = MT_CLS_DUAL2,
> + { .name = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
> .quirks = MT_QUIRK_VALID_IS_INRANGE |
> MT_QUIRK_SLOT_IS_CONTACTNUMBER,
> .maxcontacts = 2 },
> @@ -466,15 +471,15 @@ static const struct hid_device_id mt_devices[] = {
> USB_DEVICE_ID_CYPRESS_TRUETOUCH) },
>
> /* GeneralTouch panel */
> - { .driver_data = MT_CLS_DUAL2,
> + { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
> HID_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH,
> USB_DEVICE_ID_GENERAL_TOUCH_WIN7_TWOFINGERS) },
>
> /* PixCir-based panels */
> - { .driver_data = MT_CLS_DUAL1,
> + { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTID,
> HID_USB_DEVICE(USB_VENDOR_ID_HANVON,
> USB_DEVICE_ID_HANVON_MULTITOUCH) },
> - { .driver_data = MT_CLS_DUAL1,
> + { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTID,
> HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
> USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
>
> --
> 1.7.3.4
>

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/