On Mon, Jun 22, 2026 at 05:06:13PM -0400, John Troy wrote:
> On 6/22/26 4:39 PM, John Troy wrote:
> > 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
>
> Apologies, Thunderbird mangled that diff. I think I found the right knob
> to stop it.
>
It's all kernel locked. You don't need atomic operations here.
>
> 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)
> {
>
>
