From owner-freebsd-x11@FreeBSD.ORG Sat Feb 8 11:47:56 2014 Return-Path: Delivered-To: freebsd-x11@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 84E38216; Sat, 8 Feb 2014 11:47:56 +0000 (UTC) Received: from master.debian.org (master.debian.org [IPv6:2001:41b8:202:deb:216:36ff:fe40:4001]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 4656414F0; Sat, 8 Feb 2014 11:47:56 +0000 (UTC) Received: from localhost ([::1]) by master.debian.org with esmtp (Exim 4.80) (envelope-from ) id 1WC6O2-000746-Fr; Sat, 08 Feb 2014 11:47:54 +0000 Message-ID: <52F61969.2060503@freebsd.org> Date: Sat, 08 Feb 2014 11:47:53 +0000 From: Robert Millan User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Baptiste Daroussin Subject: Re: [PATCH] do not feed keyboard device path in X devd backend References: <52EFA9A9.2040901@freebsd.org> In-Reply-To: <52EFA9A9.2040901@freebsd.org> X-Enigmail-Version: 1.6 Content-Type: multipart/mixed; boundary="------------060403050100050700040708" Cc: freebsd-x11@freebsd.org X-BeenThere: freebsd-x11@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: X11 on FreeBSD -- maintaining and support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Feb 2014 11:47:56 -0000 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--