Am 18.01.2016 um 01:20 schrieb Milo Kim: > On 01/17/2016 06:34 AM, Heiner Kallweit wrote: >> In led_classdev_unregister the LED gets switched off. >> This is fine when the driver module is removed but causes issues >> when the physical LED device is removed (e.g. USB LED devices). >> >> In case of the thingm driver (hid/hid-thingm.c) it complains with >> ENODEV because the physical LED device is no longer available. > > I'd like to understand this situation clearly. > > rmmod hid_thingm > -> detach the USB LED device > -> -ENODEV is reported. > > Is it correct? I think -ENODEV seems reasonable because HID device is gone. Could you tell us what the problem is? > Sure, this is reasonable. The problem is that this causes dev_err messages. And if it's a normal situation it shouldn't result in error messages (at least not on error level). >> When I switched this driver to use the generic workqueue in the >> LED core then this error was also propagated to set_brightness_delayed >> and it complains with "Setting an LED's brightness failed (-19)". >> >> Recent commit d1aa577f5e19 [turn off the LED and wait for completion >> on unregistering LED class device] tackled a first potential issue >> in led_classdev_unregister but it seems like the case of removal >> of the physical LED device hasn't been considered yet. > > What happens if resetting only this commit? > It's not that something with this commit is wrong. The behaviour was the same before, I just mention it because you addressed the same part of the code and maybe stumbled across the same issue. >> At a first glance I see no way for the LED core to tell between >> the two unregister cases (driver module removal vs. physical LED >> device removal), but maybe I miss something. >> >> If we can't tell between the two cases them I'm not sure what's the >> best solution: Not touching the brightness is general in >> led_classdev_unregister, live with the situation as it is or add >> a special handling for ENODEV. > > We may handle this by using internal flag in LED subsystem, but I'd like to understand what the problem is and what expected behavior is. > Ideally we would detect that the device is gone in the unregister function and skip trying to write to it. > Best regards, > Milo > Regards, Heiner -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
