gpioled: defer gpio pin writes to a taskqueue

[seuros]

Summary

led(4) invoked the led_t callback with its mutex held, including from the
blink callout, so the callback must not sleep.

PR: 251032

MFC After: 1 week

[adrian]

sys/dev/gpio/gpioled_fdt.c

74

this could result in the led updates losing state is done by parallel threads though, right?

i wonder if we need to clean up locking in led to mirror whats done in spi/i2c/etc driver land...

[seuros]

sys/dev/gpio/gpioled_fdt.c

74

The task stores the requested state, not just an “update” flag. gpioled_update() reads the latest state when it runs. If several updates happen quickly, some intermediate states may be skipped, but the final state is always applied.

it already serializes callbacks with led_mtx, including the blining. So the state updates are sorted. This change only moves the GPIO write outside that thread.

I agree that the locking could be improved, but that affects every led_create() user. I think that should be a separate change.

I tried it with the Raspberries 2/3/5, i want to pair with Martin to test a robust solution across multiple SoCs and consumers.

[adrian]

sys/dev/gpio/gpioled_fdt.c

74

The task stores the requested state, not just an “update” flag. gpioled_update() reads the latest state when it runs. If several updates happen quickly, some intermediate states may be skipped, but the final state is always applied.

it already serializes callbacks with led_mtx, including the blining. So the state updates are sorted. This change only moves the GPIO write outside that thread.

I agree that the locking could be improved, but that affects every led_create() user. I think that should be a separate change.

I tried it with the Raspberries 2/3/5, i want to pair with Martin to test a robust solution across multiple SoCs and consumers.

ok, just add a comment above the task that states this.

ie "don't do this inside the locked section" in gpioled_control, and then "this pushes the last updated state, not each individual LED state update" above gpioled_update(). That way it'll be much clearer to people what the choices are here.

(Then i'll approve and you can land!)

[seuros]

add comments