Re: ukbd_cnattach() called unconditionally

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

On Sun, Sep 23, 2018 at 03:33:37PM -0300, Jared McNeill wrote:
> It is only called for the device matching /chosen stdout-path. On all of our sunxi boards to date, this points to the debug UART.
> 
> The ?console=fb? code replaces the value of this property with the path to the frame buffer, which would cause it to be called.
> 
> However...
> 
> armv7/arm64.img don?t set console=fb because we need a config that works on all devices. WSDISPLAY_MULTICONS attempts to solve the ?work everywhere" problem by saying ok, lets use the stdout-path device as console, and if a wsdisplay device attaches, use it too. Downside is that you don?t get _early_ fb console on things like Pinebook, but what can you do..
> 
> So in the general case of ?no console= passed on the kernel cmdline?, console_consinit will happen for the UART driver, and simplefb_attach will take over from there.
> 
> We could probably call ukbd_cnattach from both simplefb_console_consinit and simplefb_attach, but then all display drivers need to start adopting this hack. Maybe we can do it in fdt_device_register instead? Something like this:
> 
> #if NUKBD > 0
>         if (device_is_a(self, "wsdisplay")) {
>                 struct wsdisplay_softc *sc = device_private(self);
>                 if (sc->sc_isconsole)
>                         ukbd_cnattach();
>         }
> #endif

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).
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.

If there's no objection I'll commit this

-- 
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 09:30:37 -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 09:30:37 -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 09:30:37 -0000
@@ -62,6 +62,7 @@ __KERNEL_RCSID(0, "$NetBSD: fdt_machdep.
 #include <sys/md5.h>
 
 #include <dev/cons.h>
+#include <dev/wscons/wsdisplayvar.h>
 #include <uvm/uvm_extern.h>
 
 #include <sys/conf.h>
@@ -116,6 +117,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 +425,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 +539,6 @@ consinit(void)
 
 	cons->consinit(&faa, uart_freq);
 
-#if NUKBD > 0
-	ukbd_cnattach();	/* allow USB keyboard to become console */
-#endif
-
 	initialized = true;
 }
 
@@ -621,6 +620,18 @@ fdt_device_register(device_t self, void 
 }
 
 static void
+fdt_device_register_post_config(device_t self, void *aux)
+{
+#if NUKBD > 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;