From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Christian Lamparter <chunkeey@gmail.com>,
linux-usb@vger.kernel.org, linux-leds@vger.kernel.org,
Jacek Anaszewski <jacek.anaszewski@gmail.com>
Subject: [v2] leds: fix regression in usbport led trigger
Date: Fri, 18 Jan 2019 10:13:37 +0100 [thread overview]
Message-ID: <20190118091337.GA17432@kroah.com> (raw)
On Fri, Jan 18, 2019 at 10:07:05AM +0100, Uwe Kleine-König wrote:
> 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.
Ok, that helps a lot then.
> 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.
But how do you know how many ports are present? And this feels like it
ends up being a "custom api" for each different type of led that is
present in the system. Or is that already the case?
thanks,
greg k-h
next reply other threads:[~2019-01-18 9:13 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-18 9:13 Greg Kroah-Hartman [this message]
-- strict thread matches above, loose matches on Subject: below --
2019-01-18 9:44 [v2] leds: fix regression in usbport led trigger Uwe Kleine-König
2019-01-18 9:28 Greg Kroah-Hartman
2019-01-18 9:22 Uwe Kleine-König
2019-01-18 9:07 Uwe Kleine-König
2019-01-18 8:56 Greg Kroah-Hartman
2019-01-11 16:29 Christian Lamparter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190118091337.GA17432@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=chunkeey@gmail.com \
--cc=jacek.anaszewski@gmail.com \
--cc=linux-leds@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=u.kleine-koenig@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.