[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Re: [patch] pvfb: Split mouse and keyboard into separate devices.



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.