Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 22 Dec 2012 13:02:04 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r244583 - projects/calloutng/sys/dev/atkbdc
Message-ID:  <201212221302.qBMD24Vg080683@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Sat Dec 22 13:02:03 2012
New Revision: 244583
URL: http://svnweb.freebsd.org/changeset/base/244583

Log:
  Revert atkbd driver changes.  They are reported to cause mouse lags and
  other problems on some systems.  I guess the problem may be in insufficient
  locking of the driver.

Modified:
  projects/calloutng/sys/dev/atkbdc/atkbd.c
  projects/calloutng/sys/dev/atkbdc/atkbd_atkbdc.c

Modified: projects/calloutng/sys/dev/atkbdc/atkbd.c
==============================================================================
--- projects/calloutng/sys/dev/atkbdc/atkbd.c	Sat Dec 22 09:37:34 2012	(r244582)
+++ projects/calloutng/sys/dev/atkbdc/atkbd.c	Sat Dec 22 13:02:03 2012	(r244583)
@@ -62,6 +62,7 @@ __FBSDID("$FreeBSD$");
 #include <dev/atkbdc/atkbdreg.h>
 #include <dev/atkbdc/atkbdcreg.h>
 
+static timeout_t	atkbd_timeout;
 static void		atkbd_shutdown_final(void *v);
 
 int
@@ -113,6 +114,12 @@ atkbd_attach_unit(int unit, keyboard_t *
 		return error;
 #endif
 
+	/*
+	 * This is a kludge to compensate for lost keyboard interrupts.
+	 * A similar code used to be in syscons. See below. XXX
+	 */
+	atkbd_timeout(*kbd);
+
 	if (bootverbose)
 		(*sw->diag)(*kbd, bootverbose);
 
@@ -122,6 +129,53 @@ atkbd_attach_unit(int unit, keyboard_t *
 	return 0;
 }
 
+static void
+atkbd_timeout(void *arg)
+{
+	keyboard_t *kbd;
+	int s;
+
+	/*
+	 * The original text of the following comments are extracted 
+	 * from syscons.c (1.287)
+	 * 
+	 * With release 2.1 of the Xaccel server, the keyboard is left
+	 * hanging pretty often. Apparently an interrupt from the
+	 * keyboard is lost, and I don't know why (yet).
+	 * This ugly hack calls the low-level interrupt routine if input
+	 * is ready for the keyboard and conveniently hides the problem. XXX
+	 *
+	 * Try removing anything stuck in the keyboard controller; whether
+	 * it's a keyboard scan code or mouse data. The low-level
+	 * interrupt routine doesn't read the mouse data directly, 
+	 * but the keyboard controller driver will, as a side effect.
+	 */
+	/*
+	 * And here is bde's original comment about this:
+	 *
+	 * This is necessary to handle edge triggered interrupts - if we
+	 * returned when our IRQ is high due to unserviced input, then there
+	 * would be no more keyboard IRQs until the keyboard is reset by
+	 * external powers.
+	 *
+	 * The keyboard apparently unwedges the irq in most cases.
+	 */
+	s = spltty();
+	kbd = (keyboard_t *)arg;
+	if (kbdd_lock(kbd, TRUE)) {
+		/*
+		 * We have seen the lock flag is not set. Let's reset
+		 * the flag early, otherwise the LED update routine fails
+		 * which may want the lock during the interrupt routine.
+		 */
+		kbdd_lock(kbd, FALSE);
+		if (kbdd_check_char(kbd))
+			kbdd_intr(kbd, NULL);
+	}
+	splx(s);
+	timeout(atkbd_timeout, arg, hz/10);
+}
+
 /* LOW-LEVEL */
 
 #define ATKBD_DEFAULT	0

Modified: projects/calloutng/sys/dev/atkbdc/atkbd_atkbdc.c
==============================================================================
--- projects/calloutng/sys/dev/atkbdc/atkbd_atkbdc.c	Sat Dec 22 09:37:34 2012	(r244582)
+++ projects/calloutng/sys/dev/atkbdc/atkbd_atkbdc.c	Sat Dec 22 13:02:03 2012	(r244583)
@@ -47,8 +47,6 @@ __FBSDID("$FreeBSD$");
 typedef struct {
 	struct resource	*intr;
 	void		*ih;
-	keyboard_t	*kbd;
-	struct callout	callout;
 } atkbd_softc_t;
 
 static devclass_t	atkbd_devclass;
@@ -58,7 +56,6 @@ static int	atkbdprobe(device_t dev);
 static int	atkbdattach(device_t dev);
 static int	atkbdresume(device_t dev);
 static void	atkbdintr(void *arg);
-static timeout_t atkbdtimeout;
 
 static device_method_t atkbd_methods[] = {
 	DEVMETHOD(device_identify,	atkbdidentify),
@@ -116,18 +113,18 @@ static int
 atkbdattach(device_t dev)
 {
 	atkbd_softc_t *sc;
+	keyboard_t *kbd;
 	u_long irq;
 	int flags;
 	int rid;
 	int error;
 
 	sc = device_get_softc(dev);
-	callout_init(&sc->callout, TRUE);
 
 	rid = KBDC_RID_KBD;
 	irq = bus_get_resource_start(dev, SYS_RES_IRQ, rid);
 	flags = device_get_flags(dev);
-	error = atkbd_attach_unit(device_get_unit(dev), &sc->kbd,
+	error = atkbd_attach_unit(device_get_unit(dev), &kbd,
 				  device_get_unit(device_get_parent(dev)),
 				  irq, flags);
 	if (error)
@@ -138,17 +135,11 @@ atkbdattach(device_t dev)
 	if (sc->intr == NULL)
 		return ENXIO;
 	error = bus_setup_intr(dev, sc->intr, INTR_TYPE_TTY, NULL, atkbdintr,
-			       sc->kbd, &sc->ih);
-	if (error) {
+			       kbd, &sc->ih);
+	if (error)
 		bus_release_resource(dev, SYS_RES_IRQ, rid, sc->intr);
-		return (error);
-	}
 
-	/*
-	 * This is a kludge to compensate for lost keyboard interrupts.
-	 */
-	atkbdtimeout(dev);
-	return (0);
+	return error;
 }
 
 static int
@@ -159,14 +150,16 @@ atkbdresume(device_t dev)
 	int args[2];
 
 	sc = device_get_softc(dev);
-	kbd = sc->kbd;
-	kbd->kb_flags &= ~KB_INITIALIZED;
-	args[0] = device_get_unit(device_get_parent(dev));
-	args[1] = rman_get_start(sc->intr);
-	kbdd_init(kbd, device_get_unit(dev), &kbd, args,
-	    device_get_flags(dev));
-	kbdd_clear_state(kbd);
-
+	kbd = kbd_get_keyboard(kbd_find_keyboard(ATKBD_DRIVER_NAME,
+						 device_get_unit(dev)));
+	if (kbd) {
+		kbd->kb_flags &= ~KB_INITIALIZED;
+		args[0] = device_get_unit(device_get_parent(dev));
+		args[1] = rman_get_start(sc->intr);
+		kbdd_init(kbd, device_get_unit(dev), &kbd, args,
+		    device_get_flags(dev));
+		kbdd_clear_state(kbd);
+	}
 	return 0;
 }
 
@@ -179,27 +172,4 @@ atkbdintr(void *arg)
 	kbdd_intr(kbd, NULL);
 }
 
-static void
-atkbdtimeout(void *arg)
-{
-	device_t dev = (device_t)arg;
-	atkbd_softc_t *sc;
-	keyboard_t *kbd;
-
-	sc = device_get_softc(dev);
-	kbd = sc->kbd;
-	if (kbdd_lock(kbd, TRUE)) {
-		/*
-		 * We have seen the lock flag is not set. Let's reset
-		 * the flag early, otherwise the LED update routine fails
-		 * which may want the lock during the interrupt routine.
-		 */
-		kbdd_lock(kbd, FALSE);
-		if (kbdd_check_char(kbd))
-			kbdd_intr(kbd, NULL);
-	}
-	callout_reset_bt(&sc->callout, ticks2bintime(hz), zero_bt,
-	    atkbdtimeout, dev, C_PREL(0) | C_HARDCLOCK);
-}
-
 DRIVER_MODULE(atkbd, atkbdc, atkbd_driver, atkbd_devclass, 0, 0);



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