On Fri, Jan 18, 2019 at 09:56:52AM +0100, Greg Kroah-Hartman wrote: > On Fri, Jan 11, 2019 at 05:29:45PM +0100, Christian Lamparter wrote: > > The patch "usb: simplify usbport trigger" together with "leds: triggers: > > add device attribute support" caused an regression for the usbport > > trigger. it will no longer enumerate any active usb hub ports under the > > "ports" directory in the sysfs class directory, if the usb host drivers > > are fully initialized before the usbport trigger was loaded. > > > > The reason is that the usbport driver tries to register the sysfs > > entries during the activate() callback. And this will fail with -2 / > > ENOENT because the patch "leds: triggers: add device attribute support" > > made it so that the sysfs "ports" group was only being added after the > > activate() callback succeeded. > > > > This version of the patch reverts parts of the "usb: simplify usbport > > trigger" patch and restores usbport trigger's functionality. > > This feels like going backwards, as a driver should not be adding and > removing sysfs groups, because you race with userspace. Userspace has > no idea that the new sysfs files were added or removed, right? I think this is not an issue as the led trigger core code calls kobject_uevent_env() when the trigger is set. But I still agree that there seems to be something "unusual" about the usb led trigger ... Instead of providing a sysfs-file per port to make said port trigger the led, I'd prefer a single file that you can use to add and remove ports from the trigger. With this change the driver can properly benefit from the attribute handling of the led-trigger core and is more in line with the other triggers. > I'll apply this, but this feels like a problem in the led api if the > above really is true. > > also, please wrap your changelogs at 72 columns, making it easier to > read. I did that here for you... In the mail I received the changelog looked fine (though I didn't check the actual width). Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
