Jiri Slaby wrote:

On 02/04/2010 01:04 PM, Mauro Carvalho Chehab wrote:

quoted

quoted

I have 2 dvb-t receivers and both of them need fullspeed quirk. Further
disable_rc_polling (a dvb_usb module parameter) must be set to not get
doubled characters now. And then, it works like a charm.

Module parameters always bothers me. They should be used as last resort alternatives
when there's no other possible way to make it work properly.

If we know for sure that the RC polling should be disabled by an specific device, 
just add this logic at the driver.

Yes, this is planned and written below:

Ok.

quoted

quoted

Note that, it's just some kind of proof of concept. A migration of
af9015 devices from dvb-usb-remote needs to be done first.

Ideas, comments?

Please next time, send the patch inlined. As you're using Thunderbird, you'll likely need
Asalted-patches[1] to avoid thunderbird to destroy your patches.

I must disagree for two reasons: (a) it was not patch intended for merge
and (b) it was a plain-text attachment which is fine even for
submission. However I don't like patches as attachments so if I decide
to submit it for a merge later, you will not see it as an attachment
then :).

Attachments aren't good for reply, as they appear as a file. So, people need to
open the attachment on a separate application to see and to cut-and-paste
if they want to comment, like what I did.

quoted

+config HID_DVB
+	tristate "DVB remotes support" if EMBEDDED
+	depends on USB_HID
+	default !EMBEDDED
+	---help---
+	Say Y here if you have DVB remote controllers.
+

I think the better would be to use a more generic name, like HID_RC (for Remote Controller).
I suspect we may need in the future other hacks for other similar devices.

Seconded. I would only go for some other abbreviation other than RC or
not abbreviate that at all.

we're using irrcv at the IR remote class. So, this can be another name for it.

quoted

+static int dvb_event(struct hid_device *hdev, struct hid_field *field,
+		struct hid_usage *usage, __s32 value)
+{
+	/* we won't get a "key up" event */
+	if (value) {
+		input_event(field->hidinput->input, usage->type, usage->code, 1);
+		input_event(field->hidinput->input, usage->type, usage->code, 0);
+	}
+	return 1;
+}

Several V4L/DVB IR's have keyup/keydown events. So I think the name here is also wrong:
it is better to name the function as dvb_nokeyup_event() and eventually add an specific
quirk to indicate devices that only have key up events.

If such appear later, it can be rewritten. I don't plan to add such
functionality now until somebody comes with device IDs which should be
handled that way and tests it, because I guess I will definitely do it
wrong otherwise. 
Do you know/have such a device?

I have several devices here with different ways to generate keyup/keydown or
keydown/repeat, but they don't export a standard usb HID interface (in a matter
of fact, I have just one PCI device that came with a separate USB HID interface,
but this device always worked fine - also - as it has physically a separate device -
it doesn't generate any conflict with the DVB hardware).

The point is that it is better to name the function right since the beginning.
 

There are many of quirks needed for various devices. I already wrote
about af9005 which sends key repeat aside from key down etc. But the
same as above, I can't test it (and don't want to introduce
regressions). So again, if somebody can test it, I'll be happy to code it.

thanks for the input,

-- 

Cheers,
Mauro