Re: [PATCH v7 2/2] leds: is32fl3207: Add controller driver
From: Ahmad Byagowi
Date: Thu Sep 10 2026 - 13:29:49 EST
Lee,
Thanks for the detailed review. I will address the cleanup points in v8:
remove the redundant led_cdev pointer, populate brightness directly in
the class device, use chip consistently, use loop-local indices, and
fix the wrapping, alignment, braces, and spacing. I will also name the
current-scaling factors and the two PWM bytes per channel, and explain
the output-disable fallback when the initial PWM latch fails.
I will drop the unsupported-property list for multicolor components.
The binding already disallows those properties on component nodes; I
will clarify that brightness, default state, and shutdown retention
belong to the group.
> Is it even architecturally possible to set brightness on a shutdown or
> disabled LED? That sounds like a bigger problem.
The guard is intended for system shutdown, rather than normal LED
unregistration. In the current LED tree, led_classdev_unregister() stops
software blinking and flushes set_brightness_work. The system shutdown
path does not call that function, and i2c_device_shutdown() calls the
driver's shutdown callback without draining LED brightness work.
A previously queued brightness callback can therefore run after our
shutdown callback. Both paths take the same controller mutex; setting
shutting_down under that lock prevents a later callback from writing
registers or changing the final retained state. This also matters when
some LEDs retain their state and the controller must remain enabled.
I will retain the guard and add a comment explaining that distinction.
This is based on inspection of the shutdown and LED-core paths, not a
hardware reproduction of the race.
Regards,
Ahmad