Should we create a raw input interface for IR's ? - Was: Re: [PATCH 1/3 v2] lirc core device driver infrastructure
in [Kernel]
|
Prev: x86, crypto, Use gas macro for PCLMULQDQ-NI and PSHUFB
Next: [patch 6/8] x86: UV - XPC NULL deref when mesq becomes empty.
From: Mauro Carvalho Chehab on 26 Nov 2009 17:10 Krzysztof Halasa wrote: > Mauro Carvalho Chehab <mchehab(a)redhat.com> writes: > >> No. All the other API functions there work with 32 bits for scancodes. > > We don't need them, do we? We need a new ioctl for changing key mappings > anyway (a single ioctl for setting the whole table I think), and we can > have arbitrary length of scan codes there. Why do you want to replace everything into a single shot? Had you ever tried to replace a scancode table with the current API? $ wc ./keycodes/dib0700_rc_keys 216 432 3541 ./keycodes/dib0700_rc_keys This is the biggest table we have: 216 scancodes. It has codes for several different IR's bound together into the same table. Let's replace the entire table (tested on a dib8076 reference design device): $ time ./keytable ./keycodes/dib0700_rc_keys real 0m0.029s user 0m0.000s sys 0m0.027s Don't you think that 29ms to replace 216 codes to be fast enough, especially since you only need to do it once after plugging a device? Also, if you want to control your device with two different IR controllers, the better is to allow adding new keycodes there, instead of just allowing the replacement of the entire table. Maybe we'll need some extensions there, for example to extend the size of the dynamic table, but I don't see any timing issue here. 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 26 Nov 2009 17:20 Christoph Bartelmus wrote: > Hi Mauro, > > on 26 Nov 09 at 18:59, Mauro Carvalho Chehab wrote: >> Christoph Bartelmus wrote: > [...] >>>> lircd supports input layer interface. Yet, patch 3/3 exports both devices >>>> that support only pulse/space raw mode and devices that generate scan >>>> codes via the raw mode interface. It does it by generating artificial >>>> pulse codes. >>> Nonsense! There's no generation of artificial pulse codes in the drivers. >>> The LIRC interface includes ways to pass decoded IR codes of arbitrary >>> length to userspace. > >> I might have got wrong then a comment in the middle of the >> imon_incoming_packet() of the SoundGraph iMON IR patch: > > Indeed, you got it wrong. > As I already explained before, this device samples the signal at a > constant rate and delivers the current level in a bit-array. This data is > then condensed to pulse/space data. Ah, ok. It is now clear to me. IMHO, it would be better to explain this at the source code, since the imon_incoming_packet() is a little complex. It would help the review process if those big routines could be broken into a few functions. While this improves code readability, it shouldn't affect performance, as gcc will handle the static functions used only once as inline. > Christoph 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: Trent Piepho on 26 Nov 2009 18:10 On Thu, 26 Nov 2009, Mauro Carvalho Chehab wrote: > >> See above. Also, several protocols have a way to check if a keystroke were > >> properly received. When handling just one protocol, we can use this to double > >> check the key. However, on a multiprotocol mode, we'll need to disable this > >> feature. > > > > I don't think so. We can pass the space/mark data to all (configured, > > i.e. with active mapping) protocol handlers at once. Should a check > > fail, we ignore the data. Perhaps another protocol will make some sense > > out of it. > > What happens if it succeeds on two protocol handlers? Then you use the protocol that fits best. For instance decoding with one protocol might produce a scancode that isn't assigned to any key, while another protocol produces an assigned scancode. Clearly then the latter is most likely to be correct. It also possible to have a space/mark length that is within the allowable tolerances for one remote, but is even closer another remote. You don't want to just find *a* match, you want to find the *best* match. The in kernel code in v4l is very simple in that it is only designed to work with one procotol and one remote. Once you have multiple remotes of any type things become much more complicted. Keep in mind that remotes that aren't even intended to be used with the computer but are used in the same room will still be received by the receiver. It's not enough to decode the signals you expect to receive, you must also not get confused by random signals destined for somewhere else. -- 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: Trent Piepho on 26 Nov 2009 18:20 On Thu, 26 Nov 2009, Mauro Carvalho Chehab wrote: > >> lircd supports input layer interface. Yet, patch 3/3 exports both devices > >> that support only pulse/space raw mode and devices that generate scan > >> codes via the raw mode interface. It does it by generating artificial > >> pulse codes. > > > > Nonsense! There's no generation of artificial pulse codes in the drivers. > > The LIRC interface includes ways to pass decoded IR codes of arbitrary > > length to userspace. > > I might have got wrong then a comment in the middle of the > imon_incoming_packet() of the SoundGraph iMON IR patch: > > + /* > + * Translate received data to pulse and space lengths. > + * Received data is active low, i.e. pulses are 0 and > + * spaces are 1. I'm not sure about this specific code, but what is likely going on here is the waveform is being RLE encoding. For example, a cx88 receiver has two ways of being connected (without using an external decoder chip). One generates an IRQ on each edge of the signal. The time between IRQs gives mark/space lengths which is what lirc expects. This is how a simple serial port receiver works too. Another connections effectivly samples the waveform one bit deep at IIRC 4kHz. I think that's what the code you are looking at gets. The code extracts the edges from the waveform and returns the time between them. In effect one is run length encoding a sequence of bits. -- 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 26 Nov 2009 20:50
Dmitry Torokhov wrote: > On Thu, Nov 26, 2009 at 03:49:13PM -0200, Mauro Carvalho Chehab wrote: >> Dmitry, >> >> While lirc is basically a series of input drivers, considering that they have lots >> in common with the input drivers at V4L/DVB and that we'll need to work on >> some glue to merge both, do you mind if I add the lirc drivers at drivers/staging from >> my trees? >> > > Mauro, > > I would not mind if you will be pushing it to staging, however I am not > sure we have an agreement on what exactly the interface that we will be > using. I would hate to get something in that will need to be reworked > again. I understand your concerns. IMHO, we should be really careful with API's when migrating from staging to the right place, but I'm not that much concerned with staging. We already have several drivers there with bad behaviors and even with some API's there that will go to /dev/null. For example there's a V4L2 driver there (staging/go7007) that has their own private API to handle compressed streams. I won't ack moving it from staging while it has their own private extensions for something that are part of V4L2 API. Also, staging drivers without progress for a few kernel cycles will be moved to /dev/null, so I don't see much sense of denying a driver to go there. Anyway, I'll add it there only when you feel comfortable about that and send us your ack. - From what I heard on the comments, I think we have already a consensus of some points: 1) all IR's should implement the standard evdev interface; 2) IR's with raw interfaces will implement a raw pulse/space IR interface. The proposal is to use lirc API interface for raw pulse/code TX and RX. Do you think we'll need to better detail the IR raw interface API before accepting the patches for staging? If so, what level of details do you want? 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/ |