Re: [PATCH 1/4] hid-multitouch: Auto detection of maxcontacts

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

 



On Wed, Mar 9, 2011 at 12:22, Henrik Rydberg <rydberg@xxxxxxxxxxx> wrote:
> On Tue, Mar 08, 2011 at 05:32:56PM +0100, Benjamin Tissoires wrote:
>> This patch enables support of autodetection of maxcontacts.
>> We can still manually provide maxcontact in case the device
>> lies on it.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxx>
>> ---
>>  drivers/hid/hid-multitouch.c |   43 ++++++++++++++++++++++++++++-------------
>>  1 files changed, 29 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 65e7f20..363ca08 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -38,6 +38,8 @@ MODULE_LICENSE("GPL");
>>  #define MT_QUIRK_VALID_IS_INRANGE    (1 << 4)
>>  #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
>>
>> +#define MT_CONTACTMAX_DEFAULT        11
>> +
>
> I think this one can be removed, se below.
>
>>  struct mt_slot {
>>       __s32 x, y, p, w, h;
>>       __s32 contactid;        /* the device ContactID assigned to this slot */
>> @@ -53,8 +55,9 @@ struct mt_device {
>>       __s8 inputmode;         /* InputMode HID feature, -1 if non-existent */
>>       __u8 num_received;      /* how many contacts we received */
>>       __u8 num_expected;      /* expected last contact index */
>> +     __u8 maxcontacts;
>>       bool curvalid;          /* is the current contact valid? */
>> -     struct mt_slot slots[0];        /* first slot */
>> +     struct mt_slot *slots;
>>  };
>>
>>  struct mt_class {
>> @@ -87,12 +90,12 @@ static int cypress_compute_slot(struct mt_device *td)
>>  static int find_slot_from_contactid(struct mt_device *td)
>>  {
>>       int i;
>> -     for (i = 0; i < td->mtclass->maxcontacts; ++i) {
>> +     for (i = 0; i < td->maxcontacts; ++i) {
>>               if (td->slots[i].contactid == td->curdata.contactid &&
>>                       td->slots[i].touch_state)
>>                       return i;
>>       }
>> -     for (i = 0; i < td->mtclass->maxcontacts; ++i) {
>> +     for (i = 0; i < td->maxcontacts; ++i) {
>>               if (!td->slots[i].seen_in_this_frame &&
>>                       !td->slots[i].touch_state)
>>                       return i;
>> @@ -105,8 +108,7 @@ static int find_slot_from_contactid(struct mt_device *td)
>>
>>  struct mt_class mt_classes[] = {
>>       { .name = MT_CLS_DEFAULT,
>> -             .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP,
>> -             .maxcontacts = 10 },
>> +             .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
>
> Keeping .maxcontacts at two here seems sensible
>
>>       { .name = MT_CLS_DUAL_INRANGE_CONTACTID,
>>               .quirks = MT_QUIRK_VALID_IS_INRANGE |
>>                       MT_QUIRK_SLOT_IS_CONTACTID,
>> @@ -126,9 +128,22 @@ struct mt_class mt_classes[] = {
>>  static void mt_feature_mapping(struct hid_device *hdev,
>>               struct hid_field *field, struct hid_usage *usage)
>>  {
>> -     if (usage->hid == HID_DG_INPUTMODE) {
>> -             struct mt_device *td = hid_get_drvdata(hdev);
>> +     struct mt_device *td = hid_get_drvdata(hdev);
>> +
>> +     switch (usage->hid) {
>> +     case HID_DG_INPUTMODE:
>>               td->inputmode = field->report->id;
>> +             break;
>> +     case HID_DG_CONTACTMAX:
>> +             td->maxcontacts = *(field->value);
>> +             if (td->mtclass->maxcontacts)
>
> if (td->mtclass->maxcontacts > td->maxcontacts)
>
>> +                     /* check if the maxcontacts is given by the class */
>> +                     td->maxcontacts = td->mtclass->maxcontacts;
>> +
>> +             if (!td->maxcontacts)
>> +                     td->maxcontacts = MT_CONTACTMAX_DEFAULT;
>
> this part can be then dropped

Well, it works the way you are suggesting. BTW this let the corner
case where someone adds a device (MT_CLS) that does not send the
contact max and does not initialize the .maxcontact field.

>
>> +
>> +             break;
>>       }
>>  }
>>
>> @@ -186,8 +201,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>>                       td->last_slot_field = usage->hid;
>>                       return 1;
>>               case HID_DG_CONTACTID:
>> -                     input_mt_init_slots(hi->input,
>> -                                     td->mtclass->maxcontacts);
>> +                     input_mt_init_slots(hi->input, td->maxcontacts);
>>                       td->last_slot_field = usage->hid;
>>                       return 1;
>>               case HID_DG_WIDTH:
>> @@ -268,7 +282,7 @@ static void mt_complete_slot(struct mt_device *td)
>>       if (td->curvalid) {
>>               int slotnum = mt_compute_slot(td);
>>
>> -             if (slotnum >= 0 && slotnum < td->mtclass->maxcontacts)
>> +             if (slotnum >= 0 && slotnum < td->maxcontacts)
>>                       td->slots[slotnum] = td->curdata;
>>       }
>>       td->num_received++;
>> @@ -283,7 +297,7 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
>>  {
>>       int i;
>>
>> -     for (i = 0; i < td->mtclass->maxcontacts; ++i) {
>> +     for (i = 0; i < td->maxcontacts; ++i) {
>>               struct mt_slot *s = &(td->slots[i]);
>>               if ((td->mtclass->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
>>                       !s->seen_in_this_frame) {
>> @@ -415,9 +429,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>        */
>>       hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
>>
>> -     td = kzalloc(sizeof(struct mt_device) +
>> -                             mtclass->maxcontacts * sizeof(struct mt_slot),
>> -                             GFP_KERNEL);
>> +     td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
>>       if (!td) {
>>               dev_err(&hdev->dev, "cannot allocate multitouch data\n");
>>               return -ENOMEM;
>> @@ -434,6 +446,9 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>       if (ret)
>>               goto fail;
>>
>> +     td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
>> +                             GFP_KERNEL);
>> +
>
> Don't we have a race problem here?  It seems the device is started at
> this point, so I worry that events will be handled when slots is still
> NULL.

I tried again yesterday: if I put this line above the hid_hw_start ->
kernel oops at first touch.
The point is that hid_hw_start calls hid_connect that do the actual
calls to input_mapping and feature_mapping.

Cheers,
Benjamin

>
>>       mt_set_input_mode(hdev);
>>
>>       return 0;
>> --
>> 1.7.4
>>
>
> Thanks,
> Henrik
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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