Re: [PATCH V5] leds: trigger: Introduce a USB port trigger

[RafaÅ MiÅecki]

On 9 September 2016 at 11:34, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
> On Thu, Sep 08, 2016 at 06:08:24PM +0200, RafaÅ MiÅecki wrote:
>> From: RafaÅ MiÅecki <rafal@xxxxxxxxxx>
>>
>> This commit adds a new trigger responsible for turning on LED when USB
>> device gets connected to the selected 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
>> enabling it and selecting USB ports (e.g. echo 1 > ports/usb1-1).
>>
>> There was a long discussion on design of this driver. Its current state
>> is a result of picking them most adjustable solution as others couldn't
>> handle all cases.
>>
>> 1) It wasn't possible for the driver to register separated trigger for
>>    each USB port. Some physical USB ports are handled by more than one
>>    controller and so by more than one USB port. E.g. USB 2.0 physical
>>    port may be handled by OHCI's port and EHCI's port.
>>    It's also not possible to assign more than 1 trigger to a single LED
>>    and implementing such feature would be tricky due to syncing triggers
>>    and sysfs conflicts with old triggers.
>>
>> 2) Another idea was to register trigger per USB hub. This wouldn't allow
>>    handling devices with multiple USB LEDs and controllers (hubs)
>>    controlling more than 1 physical port. It's common for hubs to have
>>    few ports and each may have its own LED.
>>
>> This final trigger is highly flexible. It allows selecting any USB ports
>> for any LED. It was also modified (compared to the initial version) to
>> allow choosing ports rather than having user /guess/ proper names. It
>> was successfully tested on SmartRG SR400ac which has 3 USB LEDs,
>> 2 physical ports and 3 controllers.
>>
>> Another planned feature is support for LED reacting to the USB activity.
>> This can be implemented with another sysfs file for setting mode. The
>> default mode wouldn't change so there won't be ABI breakage and such
>> feature can be safely implemented later.
>>
>
> It has such driver at: drivers/usb/common/led.c

Oh, great, I had no idea about that. So if it comes to adding activity
support, we'll have to well discuss that. We may e.g. not implement
that or move existing feature into more generic usbport trigger or
partially duplicate this feature.

Anyway, I hope this note on "usb-gadget" and "usb-host" triggers won't
stop our work on "usbport". This driver implements different thing for
now (discovering device in a USB port) and I don't think "usb-gadget"
/ "usb-host" could be extended to handle such cases.

Thanks for a hint though, I'll definitely keep that in mind during
further development!

-- 
RafaÅ