On Fri, Mar 26, 2010 at 02:22:51PM -0300, Mauro Carvalho Chehab wrote:
> Dmitry Torokhov wrote:
> > On Fri, Mar 26, 2010 at 11:40:41AM -0300, Mauro Carvalho Chehab wrote:
> >> David Härdeman wrote:
> >>> I'd suggest:
> >>>
> >>> struct keycode_table_entry {
> >>> unsigned keycode;
> >>> unsigned index;
> >>> unsigned len;
> >>> char scancode[];
> >>> };
> >>>
> >>> Use index in EVIOCGKEYCODEBIG to look up a keycode (all other fields are
> >>> ignored), that way no special function to clear the table is necessary,
> >>> instead you do a loop with:
> >>>
> >>> EVIOCGKEYCODEBIG (with index 0)
> >>> EVIOCSKEYCODEBIG (with the returned struct from EVIOCGKEYCODEBIG and
> >>> keycode = KEY_RESERVED)
> >>>
> >>> until EVIOCGKEYCODEBIG returns an error.
> >> Makes sense.
> >
> > Yes, I think so too. Just need a nice way to handle transition, I'd
> > like in the end to have drivers implement only the improved methods and
> > map legacy methods in evdev.
>
> Ok. I'll prepare the patches for adding the new ioctl, in a way that it will
> also handle the legacy methods, and post for review.
If EVIOCGKEYCODEBIG is going to be used as a superset of the old ioctl,
might it be a good idea change the proposed struct to:
struct keycode_table_entry {
unsigned keycode;
unsigned index;
unsigned type;
unsigned len;
char scancode[];
};
Where "type" is used to give a hint of how the scancode[] member should
be interpreted?
>>>> On a related note, I really think the interface would benefit from
>>>> allowing more than one keytable per irrcv device with an input
>>>> device created per keytable. That way you can have one input device
>>>> per remote control. This implies that EVIOCLEARKEYCODEBIG is a bit
>>>> misplaced as an evdev IOCTL since there's an N-1 mapping between
>>>> input devices and irrcv devices.
>>> I don't think that an ioctl over one /dev/input/event should be the
>>> proper way
>>> to ask kernel to create another filtered /dev/input/event. As it
>>> were commented
>>> that the multimedia keys on some keyboards could benefit on having a
>>> filter
>>> capability, maybe we may have a sysfs node at class input that would
>>> allow
>>> the creation/removal of the filtered event interface.
>>
>> No, if you want separate event devices just create a new instance
>> of
>> input device for every keymap and have driver/irrcv class route
>> events
>> to proper input device.
I fully agree!
> This don't solve the issue about how to signalize to kernel that more than one
> input device is needed.
>
> As the userspace will request the creation of those keymaps, we need some way
> to receive such requests from userspace.
>
> I can see a few ways for doing it:
>
> 1) create a control device for the irrcv device as a hole,
> that would handle such requests via ioctl (/dev/irctl[0-9]* ?)
>
> 2) create a read/write sysfs node that would indicate the number of
> event/keymaps
> associated with a given IR. By writing a bigger number, it would create new
> devices.
> By writing a smaller number, it will delete some maps. There's an issue
> though:
> what criteria would be used to delete? The newly created ones?
This won't work for the reason you've already set out...which keymap
should be deleted?
> 3) create a fixed number of event devices, and add a sysfs attribute to enable
> or disable it;
You really seem to prefer sysfs over ioctls :)
> 4) create a fixed number of sysfs attributes to represent the keymaps. For
> example:
> /sys/class/irrcv/irrcv0/keymap0/enabled
> ...
> /sys/class/irrcv/irrcv0/keymap7/enabled
>
> The input/event node will be created only when the enabled=1.
This sounds like 3)
> I don't like (2) or (3), because removing a table with (2) may end by
> removing the wrong
> table, and (3) will create more event interfaces than probably needed by the
> majority
> of IR users.
>
> maybe (4) is the better one.
Personally I think 1) is the best approach. Having a device for the
irrcv device allows for three kinds of operations:
read
----
which corresponds to RX...you're eventually going to want to let
userspace devices read IR commands which have no entries in a keytable
yet in order to create keytables for new remotes, the same interface can
also be used for lirc-type user-space apps which want to access the raw
pulse/space timings for userspace decoding.
write
-----
which would correspond to TX...I'd suggest a stream of s32 integers to
imply pulse/space timings.
ioctl
-----
for controlling the RX/TX parameters, creating/destroying additional
keytables, etc...
Basically, we'll end up with a lirc_dev 2.0. And the "irrcv" class name
will be misleading since it will be irrcv + irsnd :)
--
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
