[PATCH 2/8] leds: nic78bx: explicitly unregister LEDs at module's shutdown

[George Stark]

Hello Christophe.

Thanks for the review.


On 11/6/23 11:13, Christophe Leroy wrote:
 >
 >
 > Le 25/10/2023 à 15:07, George Stark a écrit :
 >> LEDs are registered using devm_led_classdev_register() and automatically
 >> unregistered after module's remove(). led_classdev_unregister() calls
 >> led_set_brightness() to turn off the LEDs and module's appropriate 
callback
 >> uses resources those were destroyed already in module's remove().
 >> So explicitly unregister LEDs at module shutdown.
 >>
 >> Signed-off-by: George Stark <gnstark at salutedevices.com>
 >> ---
 >>    drivers/leds/leds-nic78bx.c | 4 ++++
 >>    1 file changed, 4 insertions(+)
 >>
 >> diff --git a/drivers/leds/leds-nic78bx.c b/drivers/leds/leds-nic78bx.c
 >> index f196f52eec1e..12b70fcad37f 100644
 >> --- a/drivers/leds/leds-nic78bx.c
 >> +++ b/drivers/leds/leds-nic78bx.c
 >> @@ -170,6 +170,10 @@ static int nic78bx_probe(struct platform_device 
*pdev)
 >>    static int nic78bx_remove(struct platform_device *pdev)
 >>    {
 >>    	struct nic78bx_led_data *led_data = platform_get_drvdata(pdev);
 >> +	int i;
 >> +
 >> +	for (i = 0; i < ARRAY_SIZE(nic78bx_leds); i++)
 >> +		devm_led_classdev_unregister(&pdev->dev, &nic78bx_leds[i].cdev);
 >
 > The whole purpose of devm_ functions is that you don't need to call
 > unregister when removing the driver as the dev core will do it for you.
 > I understand your problem but I think this is not the solution.

I agree my solution is questionable although 
devm_led_classdev_unregister() is exists for some reason.

Probably it's not the best solution to remove led_set_brightness() from 
led_classdev_unregister() either.
Or we'll have to patch a lot of drivers which use led subsystem to call 
led_set_brightness() manually to keep leds' previous behavior.

Well if we can't easily unregister leds before module's remove() 
callback is completed may be we can get rid of remove() itself and 
manage all resources using devm API. In that case by the time 
led_set_brightness() is called from led_classdev_unregister() all 
dependent resources will be alive.
I'll try it in the next patch series.

 >
 >>
 >>    	/* Lock LED register */
 >>    	outb(NIC78BX_LOCK_VALUE,
-- 
Best regards
George