From: Jiri Slaby on
On 02/04/2010 02:41 PM, Mauro Carvalho Chehab wrote:
> The point is that it is better to name the function right since the beginning.

Sorry, I misunderstood you for the first time. It's .event member of
hid_driver. Hence I named it dvb_event (or now rc_event or whatever).

The function may contain decisions on what to do with the event based
for example on quirks set up in .probe. And if the function grows later,
it may be factored out to rc_nokeyup_event. But rc_event is a root for
the decision tree and it should be there forever. Does it make sense now?

--
js
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Pekka Sarnila on
Yes, my comment maybe criticizes more the basic architectural structure
of usb putting it's own work up to higher layer. The only practical
thing is that, if there is a non-HID device suffering from that
FULLSPEED problem, the quirk won't help it. Anyway in current kernel
structure usb layer doesn't handle endpoint setup at all, thus it simply
can not do the job.

Pekka

Jiri Kosina wrote:
>
> Yes, I still think what I have stated before, that this should be properly
> handled in the USB stack.
>
> On the other hand, in usbhid driver we do a lot of USB-specific
> lower-level things anyway, so it's technically more-or-less OK to apply
> the quirk there as well (and that's why I have accepted it back then).
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mauro Carvalho Chehab on
Pekka Sarnila wrote:

> The problem using vendor class is that there is no standard. Each vendor
> can define its own way using endpoints (and has done so e.g Logitech
> joysticks). Thus each usb ir receiver must have its own specific driver.
> However then you get the raw ir codes. When using HID class, generic
> HID driver can do the job. But then you get HID key codes directly not
> ir codes.

We started writing an abstraction layer for IR, using the input. The idea
is to allow the IR receivers to work with different IR's, as several users
prefer to use universal IR's to control their devices, instead of the original
one. This is already used by all V4L drivers and I intend to port most of
the DVB drivers to use it as well soon.

> Also this should not be seen at all as dvb question. First, not all the
> world uses dvb standard (including USA) but uses very similar tv-sticks
> with identical ir receivers and remotes.

Despiste its name, the DVB subsystem is not specific for DVB standards, but
it is meant to be used by all DTV standards (and almost all DTV standards
are already supported).

> Second there are many other
> type of usb devices with ir receiver. So dvb layer should not be
> involved at all. There maybe would be need for hid-ir-remote layer (your
> code suggestion moved there). However even there IMHO better would be
> just to improve HID <-> input layer interface so that input layer could
> divert the remotes input to generic remotes layer instead of keyboard
> layer. IMHO this would be the real linux way.

This is already happening.

See drivers/media/IR on linux-next for the IR common code. The ir-core is
provided by ir-keytable.c and ir-sysfs.c, and it is not DVB or V4L specific.

The ir-common module were developed for V4L drivers and will probably be
changed into a more generic way, with the integration with lirc.

> However linux usb layer has been build so that it was technically
> impossible to put it there without completely redesigning usb <-> higher
> layer (including HID) interface. But I'm of the opinion that that
> redesign should be done anyway.

Feel free to submit patches. My plan is to integrate the DVB devices soon
into the new ir-core. I should start with dvb-usb-remote, where most of the
DVB USB devices use to attach their IR's. Unfortunately, af9015 doesn't
rely on the dvb-usb-remote, so, it will require some specific changes.
As I don't have any af9015 device, I'll likely postpone it or wait for
someone to do the job.

> Now the question is, how much all usb based ir receivers have in common,
> and how much they differ. This should determine on what level and in
> which way they should be handled. How much and on what level there
> should be common code and where device specific driver code would be
> needed and where and how the ir-to-code translate should take place.

There are several different ways for IR receivers, and, at least for
vendor class, no standard way to handle. They can use GPIO polling,
they can use i2c layer, they can use IRQ (on PCI devices) and the data
may be provided in parallel or on a serial interface.

The idea of the ir-core is to provide support for all those options.

> IMHO all that have HID endpoint that works (either as such or with some
> generic quirk) should be handled by HID first and then conveyed to
> generic remotes layer that handles all remote controllers what ever the
> lower layers (and does translate per remote not per ir receiver). Vendor
> class should be avoided unless that's the only way to make it work
> right. But using HID is not without problems either.

Almost all chipsets only provide IR via vendor class. I agree that using
standard HID class is the better way for doing it.

> Now with the two receivers that need the quirk. If they don't have
> vendor class bulk endpoint for reading ir codes, then specific driver is
> out anyway. However then changes to HID driver would be needed to make
> the translate work. The quirk just makes them work as generic usb
> keyboard with no remote specific translate. With afatech, driver loads
> the translate table to the device, different for different remotes. I
> don't know if one table could handle them all. Maybe this table should
> be loaded from a user space file. Nor do I know how it is with other
> receivers: can the table be loaded or is it fixed. In any case a generic
> secondary per remote user configurable translate would be highly
> desirable. And I don't mean lircd. This job is IMHO kernel level job and
> lircd won't work here anyway. It does ir code to key code or function
> translate not key code to key code translate. How nice it would be if
> there would be a generic usb ir receiver class that all vendors used.
> There seems to be no way to make this well and clean.

The ir-core provides standard ways to replace the IR keycode and IR protocols.
The IR protocol change is already working with vendor class with em28xx driver.

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mauro Carvalho Chehab on
Jiri Slaby wrote:
> On 02/04/2010 02:41 PM, Mauro Carvalho Chehab wrote:
>> The point is that it is better to name the function right since the beginning.
>
> Sorry, I misunderstood you for the first time. It's .event member of
> hid_driver. Hence I named it dvb_event (or now rc_event or whatever).
>
> The function may contain decisions on what to do with the event based
> for example on quirks set up in .probe. And if the function grows later,
> it may be factored out to rc_nokeyup_event. But rc_event is a root for
> the decision tree and it should be there forever. Does it make sense now?

Ah, ok. Due to the comments at the function, I misunderstood that you were
planning to have separate functions for separate quirks.


--

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jarod Wilson on
On 02/04/2010 08:41 AM, Mauro Carvalho Chehab wrote:
> Jiri Slaby wrote:
>> On 02/04/2010 01:04 PM, Mauro Carvalho Chehab wrote:
>>>> 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.
>>
>>>> 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.

Just as an FYI... If you use mutt appropriately configured, it'll DTRT
with attached patches and let you reply with them quoted inline, and
actually, thunderbird 3 will more or less work with attached patches if
you do a select-all, then hit reply (tbird finally has 'quote selected
text' support).

Not that I'm advocating patches as attachments.

--
Jarod Wilson
jarod(a)redhat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/