lkml.org 
[lkml]   [2011]   [Jan]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 1/2] hid-multitouch: work on default classes and renaming of classes
    From
    On Fri, Jan 28, 2011 at 18:18, Henrik Rydberg <rydberg@euromail.se> wrote:
    > 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?

    Hi Henrik,

    In it's current form, no. I often rely on it to make a quick patch
    (just adding the ids in hid-ids, hid-core and hid-multitouch) to test
    a new (dual touch) device. I thought it would be useful to be
    mainstream.

    >
    >> 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@enac.fr>
    >> ---
    >
    > 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.

    ok, will do

    >
    >>  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@enac.fr>");
    >> +MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
    >
    > Adding this is fine, based on your prior contributions. Perhaps it
    > should be motivated as such in a separate patch instead.

    ok

    >
    >>  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.

    I just wanted to keep device-specific at the end of the list. Hence
    the 10 to keep a little space between generic and devices:
    MT_CLS_DUAL_CONFIDENCE_CONTACT* are coming...


    >
    >>
    >>  /*
    >>   * 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
    >

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

    \
     
     \ /
      Last update: 2011-01-28 18:55    [W:0.034 / U:0.560 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site