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

Reply via email to