From: "Marek Behún" <marek.behun@nic.cz>
To: "Ondřej Jirman" <megous@megous.com>
Cc: linux-kernel@vger.kernel.org,
Jacek Anaszewski <jacek.anaszewski@gmail.com>,
Pavel Machek <pavel@ucw.cz>, Dan Murphy <dmurphy@ti.com>,
"open list:LED SUBSYSTEM" <linux-leds@vger.kernel.org>
Subject: Re: [PATCH RFC] leds: Add support for per-LED device triggers
Date: Wed, 8 Jul 2020 16:55:33 +0200 [thread overview]
Message-ID: <20200708165533.12f1949f@dellmb> (raw)
In-Reply-To: <20200703130809.h4mvhwbdiymuo5pn@core.my.home>
Hi Ondrej,
I overlooked your reply in my inbox, sorry this took so long.
On Fri, 3 Jul 2020 15:08:09 +0200
Ondřej Jirman <megous@megous.com> wrote:
> Do you have such a switch? Also what's your real usecase?
Yes, on Turris MOX three 8-port ethernet switches can be plugged,
resulting in 24 ethernet ports, each having 2 LEDs.
The current driver does not expose these LEDs via sysfs, but I want to
add the support there. Each of these LEDs can be controlled by
software, or can be put into one of these HW controlled modes:
- Link (with three submodes: 10/100, Gb, 10Gb)
- Link activity (again with three submodes as above)
- PTP activity
- Force blink
> My usecase is a PMIC which has either a user-controllable or
> self-working mode for a single LED it controls. I want to be able to
> switch between those quickly and easily.
I understand your usecase completely. This is the same thing I want. I
just have reservations about how you want to implement this.
Marek
> I want the LED to be mostly controlled by PMIC, because that way PMIC
> can signal events that are not exposed to OS like overvoltage,
> overheating, etc. ... all automagically, but also be able to control
> it sometimes via SW (for non PMIC related notifications, eg.).
>
> So in my mindset LED is either controlled by Linux via various SW
> triggers, or it's self-working in some arbitrary device specific
> configuration that doesn't need any passing of the data via CPU for
> the LED to reflect some HW state.
>
> So I'd expose a 'hw-trigger' only on the LED device that allows this,
> that you can select among all the other regular triggers as you do
> now, and then configure its precise mode/options in sysfs (on the
> trigger kobj). The driver would come with some sane device specific
> defaults for the self-working mode.
>
> User can then select hw-trigger, in the triggers and would get a nice
> PMIC LED behavior controlled by HW, or a common LED behavior of the
> ehternet port, or on the wireless card, or whatever.
>
> From the perspective of this use case the interface is nice and
> generic:
>
> - you set LED to hw-trigger mode on boot
> - you set trigger to none and poke the LED with a pattern you want
> for the notification and put it back to hw-trigger mode again
> afterwards
>
> We can standardize on hw-trigger, or self-controlled, or some other
> name for this kind of private LED trigger, and then the userspace
> would not need to even care about the specific LED device type, when
> sitching between SW controlled and self-working modes.
>
> You'd be able to take SW control of the ethernet PHY controlled LEDs
> this way just the same as the PMIC LED with the same interface,
> described above. And if you don't like the default self-controled
> mode, you can change it via sysfs attributes on the trigger.
>
> It would also allow the user to switch between SW and HW control,
> without having to remember the previous HW triggering mode, because
> that could be persisted by the LED HW trigger device. So you can go
> back to previous HW triggering mode just by 'echo hw-trigger >
> your-led/trigger'.
>
> I've read through the discussions, and this seems like a workable
> interface.
>
> Most of the LED devices would just add one extra private trigger to
> the triggers_list, so it would not explode in the way you describe
> above.
>
> Also benefit of this approach is that it fits quite nicely with the
> existing code, and requires minimal changes. The trigger already
> allows for specifying sysfs attributes, too.
>
> regards,
> o.
next prev parent reply other threads:[~2020-07-08 14:55 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-02 14:47 [PATCH RFC] leds: Add support for per-LED device triggers Ondrej Jirman
2020-07-02 14:51 ` Ondřej Jirman
2020-07-03 10:06 ` Marek Behún
2020-07-03 13:08 ` Ondřej Jirman
2020-07-08 14:55 ` Marek Behún [this message]
2020-07-04 12:59 ` Pavel Machek
2020-07-08 14:56 ` Marek Behún
2020-07-04 12:04 ` Pavel Machek
2020-07-04 12:17 ` Ondřej Jirman
2020-07-04 20:07 ` Pavel Machek
2020-07-11 10:04 ` Pavel Machek
2020-07-11 21:01 ` Ondřej Jirman
2020-07-12 7:25 ` Pavel Machek
2020-07-12 13:49 ` Ondřej Jirman
2020-07-12 19:11 ` Pavel Machek
2020-07-12 21:10 ` Marek Behun
2020-07-12 22:12 ` Ondřej Jirman
2020-07-12 22:38 ` Ondřej Jirman
2020-07-12 23:15 ` Marek Behun
2020-07-12 23:18 ` Marek Behun
2020-07-13 7:12 ` Pavel Machek
2020-07-12 23:20 ` Ondřej Jirman
2020-07-13 1:56 ` Ondřej Jirman
2020-07-15 17:07 ` Marek Behún
2020-07-15 17:55 ` Marek Behún
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=20200708165533.12f1949f@dellmb \
--to=marek.behun@nic.cz \
--cc=dmurphy@ti.com \
--cc=jacek.anaszewski@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=megous@megous.com \
--cc=pavel@ucw.cz \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).