Gerd Hoffmann <kraxel@xxxxxxx> writes:
> Hi,
>
> This patch creates two separate input devices for keyboard and mouse
> events. The reason for this is to separate them in the linux input
> layer and allow them being routed different ways.
>
> Use case: Configure the X-Server like this to get the mouse
> events directly from the linux input layer, which has the major
> advantage that absolute coordinates work correctly:
>
> Section "InputDevice"
> Driver "evdev"
> Identifier "Mouse[1]"
> Option "Device" "/dev/input/event<nr>"
> EndSection
>
> This makes the keyboard stop working though in case mouse and
> keyboard events are coming through the same input device.
Review below.
I'll leave judging whether this is a good idea to those who know more
about X than I do.
> please apply,
> Gerd
>
> --
> Gerd Hoffmann <kraxel@xxxxxxx>
[...]
> ---
> linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c | 60
> +++++++++++++---------
> 1 file changed, 38 insertions(+), 22 deletions(-)
>
> Index:
> build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
> ===================================================================
> ---
> build-64-unstable-13762.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
> +++ build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
> @@ -29,7 +29,8 @@
>
> struct xenkbd_info
> {
> - struct input_dev *dev;
> + struct input_dev *kbd;
> + struct input_dev *ptr;
> struct xenkbd_page *page;
> int irq;
> struct xenbus_device *xbdev;
> @@ -60,19 +61,21 @@ static irqreturn_t input_handler(int rq,
>
> switch (event->type) {
> case XENKBD_TYPE_MOTION:
> - input_report_rel(info->dev, REL_X, event->motion.rel_x);
> - input_report_rel(info->dev, REL_Y, event->motion.rel_y);
> + input_report_rel(info->ptr, REL_X, event->motion.rel_x);
> + input_report_rel(info->ptr, REL_Y, event->motion.rel_y);
> + input_sync(info->ptr);
> break;
> case XENKBD_TYPE_KEY:
> - input_report_key(info->dev, event->key.keycode,
> event->key.pressed);
> + input_report_key(info->kbd, event->key.keycode,
> event->key.pressed);
> + input_sync(info->kbd);
> break;
> case XENKBD_TYPE_POS:
> - input_report_abs(info->dev, ABS_X, event->pos.abs_x);
> - input_report_abs(info->dev, ABS_Y, event->pos.abs_y);
> + input_report_abs(info->ptr, ABS_X, event->pos.abs_x);
> + input_report_abs(info->ptr, ABS_Y, event->pos.abs_y);
> + input_sync(info->ptr);
> break;
> }
> }
> - input_sync(info->dev);
> mb(); /* ensure we got ring contents */
> page->in_cons = cons;
> notify_remote_via_irq(info->irq);
> @@ -85,7 +88,7 @@ int __devinit xenkbd_probe(struct xenbus
> {
> int ret, i;
> struct xenkbd_info *info;
> - struct input_dev *input_dev;
> + struct input_dev *kbd, *ptr;
>
> info = kzalloc(sizeof(*info), GFP_KERNEL);
> if (!info) {
> @@ -101,32 +104,44 @@ int __devinit xenkbd_probe(struct xenbus
> info->page->in_cons = info->page->in_prod = 0;
> info->page->out_cons = info->page->out_prod = 0;
>
> - input_dev = input_allocate_device();
> - if (!input_dev)
> + kbd = input_allocate_device();
> + ptr = input_allocate_device();
> + if (!kbd || !ptr)
> goto error_nomem;
If just one of two fails, the other is leaked, I fear.
>
> - input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
> - input_dev->keybit[LONG(BTN_MOUSE)]
> + ptr->evbit[0] = BIT(EV_REL) | BIT(EV_ABS);
> + ptr->keybit[LONG(BTN_MOUSE)]
> = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
> /* TODO additional buttons */
> - input_dev->relbit[0] = BIT(REL_X) | BIT(REL_Y);
> + ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
>
> + kbd->evbit[0] = BIT(EV_KEY);
> /* FIXME not sure this is quite right */
> for (i = 0; i < 256; i++)
> - set_bit(i, input_dev->keybit);
> + set_bit(i, kbd->keybit);
>
> - input_dev->name = "Xen Virtual Keyboard/Mouse";
> + kbd->name = "Xen Virtual Keyboard";
> + ptr->name = "Xen Virtual Touchscreen";
Touchscreen is going to confuse people. I'd rather call it a mouse.
Or pointing device, if need be.
>
> - input_set_abs_params(input_dev, ABS_X, 0, XENFB_WIDTH, 0, 0);
> - input_set_abs_params(input_dev, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
> + input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0);
> + input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
>
> - ret = input_register_device(input_dev);
> + ret = input_register_device(kbd);
> if (ret) {
> - input_free_device(input_dev);
> - xenbus_dev_fatal(dev, ret, "input_register_device");
> + input_free_device(kbd);
> + input_free_device(ptr);
> + xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
> goto error;
> }
> - info->dev = input_dev;
> + info->kbd = kbd;
> +
> + ret = input_register_device(ptr);
> + if (ret) {
> + input_free_device(ptr);
> + xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
> + goto error;
> + }
> + info->ptr = ptr;
>
> ret = xenkbd_connect_backend(dev, info);
> if (ret < 0)
It *might* be simpler to have something like
error:
if (ptr)
input_free_device(ptr);
if (kbd)
input_free_device(kbd);
> @@ -155,7 +170,8 @@ static int xenkbd_remove(struct xenbus_d
> struct xenkbd_info *info = dev->dev.driver_data;
>
> xenkbd_disconnect_backend(info);
> - input_unregister_device(info->dev);
> + input_unregister_device(info->kbd);
> + input_unregister_device(info->ptr);
> free_page((unsigned long)info->page);
> kfree(info);
> return 0;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|