Re: ukbd_cnattach() called unconditionally

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Tue, Sep 25, 2018 at 07:19:37AM -0300, Jared McNeill wrote:
> On Tue, 25 Sep 2018, Manuel Bouyer wrote:
> 
> > I've come up with the attached patch, which I successfully tested in both
> > console=serial and console=fb cases (actually I use both, depending on
> > environnement).
> 
> Also please make sure it works in the case where console is not specified.
> Two scenarios here -- one w/ UART only, and one w/ UART + FB configured.

I also tested the second case, it's no worse than before (MULTICONS has never
really worked for me).

> 
> > wsdisplay_softc content is not publically accessible, so I added a
> > wsdisplay_isconsole() function to access sc_isconsole.
> > fdt_device_register() is called before the device is attached, so sc_isconsole
> > is not available at this point. So I added a device_register_post_config()
> > callback for this.
> 
> Since you're using a function instead of the softc directly, you'll need to
> guard against NUKBD > 0 && NWSDISPLAY > 0 instead.

Sure. Updated patch attached.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: sys/dev/wscons/wsdisplay.c
===================================================================
RCS file: /cvsroot/src/sys/dev/wscons/wsdisplay.c,v
retrieving revision 1.145
diff -u -p -u -r1.145 wsdisplay.c
--- sys/dev/wscons/wsdisplay.c	18 Dec 2017 22:44:30 -0000	1.145
+++ sys/dev/wscons/wsdisplay.c	25 Sep 2018 10:27:28 -0000
@@ -2127,6 +2127,13 @@ wsdisplay_reset(device_t dv, enum wsdisp
 	}
 }
 
+
+bool
+wsdisplay_isconsole(struct wsdisplay_softc *sc)
+{
+	return sc->sc_isconsole;
+}
+
 /*
  * Interface for (external) VT switch / process synchronization code
  */
Index: sys/dev/wscons/wsdisplayvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/wscons/wsdisplayvar.h,v
retrieving revision 1.52
diff -u -p -u -r1.52 wsdisplayvar.h
--- sys/dev/wscons/wsdisplayvar.h	19 May 2017 19:22:33 -0000	1.52
+++ sys/dev/wscons/wsdisplayvar.h	25 Sep 2018 10:27:28 -0000
@@ -208,6 +208,8 @@ int wsdisplay_stat_ioctl(struct wsdispla
 int wsdisplay_cfg_ioctl(struct wsdisplay_softc *, u_long, void *,
 			int, struct lwp *);
 
+bool wsdisplay_isconsole(struct wsdisplay_softc *);
+
 struct wsdisplayio_edid_info;
 int wsdisplayio_get_edid(device_t, struct wsdisplayio_edid_info *);
 
Index: sys/arch/evbarm/fdt/fdt_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/evbarm/fdt/fdt_machdep.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 fdt_machdep.c
--- sys/arch/evbarm/fdt/fdt_machdep.c	27 Aug 2018 09:54:16 -0000	1.34
+++ sys/arch/evbarm/fdt/fdt_machdep.c	25 Sep 2018 10:27:28 -0000
@@ -38,6 +38,7 @@ __KERNEL_RCSID(0, "$NetBSD: fdt_machdep.
 #include "opt_cpuoptions.h"
 
 #include "ukbd.h"
+#include "wsdisplay.h"
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -84,6 +85,9 @@ __KERNEL_RCSID(0, "$NetBSD: fdt_machdep.
 #if NUKBD > 0
 #include <dev/usb/ukbdvar.h>
 #endif
+#if NWSDISPLAY > 0
+#include <dev/wscons/wsdisplayvar.h>
+#endif
 
 #ifdef MEMORY_DISK_DYNAMIC
 #include <dev/md.h>
@@ -116,6 +120,7 @@ extern char KERNEL_BASE_phys[];
 
 static void fdt_update_stdout_path(void);
 static void fdt_device_register(device_t, void *);
+static void fdt_device_register_post_config(device_t, void *);
 static void fdt_cpu_rootconf(void);
 static void fdt_reset(void);
 static void fdt_powerdown(void);
@@ -423,6 +428,7 @@ initarm(void *arg)
 	cpu_reset_address = fdt_reset;
 	cpu_powerdown_address = fdt_powerdown;
 	evbarm_device_register = fdt_device_register;
+	evbarm_device_register_post_config = fdt_device_register_post_config;
 	evbarm_cpu_rootconf = fdt_cpu_rootconf;
 
 	/* Talk to the user */
@@ -536,10 +542,6 @@ consinit(void)
 
 	cons->consinit(&faa, uart_freq);
 
-#if NUKBD > 0
-	ukbd_cnattach();	/* allow USB keyboard to become console */
-#endif
-
 	initialized = true;
 }
 
@@ -621,6 +623,18 @@ fdt_device_register(device_t self, void 
 }
 
 static void
+fdt_device_register_post_config(device_t self, void *aux)
+{
+#if NUKBD > 0 && NWSDISPLAY > 0
+	if (device_is_a(self, "wsdisplay")) {
+		struct wsdisplay_softc *sc = device_private(self);
+		if (wsdisplay_isconsole(sc))
+			ukbd_cnattach();
+	}
+#endif
+}
+
+static void
 fdt_cpu_rootconf(void)
 {
 	device_t dev;