Re: [PATCH 3/4] hid-multitouch: migrate 3M PCT touch screens to theunified driver.

[Benjamin Tissoires]

Hi Henrik

On Wed, Mar 9, 2011 at 09:46, Henrik Rydberg <rydberg@xxxxxxxxxxx> wrote:
> Hi Benjamin,
>
> On Tue, Mar 08, 2011 at 05:32:58PM +0100, Benjamin Tissoires wrote:
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxx>
>> ---
>
> Semms this patch is missing a commit message.

You can copy/paste this point to the other patches as well.

How about:
"3M PCT touch screens can be handled by the unified driver. This will
ease future evolution. So, just move it."

Then I'll copy/paste for the others.

>
> [...]
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index e7a4f83..f32bf46 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -5,6 +5,11 @@
>>   *  Copyright (c) 2010-2011 Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx>
>>   *  Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
>>   *
>> + *  based on hid-3m-pct.c copyrighted as follows:
>> + *    Copyright (c) 2009-2010 Stephane Chatty <chatty@xxxxxxx>
>> + *    Copyright (c) 2010      Henrik Rydberg <rydberg@xxxxxxxxxxx>
>> + *    Copyright (c) 2010      Canonical, Ltd.
>> + *
>>   */
>>
>>  /*
>> @@ -74,6 +79,7 @@ struct mt_class {
>>  #define MT_CLS_DUAL_INRANGE_CONTACTNUMBER    3
>>  #define MT_CLS_CYPRESS                               4
>>  #define MT_CLS_STANTUM                               5
>> +#define MT_CLS_3M                            6
>>
>>  /*
>>   * these device-dependent functions determine what slot corresponds
>> @@ -124,6 +130,11 @@ struct mt_class mt_classes[] = {
>>               .maxcontacts = 10 },
>>       { .name = MT_CLS_STANTUM,
>>               .quirks = MT_QUIRK_VALID_IS_CONFIDENCE },
>> +     { .name = MT_CLS_3M,
>> +             .quirks = MT_QUIRK_VALID_IS_CONFIDENCE |
>> +                     MT_QUIRK_SLOT_IS_CONTACTID,
>> +             .sn_move = 2048,
>> +             .sn_pressure = 128 },
>>
>>       { }
>>  };
>> @@ -479,6 +490,14 @@ static void mt_remove(struct hid_device *hdev)
>>
>>  static const struct hid_device_id mt_devices[] = {
>>
>> +     /* 3M panels */
>> +     { .driver_data = MT_CLS_3M,
>> +             HID_USB_DEVICE(USB_VENDOR_ID_3M,
>> +                     USB_DEVICE_ID_3M1968) },
>> +     { .driver_data = MT_CLS_3M,
>> +             HID_USB_DEVICE(USB_VENDOR_ID_3M,
>> +                     USB_DEVICE_ID_3M2256) },
>> +
>>       /* Cypress panel */
>>       { .driver_data = MT_CLS_CYPRESS,
>>               HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
>
> The touch width correction from the original driver has gone missing
> here. Otherwise, looks good, but will defer testing to the next round.

Oops, it's not missing, it has been transformed into sn_pressure...
Will correct it.

By looking at it, I just saw that we override values for HID_DG_HEIGHT
to the range [0-1].
I'll have to dig into.

For the tests, I've tested it on the 1968.

Thanks,
Benjamin
--
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/