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

["Rafał Miłecki" <zajec5@gmail.com>]

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.
>
> Therefore, maybe it would be good to follow the "triggers" sysfs
> attribute pattern, where it lists the available LED triggers?
>
> The question is whether there is some mechanism available for
> notifying addition/removal of a USB port?

Every port is part of some hub and every hub (even root one) is a USB
device. So I could just get struct usb_device and check its "maxchild"
property. If e.g. I get USB device "1-1" with maxchild 4, I know there
are:
1-1.1
1-1.2
1-1.3
1-1.4

So the sysfs structure you suggested would be possible, I just don't
know if it's preferred one or not.

Confirmation: yes, right now I create simple files with chmod 000 for
every port added to the usbport observable list.


> Also a description of the device connected to the port would be a nice
> feature, however I am not certain about the feasibility thereof.

What kind of description do you mean? Where should it be used / where
should it appear?

-- 
Rafał