linux-kernel - Re: [PATCH V4] leds: trigger: Introduce an USB port trigger

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.44L0.1609011032040.1386-100000@iolanthe.rowland.org>
Date:   Thu, 1 Sep 2016 10:36:01 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Jacek Anaszewski <j.anaszewski@...sung.com>
cc:     Rafał Miłecki <zajec5@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Richard Purdie <rpurdie@...ys.net>,
        Felipe Balbi <balbi@...nel.org>,
        Peter Chen <hzpeterchen@...il.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        Rafał Miłecki <rafal@...ecki.pl>,
        Jonathan Corbet <corbet@....net>,
        Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Stephan Linz <linz@...pro.net>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        "open list:LED SUBSYSTEM" <linux-leds@...r.kernel.org>
Subject: Re: [PATCH V4] leds: trigger: Introduce an USB port trigger

On Thu, 1 Sep 2016, Jacek Anaszewski wrote:

> On 09/01/2016 07:25 AM, Rafał Miłecki wrote:
> > On 31 August 2016 at 21:00, Rafał Miłecki <zajec5@...il.com> wrote:
> >> On 31 August 2016 at 20:23, Alan Stern <stern@...land.harvard.edu> wrote:
> >>> On Tue, 30 Aug 2016, Rafał Miłecki wrote:
> >>>> Not really as it won't cover some pretty common use cases. Many home
> >>>> routers have few USB ports (2-5) and only 1 USB LED. It has to be
> >>>> possible to assign few USB ports to a single LED (trigger). That way
> >>>> LED should be turned on (and kept on) if there is at least 1 USB
> >>>> device connected. You obviously can't do:
> >>>> echo "usb1-1 usb1-2 usb2-1" > /sys/class/leds/foo/trigger
> >>>>
> >>>> This was already brought up by Rob (who mentioned CPU trigger) and I
> >>>> replied him pretty much the same way in:
> >>>> https://lkml.org/lkml/2016/7/29/38
> >>>> (reply starts with "Anyway, the serious limitation I see").
> >>>
> >>> The code for a bunch of triggers must already be written.  What would
> >>> the user do if he wanted to flash a single LED in response to both
> >>> CPU activity and MTD activity?  If not
> >>>
> >>>         echo "cpu mtd" >/sys/class/leds/foo/trigger
> >>>
> >>> then what?
> >>
> >> Well, it sounds like a new feature then. Shall we add an extra API
> >> with a request function for turning LED on? It could internally count
> >> how many requests were raised and keep LED on as long as there is at
> >> least 1 left. I guess we should implement it in trigger "subsystem"
> >> (if I can call it so). Does it sound like a good plan?
> >
> > I'm pretty sure noone ever planned to have more than 1 trigger
> > assigned to a single LED. I just realized there will be a problem with
> > proposed solution: sysfs files conflict.
> >
> > Consider 2 existing triggers for a moment:
> > 1) oneshot: it creates following sysfs files:
> > /sys/class/leds/foo/delay_on
> > /sys/class/leds/foo/delay_off
> > /sys/class/leds/foo/invert
> > /sys/class/leds/foo/shot
> > 2) timer: it creates following sysfs files:
> > /sys/class/leds/foo/delay_on
> > /sys/class/leds/foo/delay_off
> >
> > Activating both of them will probably cause a WARNING in sysfs. They
> > can't coexist :(
> >
> > We should probably have per-trigger subdirs, e.g.:
> > /sys/class/leds/foo/trigger-oneshot/delay_on
> > /sys/class/leds/foo/trigger-oneshot/delay_off
> > /sys/class/leds/foo/trigger-oneshot/invert
> > /sys/class/leds/foo/trigger-oneshot/shot
> > /sys/class/leds/foo/trigger-timer/delay_on
> > /sys/class/leds/foo/trigger-timer/delay_off
> > but implementing it now would break the ABI.
> >
> > One workaround I can see is doing triggers V2, they:
> > 1) Would put sysfs files in /sys/class/leds/foo/trigger-bar/
> > 2) Use new API for *requesting* LED to be on/off
> > 3) There would be a counter of requests in V2 API
> > 4) Multiple triggers V2 would be allowed to be used (assigned) at the same time
> 
> Currently we support only triggers dedicated to specific type of
> devices. Even in case of triggers that don't expose custom sysfs
> attributes, registered with led_trigger_register_simple(), device
> drivers have to generate trigger event with dedicated function, e.g:
> 
> - ledtrig-cpu: void ledtrig_cpu(enum cpu_led_event ledevt)
> - ledtrig-disk: void ledtrig_disk_activity(void)
> - ledtrig-mtd: void ledtrig_mtd_activity(void)
> 
> If one wanted to have the LED notified by different type of devices,
> then they would have to implement a trigger that would exposed all
> required types of API.
> 
> Unfortunately, there are many possible combinations of
> triggers and that doesn't sound sane to add a new one when someone
> will find it useful. Of course it would entail adding a call to the
> new trigger API in the drivers, which doesn't seem like something
> acceptable in the mainline.

Maybe it would make more sense, in this case, to allow only three 
possibilities for a USB port activity trigger.  Toggle the LED 
whenever:

	There is activity on the specified port, or

	There is any activity on any port on the specified hub, or

	There is any USB activity on any port.

That ought to cover most of the normal use cases, and it would be
simple enough to implement.

Alan Stern