Re: [PATCH 15/15] V4L/DVB: input: Add support for EVIO[CS]GKEYCODEBIG

2010-04-24 Thread David Härdeman
On Thu, Apr 01, 2010 at 02:56:31PM -0300, Mauro Carvalho Chehab wrote:
> Several devices use a high number of bits for scancodes. One important
> group is the Remote Controllers. Some new protocols like RC-6 define a
> scancode space of 64 bits.
> 
> The current EVIO[CS]GKEYCODE ioctls allow replace the scancode/keycode
> translation tables, but it is limited to up to 32 bits for scancode.
> 
> Also, if userspace wants to clean the existing table, replacing it by
> a new one, it needs to run a loop calling the old ioctls, over the
> entire sparsed scancode userspace.
> 
> To solve those problems, this patch introduces two new ioctls:
>   EVIOCGKEYCODEBIG - reads a scancode from the translation table;
>   EVIOSGKEYCODEBIG - writes a scancode into the translation table.
...
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 663208a..6445fc9 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -34,7 +34,7 @@ struct input_event {
>   * Protocol version.
>   */
>  
> -#define EV_VERSION   0x01
> +#define EV_VERSION   0x010001
>  
>  /*
>   * IOCTLs (0x00 - 0x7f)
> @@ -56,12 +56,22 @@ struct input_absinfo {
>   __s32 resolution;
>  };
>  
> +struct keycode_table_entry {
> + __u32 keycode;  /* e.g. KEY_A */
> + __u32 index;/* Index for the given scan/key table, on 
> EVIOCGKEYCODEBIG */
> + __u32 len;  /* Length of the scancode */
> + __u32 reserved[2];  /* Reserved for future usage */
> + char *scancode; /* scancode, in machine-endian */
> +};

Wouldn't changing the scancode member from a pointer to a flexible array 
member (C99 feature, which I assume is ok since other C99 features are 
already in use in the kernel code) remove the need for any compat32 
code?

struct keycode_table_entry {
__u32 keycode;
__u32 index;
__u32 len;
__u32 reserved[2];
char scancode[];
};

-- 
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


[PATCH 15/15] V4L/DVB: input: Add support for EVIO[CS]GKEYCODEBIG

2010-04-01 Thread Mauro Carvalho Chehab
Several devices use a high number of bits for scancodes. One important
group is the Remote Controllers. Some new protocols like RC-6 define a
scancode space of 64 bits.

The current EVIO[CS]GKEYCODE ioctls allow replace the scancode/keycode
translation tables, but it is limited to up to 32 bits for scancode.

Also, if userspace wants to clean the existing table, replacing it by
a new one, it needs to run a loop calling the old ioctls, over the
entire sparsed scancode userspace.

To solve those problems, this patch introduces two new ioctls:
EVIOCGKEYCODEBIG - reads a scancode from the translation table;
EVIOSGKEYCODEBIG - writes a scancode into the translation table.

The EVIOSGKEYCODEBIG can also be used to cleanup the translation entries
by associating KEY_RESERVED to a scancode.

EVIOCGKEYCODEBIG uses kt_entry::index field in order to retrieve a keycode
from the table. This field is unused on EVIOSGKEYCODEBIG.

By default, kernel will implement a default handler that will work with
both EVIO[CS]GKEYCODEBIG and the legacy EVIO[CS]GKEYCODE ioctls.

Compatibility code were also added to allow drivers that implement
only the ops handler for EVIO[CS]GKEYCODE to keep working.

Userspace compatibility for EVIO[CS]GKEYCODE is also granted: the evdev/input
ioctl handler will automatically map those ioctls with the new
getkeycodebig()/setkeycodebig() operations to handle a request using the
legacy API.

So, new drivers should only implement the EVIO[CS]GKEYCODEBIG operation
handlers: getkeycodebig()/setkeycodebig().

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 258c639..1730b5b 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -513,6 +513,8 @@ static long evdev_do_ioctl(struct file *file, unsigned int 
cmd,
struct input_absinfo abs;
struct ff_effect effect;
int __user *ip = (int __user *)p;
+   struct keycode_table_entry kt, *kt_p = p;
+   char scancode[16];
int i, t, u, v;
int error;
 
@@ -567,6 +569,43 @@ static long evdev_do_ioctl(struct file *file, unsigned int 
cmd,
 
return input_set_keycode(dev, t, v);
 
+   case EVIOCGKEYCODEBIG:
+   if (copy_from_user(&kt, kt_p, sizeof(kt)))
+   return -EFAULT;
+
+   if (kt.len > sizeof(scancode))
+   return -EINVAL;
+
+   kt.scancode = scancode;
+
+   error = input_get_keycode_big(dev, &kt);
+   if (error)
+   return error;
+
+   if (copy_to_user(kt_p, &kt, sizeof(kt)))
+   return -EFAULT;
+
+   /* FIXME: probably need some compat32 code */
+   if (copy_to_user(kt_p->scancode, kt.scancode, kt.len))
+   return -EFAULT;
+
+   return 0;
+
+   case EVIOCSKEYCODEBIG:
+   if (copy_from_user(&kt, kt_p, sizeof(kt)))
+   return -EFAULT;
+
+   if (kt.len > sizeof(scancode))
+   return -EINVAL;
+
+   kt.scancode = scancode;
+
+   /* FIXME: probably need some compat32 code */
+   if (copy_from_user(kt.scancode, kt_p->scancode, kt.len))
+   return -EFAULT;
+
+   return input_set_keycode_big(dev, &kt);
+
case EVIOCRMFF:
return input_ff_erase(dev, (int)(unsigned long) p, file);
 
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 86cb2d2..d2bb5b5 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -551,6 +551,11 @@ static void input_disconnect_device(struct input_dev *dev)
spin_unlock_irq(&dev->event_lock);
 }
 
+/*
+ * Those routines handle the default case where no [gs]etkeycode() is
+ * defined. In this case, an array indexed by the scancode is used.
+ */
+
 static int input_fetch_keycode(struct input_dev *dev, int scancode)
 {
switch (dev->keycodesize) {
@@ -565,25 +570,74 @@ static int input_fetch_keycode(struct input_dev *dev, int 
scancode)
}
 }
 
-static int input_default_getkeycode(struct input_dev *dev,
-   int scancode, int *keycode)
+/*
+ * Supports only 8, 16 and 32 bit scancodes. It wouldn't be that
+ * hard to write some machine-endian logic to support 24 bit scancodes,
+ * but it seemed overkill. It should also be noticed that, since there
+ * are, in general, less than 256 scancodes sparsed into the scancode
+ * space, even with 16 bits, the codespace is sparsed, with leads into
+ * memory and code ineficiency, when retrieving the entire scancode
+ * space.
+ * So, it is highly recommended to implement getkeycodebig/setkeycodebig
+ * instead of using a normal table approach, when more than 8 bits is
+ * needed for the scancode.
+ */
+static int input_fetch_scancode(struct keycode_table_entry *kt_entry,
+   u32 *scancode)
 {
+   switch (k