gpioled(4), the taskqueue deferral

Source: sys/dev/gpio/gpioled_fdt.c, with the contract set by sys/dev/led/led.c. Fix landed upstream as D59263 (PR 251032).

What it is

gpioled binds device-tree leds nodes to led(4), so a heartbeat or disk LED shows up as /dev/led/ACT. Small driver, one interesting problem: on a Raspberry Pi the ACT LED does not hang off a memory-mapped GPIO controller. It hangs off the VideoCore firmware GPIO expander, and setting that pin means a mailbox round trip that sleeps.

The reversal

WITNESS reports it on every RPi3 boot, class sleepable after non-sleepable:

lock order reversal: (sleepable after non-sleepable)
 1st LED mtx (sleep mutex) @ sys/dev/led/led.c:295
 2nd Raspberry Pi firmware gpio (sx) @ sys/arm/broadcom/bcm2835/raspberrypi_gpio.c:250

led(4) has one global mutex, and it is also the callout mutex: the blink timer runs from softclock with led_mtx held. Every path into a driver’s LED callback holds it:

The sleep is real, not a WITNESS technicality: the thread parks waiting for firmware while holding led_mtx. Worst case it does so from softclock, ten times a second, for as long as the LED blinks. And the shape is not an RPi peculiarity: any LED behind an I2C or SPI GPIO expander has the same chain.

Why the fix is where it is

Three places could have taken the patch, two of them wrong:

• led.c: its contract is that callbacks are called under led_mtx, and that is structural, because the mutex doubles as the callout lock. Dropping it around the callback races the list walk and blink state. Deferring in led.c would tax every LED driver in the tree, most of which are single register writes that want to be synchronous for blink timing.
• raspberrypi_gpio: cannot be fixed. Talking to firmware sleeps. That is what it is.
• gpioled_fdt: the one place that knows both contracts, “callback must not sleep” above and “pin write may sleep” below. The translation belongs at the boundary.

Patching only the attach-time call site WITNESS happened to report would have silenced the message and left the 10 Hz softclock sleep in place.

The fix

The callback stores the requested state atomically and enqueues a task; the task writes whatever the latest requested state is. Enqueueing an already-pending task is a no-op, so rapid toggles coalesce: last state wins, intermediate states may never reach the pin. For an LED that is the correct semantic, not a compromise.

taskqueue_thread exists before any device attaches (SI_SUB_TASKQ precedes SI_SUB_CONFIGURE), so the attach-time initial state is safe too.

Detach ordering

Three steps, order load-bearing:

1. led_destroy: after this, no new callback invocations exist.
2. taskqueue_drain: waits out any task already in flight. Step 1 guarantees nothing re-enqueues, so the drain is terminal.
3. gpio_pin_release: only now, so the task can never touch a released pin.

And TASK_INIT runs before led_create_state at attach, because led_create_state fires the callback synchronously for the initial state.

Verified

Two RPi3B+ boards (dory, skiff), WITNESS kernels: boot LOR count went from one to zero, and /dev/led/ACT still answers 0, 1, and blink patterns.

One loose end, deliberately left: the hints-based sibling sys/dev/gpio/gpioled.c still writes the pin synchronously. It only attaches on hinted platforms, where sleepable GPIO controllers do not occur in practice; FDT platforms, where they do, all go through gpioled_fdt.