Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

[Pavel Machek <pavel@ucw.cz>]

On Thu 2016-08-25 11:04:38, Jacek Anaszewski wrote:
> On 08/25/2016 10:29 AM, Rafał Miłecki wrote:
> >On 25 August 2016 at 10:03, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
> >>On 08/24/2016 07:52 PM, Rafał Miłecki wrote:
> >>>
> >>>From: Rafał Miłecki <rafal@milecki.pl>
> >>>
> >>>This commit adds a new trigger responsible for turning on LED when USB
> >>>device gets connected to the specified USB port. This can can useful for
> >>>various home routers that have USB port(s) and a proper LED telling user
> >>>a device is connected.
> >>>
> >>>The trigger gets its documentation file but basically it just requires
> >>>specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
> >>>
> >>>During work on this trigger there was a plan to add DT bindings for it,
> >>>but there wasn't an agreement on the format yet. This can be worked on
> >>>later, a sysfs interface is needed anyway for platforms not using DT.
> >>>
> >>>Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >>>---
> >>>V2: Trying to add DT support, idea postponed as it will take more time
> >>>    to discuss the bindings.
> >>>V3: Fix typos in commit and Documentation (thanks Jacek!)
> >>>    Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
> >>>    Check if there is USB device connected after adding new USB port
> >>>    Fix memory leak or two
> >>>V3.5: Fix e-mail address (thanks Matthias)
> >>>      Simplify conditions in usbport_trig_notify (thx Matthias)
> >>>      Make "ports" a subdirectory with file per port, to match one value
> >>>      per file sysfs rule (thanks Greg)
> >>>      As "ports" couldn't be used for adding and removing ports anymore,
> >>>      there are now "new_port" and "remove_port". Having them makes this
> >>>      API also common with e.g. pci and usb buses.
> >>
> >>
> >>Now writing new_port with "1-1" produces a file named "1-1" in the
> >>ports directory with 000 permissions. I think that what Greg had
> >>on mind by referring to "one value per file" rule was a set of
> >>files representing ports, like "1-1 1-2 2-1", and each file should be
> >>readable/writeable.
> >>
> >>For instance "echo 1 > 1-1" would enable the trigger for the port 1-1
> >>and "echo 0 > 1-1" would disable it. The problem is that we don't know
> >>the number of required ports at compilation time and the sysfs
> >>attributes would have to be dynamically created on driver instantiation.
> >>What is more, as the USB ports can dynamically appear/disappear in the
> >>system, the files would have to be created/removed accordingly during
> >>LED class device lifetime, which is not the best design for the sysfs
> >>interface I think.

Could we add a flag to the USB port, instead? That way, USB code would
take care of creating the enable file in its own directory.

Is per-port control needed? Would just global control be sufficient?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html