On Fri, Dec 5, 2008 at 7:32 PM, Anssi Hannula <anssi.hannula@gmail.com> wrote:
> Łukasz Lubojański wrote:
>>
>> On Thu, Dec 4, 2008 at 9:35 PM, Łukasz Lubojański
>> <lukasz@lubojanski.info> wrote:
>>>
>>> 2008/11/29 Jiri Kosina <jkosina@suse.cz>:
>>>>
>>>> On Fri, 28 Nov 2008, Łukasz Lubojański wrote:
>>>>
>>>>>> It seems the protocol resembles more the hid-lg2ff one. The
>>>>>> differences
>>>>>> are the additional 0xfa 0xfe 0x0 report sent to the device, and the
>>>>>> missing 0xf3 stop command.
>>>>>
>>>>> Yep - different reports are send in case of Pantherlord and GreenAsia
>>>>> 0x12 - It could be implemented in it but it will require checking what
>>>>> hardware is used and send different reports.
>>>>
>>>> OK, so as the reports are not really identical, and in the future we
>>>> might
>>>> discover that there are many more other Greenasia devices which require
>>>> a
>>>> slightly different handling as well, I would rather prefer to have it as
>>>> a
>>>> separate driver, to avoid additions of here-and-there device-specific
>>>> quirks to random places in the code. That's exactly what we are trying
>>>> to
>>>> avoid with the HID bus approach in the first place.
>>>>
>>>> So I think separate driver is fine.
>>>>
>>>> Thanks to both of you.
>>>>
>>>> --
>>>> Jiri Kosina
>>>> SUSE Labs
>>>
>>> Hi,
>>>
>>> Here is new version of the GreenAsia patch - I hope this time
>>> everything will be OK. It is based on the Pantherlord.
>>>
>>> Sorry to take so long but I have problems with the 2.6.28 (2.6.28-rc6
>>> was not loading my driver and 2.6.28-rc7 is crashing when IO APIC is
>>> enabled). Anyway I done it and I'm waiting for your feedback :D
>
>
>> +static const signed short ff_rumble[] = {
>> +       FF_RUMBLE,
>> +       -1
>> +};
>
> This seems unnecessary.
>
>> +
>> +       list_for_each_entry(hidinput, &hid->inputs, list) {
>> +
>> +               report_ptr = report_ptr->next;
>> +
>> +               if (report_ptr == report_list) {
>> +                       dev_err(&hid->dev, "required output report is "
>> +                                       "missing\n");
>> +                       return -ENODEV;
>> +               }
>
> [...]
>>
>> +       if (id->driver_data)
>> +               hdev->quirks |= HID_QUIRK_MULTI_INPUT;
>>
>
> Is this really a HID_QUIRK_MULTI_INPUT device (Multiple controllers on one
> device, for example a 2-in-1 adapter)? Just asking because your previous
> patch didn't have this.
>
> If this is not the case, there is also no need to have 2 new Kconfig
> entries, but a simple FF-only entry (see ZEROPLUS_FF / hid-zpff.c).
>
> --
> Anssi Hannula
>
>

In case of both devices that I have the HID_QUIRK_MULTI_INPUT is not
set - so I think the multi input code can be removed. I did that and
it still works properly in my test :D

I have seen that hid-zpff and hid-tmff are modules for FF only and
hid-pl is "dummy" support for the device and if selected also for the
FF. I was confused about it so I have chosen way of hid-pl because I
based on it.

Because the module supports only the FF now - I have changed module
name to hid-gaff.

regards
Lukasz Lubojanski