Em 27-09-2010 13:07, Jarod Wilson escreveu:
> On Mon, Sep 27, 2010 at 10:42:19AM -0400, George Spelvin wrote:
>>> IMO, ati_remote.c and ati_remote2.c should be adapted to use the ir-core
>>> (soon to be rc-core) interfaces. I've got remote2 hardware myself, so
>>> doing the conversion for that driver is already on my personal TODO list,
>>> but its a long queue of more important things ahead of it first...
>>
>> Just what I want, more scope creep... I'm not immovably opposed to a
>> larger conversion job, but can you tell me that the ir-core/rc-core
>> layer gives me over the raw input layer?
>
> - Hopefully, a simplified interface, with less code duplication. Quite a
> few remote drivers reimplement the same things over and over. I've not
> actually looked at ati_remote.c to see exactly what its got, but
> ati_remote2.c has some low-level evbit, keybit, __set_bit, etc.,
> manipulations that would mostly disappear w/ir-core, in favor of using
> common shared code.
>
> - Userspace remote-specific manipulation tools in v4l-utils
>
> - Sysfs hooks that reveal its a remote in the first place -- which may be
> of benefit to a userspace daemon, udev loading a new keymap
> automatically, or to media center apps that want to look directly at a
> remote control's input device until such time as X sucks less and will
> pass through keycodes larger than 8-bit.
>
>> I see how there are a bunch
>> of utilities for decoding raw pulse streams, but there's only one
>> RC_DRIVER_SCANCODE driver, imon.c, and even that seems to use the RC6
>> protocol sometimes.
>
> imon is always scancode, but some of the devices can be configured to use
> one device or another. One of them is an RC6A MCE remote. But it still
> does decoding in hardware and passes along scancodes. There's tm6000 in
> staging that is RC_DRIVER_SCANCODE, and I swear more devices under
> drivers/media/ are scancode-only and simply haven't been ported fully over
> to ir-core just yet, but I could be wrong about that.
dib0700 is also scancode only, already ported to rc core. Also, partially
ported, we have saa7134 and em28xx drivers (only scancode).
>> Looking at the code leads to a few obvious questions:
>> ir-common.h:
>> Why is the linux keycode u32, when the input layer's
>> key codes are __u16? And why is keypressed an int
>> rather than a bool?
>>
>> And why is the type __u64? It appears top be a bitmap,
>> with about 6 values defined so far...
>
> ir-common.h is going away RSN, this is from an older pre-{ir,rc}-core IR
> layer in the media tree. Its not used by imon, mceusb or streamzap,
> anyway.
The type is to handle the different IR protocols. As it is a bitmap, I
opted to define it as u64, as I was afraid that we might run out of space
with just 32 bits.
>> ir-sysfs.c:
>> Why is store_protocol, which appears to be turning an ASCII
>> string into an ir_type bitmap, documented as "triggered by
>> READING /sys/class/rc/rc?/current_protocol"? Why doesn't that
>> code support the rc6 protocol?
>
> Um... what? I see:
>
> * This routine is a callback routine for changing the IR protocol type.
> * It is trigged by writing to /sys/class/rc/rc?/protocols.
>
> And it definitely supports rc6.
George, you're probably looking at an older implementation.
>> In general, I'm bit confused about what it means to change_protocol
>> to a bitmap with multiple bits set. Are you sure ir_type needs to be
>> a bitmap? The only place I can see its bitmapness actually used is in
>> show_supported_protocols(), and that could be replaced by an array of a
>> few chars or something. (Most devices support only one or two protocols.)
>> Reserving 0 for "end of list" would make the structure initialization
>> simpler.
>
> May or may not need to be a bitmap, I didn't write that part. Mauro may be
> able to shed some light here. Generally, you'll only change_protocol to a
> single proto (that's the case w/imon), but there could be receivers where
> more than one can be enabled simultaneously.
The same bitmap is also used by raw decoders. So, you may enable just a few
protocols. There are some devices with scancodes that are able to handle
more than one protocol at the same time. So, a bitmap is more flexible than
a list of values.
>
>> One thing that would be nice is to have the permissions on the sysfs
>> protocol file depend on the existence of a change_protocol method.
>>
>> Um.. I also notice that change_protocol isn't even supported on
>> non-RC_DRIVER_SCANCODE drivers. Is this all a big kludge invented for
>> the imon driver?
>
> No. It existed before the imon driver. git grep for change_protocol, and
> you'll see its used in a number of places besides imon.c.
This were unified at the newer implementations. From userspace POV, both
raw and non-raw IR's now have the same way to enable/disable protocols.
>> ir-functions.c: Is ir->last_bit actually used for anything? I can't
>> find an assignement to a non-zero value anywhere...
>> Oh, wait, drivers/media/video/cx23885/cx23885-input.c and
>> drivers/media/video/saa7134/saa7134-input.c
>> Does that need to be in the generic structure?
>
> Not sure, haven't ever touched it myself.
>
ir-functions will die as soon as we move all drivers to use rc-core.
Cheers,
Mauro.
--
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
