Re: New Force Feedback device support - GreenAsia 0x12

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

 



Łukasz Lubojański wrote:
On Fri, Dec 5, 2008 at 7:32 PM, Anssi Hannula <anssi.hannula@xxxxxxxxx> wrote:
Łukasz Lubojański wrote:
On Thu, Dec 4, 2008 at 9:35 PM, Łukasz Lubojański
<lukasz@xxxxxxxxxxxxxxx> wrote:
2008/11/29 Jiri Kosina <jkosina@xxxxxxx>:
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

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


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.

hid-pl just sets HID_QUIRK_MULTI_INPUT when FF is disabled. For gaff we do not need that.

Because the module supports only the FF now - I have changed module
name to hid-gaff.
[...]
+	gaff->report->field[0]->value[0] = 0x51;
+	gaff->report->field[0]->value[1] = 0x0;
+	gaff->report->field[0]->value[2] = right;
+	gaff->report->field[0]->value[3] = 0;
+	gaff->report->field[0]->value[4] = left;
+	gaff->report->field[0]->value[5] = 0;

[...]
+	if (list_empty(report_list)) {
+		dev_err(&hid->dev, "no output reports found\n");
+		return -ENODEV;
+	}
+
+	report_ptr = report_ptr->next;

+	if (report_ptr == report_list) {
+		dev_err(&hid->dev, "required output report is "
+				"missing\n");
+		return -ENODEV;
+	}

Unneeded test. list_empty() call above already confirmed that there are output reports.

+	report = list_entry(report_ptr, struct hid_report, list);
+	if (report->maxfield < 1) {
+		dev_err(&hid->dev, "no fields in the report\n");
+		return -ENODEV;
+	}
+
+	if (report->field[0]->report_count < 4) {
+		dev_err(&hid->dev, "not enough values in the field\n");

Here you test for only 4, while in hid_gaff_play() you use 6.

--
Anssi Hannula

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