From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gerd Hoffmann Subject: Re: [patch] pvfb: Split mouse and keyboard into separate devices. Date: Thu, 01 Feb 2007 14:47:35 +0100 Message-ID: <45C1EF77.30003@suse.de> References: <45C1C7FC.8080208@suse.de> <87bqke81eq.fsf@pike.pond.sub.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------080704090100010400050008" Return-path: In-Reply-To: <87bqke81eq.fsf@pike.pond.sub.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Markus Armbruster Cc: Xen devel list List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------080704090100010400050008 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Markus Armbruster wrote: > Review below. >> + 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. Yep, you are right, fixed version attached. > It *might* be simpler to have something like > > error: > if (ptr) > input_free_device(ptr); > if (kbd) > input_free_device(kbd); I don't think it would simplify the code because you'll have to take care then that the xenkbd_remove() call in the error path doesn't result in a double-free ... cheers, Gerd -- Gerd Hoffmann --------------080704090100010400050008 Content-Type: text/x-patch; name="fix-xenkbd.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="fix-xenkbd.diff" pvfb: Split mouse and keyboard into separate devices. 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" EndSection This makes the keyboard stop working though in case mouse and keyboard events are coming through the same input device. Signed-off-by: Gerd Hoffmann Cc: Markus Armbruster --- linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c | 64 ++++++++++++++-------- 1 file changed, 42 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,46 @@ 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(); + if (!kbd) goto error_nomem; + ptr = input_allocate_device(); + if (!ptr) + goto error_nomem_2; - 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"; - 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) @@ -134,6 +151,8 @@ int __devinit xenkbd_probe(struct xenbus return 0; + error_nomem_2: + input_free_device(kbd); error_nomem: ret = -ENOMEM; xenbus_dev_fatal(dev, ret, "allocating device memory"); @@ -155,7 +174,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; --------------080704090100010400050008 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------080704090100010400050008--