Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 08 Feb 2014 11:47:53 +0000
From:      Robert Millan <rmh@freebsd.org>
To:        Baptiste Daroussin <bapt@FreeBSD.org>
Cc:        freebsd-x11@freebsd.org
Subject:   Re: [PATCH] do not feed keyboard device path in X devd backend
Message-ID:  <52F61969.2060503@freebsd.org>
In-Reply-To: <52EFA9A9.2040901@freebsd.org>
References:  <52EFA9A9.2040901@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------060403050100050700040708
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On 03/02/2014 14:37, Robert Millan wrote:
> 
> When feeding the keyboard device path (e.g. /dev/ukbd0) to X server,
> it will attempt to open it. This is incorrect because X doesn't want
> _all_ the input from keyboard but rather just the one typed in its
> VT (usually /dev/ttyv7).
> 
> Plus, attempting to open /dev/ukbd0 usually fails with EBUSY as the
> keyboard is already being used by syscons.
> 
> This patch adjusts devd.c to follow the same approach as HAL: detect
> the keyboard but feed it a zero-length device path. The result is
> that X detects the presence of a keyboard, and therefore loads the
> kbd_drv module, but doesn't attempt to open it directly (which is
> unnecessary since /dev/ttyv7 is already open).

Unfortunately this didn't completely fix the problem. Testing by Debian
developers revealed that attaching a second keyboard and then detaching
it disabled keyboard handling in X somehow:

https://lists.debian.org/debian-bsd/2014/02/msg00074.html

I investigated and found that HAL avoids this by ignoring keyboard events
completely. Instead, it generates a fake "AT Keyboard" event during
initialization (with empty device path too), which makes X load the kbd
driver just once.

Here's a patch that implements the same behaviour with devd. I've verified
that attaching/detaching a second keyboard no longer causes this ill effect.

-- 
Robert Millan

--------------060403050100050700040708
Content-Type: text/x-patch;
 name="devd_fake_kbd.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="devd_fake_kbd.diff"

=== modified file 'devd.c'
--- devd.c	2014-02-08 11:38:01 +0000
+++ devd.c	2014-02-08 11:39:02 +0000
@@ -69,8 +69,6 @@ struct hw_type {
 };
 
 static struct hw_type hw_types[] = {
-	{ "ukbd", ATTR_KEYBOARD, "kdb" },
-	{ "atkbd", ATTR_KEYBOARD, "kdb" },
 	{ "ums", ATTR_POINTER, "mouse" },
 	{ "psm", ATTR_POINTER, "mouse" },
 	{ "joy", ATTR_JOYSTICK, NULL },
@@ -182,6 +180,44 @@ sysctl_get_str(const char *format, ...)
 }
 
 static void
+keyboard_added()
+{
+    InputOption *options = NULL;
+    InputAttributes attrs = {};
+    DeviceIntPtr dev = NULL;
+    int rc;
+
+    options =  input_option_new(NULL, "_source", "server/devd");
+    if (!options)
+        return;
+
+    options = input_option_new(options, "name", xstrdup("AT Keyboard"));
+    options = input_option_new(options, "path", xstrdup(""));
+    options = input_option_new(options, "device", xstrdup(""));
+    options = input_option_new(options, "config_info", xstrdup("devd:AT Keyboard"));
+
+    LogMessage(X_INFO, "config/devd: Adding AT Keyboard\n");
+
+    memset(&attrs, 0, sizeof(attrs));
+    attrs.flags = ATTR_KEYBOARD;
+    attrs.vendor = xstrdup("(unnamed)");
+    attrs.product = xstrdup("(unnamed)");
+
+    rc = NewInputDeviceRequest(options, &attrs, &dev);
+
+    if (rc != Success)
+        goto unwind;
+
+    return;
+
+ unwind:
+    input_option_free_list(&options);
+    free(attrs.vendor);
+    free(attrs.product);
+    return;
+}
+
+static void
 device_added(char *line)
 {
     char *walk;
@@ -435,6 +471,13 @@ config_devd_init(void)
     char devicename[1024];
     int i, j;
 
+    /* For keyboards, we don't want to open the actual device, because
+       we only need input from the VT that X is running on (see
+       xf86OpenConsole() in bsd_init.c). However, we still want
+       kbd_drv to be loaded, hence this dummy function which registers
+       a keyboard with zero-length device path. */
+    keyboard_added();
+
     /* first scan the sysctl to determine the hardware if needed */
 
     for (i = 0; hw_types[i].driver != NULL; i++) {


--------------060403050100050700040708--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?52F61969.2060503>