From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jon Smirl Subject: Re: [RFC] What are the goals for the architecture of an in-kernel IR system? Date: Fri, 26 Mar 2010 18:37:41 -0400 Message-ID: <9e4733911003261537s770a66c8v92ab7384fde34839@mail.gmail.com> References: <20091215195859.GI24406@elf.ucw.cz> <9e4733910912151229o371ee017tf3640d8f85728011@mail.gmail.com> <20091215203300.GL24406@elf.ucw.cz> <9e4733910912151245ne442a5dlcfee92609e364f70@mail.gmail.com> <9e4733910912151338n62b30af5i35f8d0963e6591c@mail.gmail.com> <4BAB7659.1040408@redhat.com> <20100326112755.GB5387@hardeman.nu> <4BACC769.6020906@redhat.com> <20100326160150.GA28804@core.coreip.homeip.net> <4BACED6B.9030409@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-qy0-f188.google.com ([209.85.221.188]:42404 "EHLO mail-qy0-f188.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754261Ab0CZWho convert rfc822-to-8bit (ORCPT ); Fri, 26 Mar 2010 18:37:44 -0400 In-Reply-To: <4BACED6B.9030409@redhat.com> Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Mauro Carvalho Chehab Cc: Dmitry Torokhov , Pavel Machek , Krzysztof Halasa , hermann pitton , Christoph Bartelmus , awalls@radix.net, j@jannau.net, jarod@redhat.com, jarod@wilsonet.com, kraxel@redhat.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, superm1@ubuntu.com On Fri, Mar 26, 2010 at 1:22 PM, Mauro Carvalho Chehab wrote: > Dmitry Torokhov wrote: >> On Fri, Mar 26, 2010 at 11:40:41AM -0300, Mauro Carvalho Chehab wrot= e: >>> David H=E4rdeman wrote: >>>> On Thu, Mar 25, 2010 at 11:42:33AM -0300, Mauro Carvalho Chehab wr= ote: >>>>>> =A0 =A0 =A0 =A010) extend keycode table replacement to support b= ig/variable >>>>>> =A0 =A0 =A0 =A0sized scancodes; >>>>> Pending. >>>>> >>>>> The current limit here is the scancode ioctl's are defined as: >>>>> >>>>> #define EVIOCGKEYCODE =A0 =A0 =A0 =A0 =A0 _IOR('E', 0x04, int[2])= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* get keycode */ >>>>> #define EVIOCSKEYCODE =A0 =A0 =A0 =A0 =A0 _IOW('E', 0x04, int[2])= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* set keycode */ >>>>> >>>>> As int size is 32 bits, and we must pass both 64 (or even bigger)= scancodes, associated >>>>> with a keycode, there's not enough bits there for IR. >>>>> >>>>> The better approach seems to create an struct with an arbitrary l= ong size, like: >>>>> >>>>> struct keycode_table_entry { >>>>> =A0 =A0unsigned keycode; >>>>> =A0 =A0char scancode[32]; =A0 =A0 =A0/* 32 is just an arbitrary l= ong array - maybe shorter */ >>>>> =A0 =A0int len; >>>>> } >>>>> >>>>> and re-define the ioctls. For example we might be doing: >>>>> >>>>> #define EVIOCGKEYCODEBIG =A0 =A0 =A0 =A0 =A0 _IOR('E', 0x04, stru= ct keycode_table_entry) >>>>> #define EVIOCSKEYCODEBIG =A0 =A0 =A0 =A0 =A0 _IOW('E', 0x04, stru= ct keycode_table_entry) >>>>> #define EVIOCLEARKEYCODEBIG =A0 =A0 =A0 =A0_IOR('E', 0x04, void) >>>>> >>>>> Provided that the size for struct keycode_table_entry is differen= t, _IO will generate >>>>> a different magic number for those. >>>>> >>>>> Or, instead of using 0x04, just use another sequential number at = the 'E' namespace. >>>>> >>>>> An specific function to clear the table is needed with big scanco= de space, >>>>> as already discussed. >>>>> >>>> I'd suggest: >>>> >>>> struct keycode_table_entry { >>>> =A0 =A0 unsigned keycode; >>>> =A0 =A0 unsigned index; >>>> =A0 =A0 unsigned len; >>>> =A0 =A0 char scancode[]; >>>> }; >>>> >>>> Use index in EVIOCGKEYCODEBIG to look up a keycode (all other fiel= ds are >>>> ignored), that way no special function to clear the table is neces= sary, >>>> instead you do a loop with: >>>> >>>> EVIOCGKEYCODEBIG (with index 0) >>>> EVIOCSKEYCODEBIG (with the returned struct from EVIOCGKEYCODEBIG a= nd >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 keycode =3D 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. > >>>> On a related note, I really think the interface would benefit from >>>> allowing more than one keytable per irrcv device with an input dev= ice >>>> created per keytable. That way you can have one input device per r= emote >>>> 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 we= re 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 woul= d 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 even= ts >> to proper input device. > > 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 e= vent/keymaps > associated with a given IR. By writing a bigger number, it would crea= te new devices. > By writing a smaller number, it will delete some maps. There's an iss= ue though: > what criteria would be used to delete? The newly created ones? This is normally handled a sysfs node on the core, something like 'adddev'. You echo '1' to this node and a new interface is created. Each interface has a sysfs node, make a 'remove' attribute in it. Echo '1' to remove to make it disappear. You have to implement the code behind these interfaces but this convention is used in other subsubsystems. BTW - you're recreating everything the configfs interface did. it achieved the same results with mkdir/rmdir. I liked the configfs scheme since there are no obscure commands to learn. Everybody can make files and directories. > > 3) create a fixed number of event devices, and add a sysfs attribute = to enable > or disable it; > > 4) create a fixed number of sysfs attributes to represent the keymaps= =2E For example: > /sys/class/irrcv/irrcv0/keymap0/enabled > =A0 =A0 =A0 =A0... > /sys/class/irrcv/irrcv0/keymap7/enabled > > The input/event node will be created only when the enabled=3D1. > > 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. > > -- > > Cheers, > Mauro > --=20 Jon Smirl jonsmirl@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-input" = in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html