Re: Panic when pressing Caps Lock in the console

John Troy <[email protected]> Mon, 22 Jun 2026 16:39:21 -0400
Newsgroups gmane.os.openbsd.bugs
Message-ID <[email protected]>
On 6/19/26 6:54 AM, Stefan Sperling wrote:
> Nice find. I suspect a task should be used here, perhaps depending on
> the type of child HID device. We will definitely want interrupts to
> be used when ihidev is serving touchpads since jcs@ spent quite a lot
> of effort on avoiding polling mode for them.
> 

Thanks for taking a look! As far as I can tell, ikbd_set_leds() is the
only caller of ihidev_send_report(), so modifying it wouldn't interfere
with touchpads. That said, it looked simpler to put the task in the ikbd
code where only a single byte needs to be handed off to the task rather
than a buffer. The patch below is patterned after how wskbd handles
backlight/brightness. Let me know if this really does belong in ihidev,
or somewhere else, instead.

-John


diff --git a/sys/dev/i2c/ikbd.c b/sys/dev/i2c/ikbd.c
index 6551b918f01..6d058d5b7e2 100644
--- a/sys/dev/i2c/ikbd.c
+++ b/sys/dev/i2c/ikbd.c
@@ -23,6 +23,8 @@
  #include <sys/device.h>
  #include <sys/ioctl.h>
  #include <sys/timeout.h>
+#include <sys/atomic.h>
+#include <sys/task.h>
  
  #include <dev/i2c/i2cvar.h>
  #include <dev/i2c/ihidev.h>
@@ -38,6 +40,8 @@ struct ikbd_softc {
  	struct ihidev	sc_hdev;
  #define sc_ledsize	sc_hdev.sc_osize
  	struct hidkbd	sc_kbd;
+	struct task	sc_led_task;
+	u_int		sc_led_report;
  	int		sc_spl;
  };
  
@@ -55,6 +59,7 @@ const struct wskbd_consops ikbd_consops = {
  
  int	ikbd_enable(void *, int);
  void	ikbd_set_leds(void *, int);
+void	ikbd_set_leds_task(void *);
  int	ikbd_ioctl(void *, u_long, caddr_t, int, struct proc *);
  
  const struct wskbd_accessops ikbd_accessops = {
@@ -117,6 +122,8 @@ ikbd_attach(struct device *parent, struct device *self, void *aux)
  
  	printf("\n");
  
+	task_set(&sc->sc_led_task, ikbd_set_leds_task, sc);
+
  	if (kbd->sc_console_keyboard) {
  		extern struct wskbd_mapdata ukbd_keymapdata;
  
@@ -133,8 +140,11 @@ ikbd_detach(struct device *self, int flags)
  {
  	struct ikbd_softc *sc = (struct ikbd_softc *)self;
  	struct hidkbd *kbd = &sc->sc_kbd;
+	int rv;
  
-	return hidkbd_detach(kbd, flags);
+	rv = hidkbd_detach(kbd, flags);
+	taskq_del_barrier(systq, &sc->sc_led_task);
+	return rv;
  }
  
  void
@@ -173,11 +183,22 @@ ikbd_set_leds(void *v, int leds)
  	uint8_t res;
  
  	if (sc->sc_ledsize && hidkbd_set_leds(kbd, leds, &res) != 0) {
-		ihidev_send_report((struct device *)sc->sc_hdev.sc_parent,
-		    sc->sc_hdev.sc_report_id, &res, 1);
+		atomic_store_int(&sc->sc_led_report, res);
+		task_add(systq, &sc->sc_led_task);
  	}
  }
  
+void
+ikbd_set_leds_task(void *v)
+{
+	struct ikbd_softc *sc = v;
+	uint8_t res;
+
+	res = atomic_load_int(&sc->sc_led_report);
+	ihidev_send_report((struct device *)sc->sc_hdev.sc_parent,
+	    sc->sc_hdev.sc_report_id, &res, 1);
+}
+
  int
  ikbd_ioctl(void *v, u_long cmd, caddr_t data, int flag, struct proc *p)
  {