From: "Marek Behún" <kabel@kernel.org>
To: Ansuel Smith <ansuelsmth@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Jonathan Corbet <corbet@lwn.net>, Pavel Machek <pavel@ucw.cz>,
John Crispin <john@phrozen.org>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-leds@vger.kernel.org
Subject: Re: [RFC PATCH v4 0/8] Adds support for PHY LEDs with offload triggers
Date: Thu, 11 Nov 2021 03:16:08 +0100 [thread overview]
Message-ID: <20211111031608.11267828@thinkpad> (raw)
In-Reply-To: <20211111013500.13882-1-ansuelsmth@gmail.com>
On Thu, 11 Nov 2021 02:34:52 +0100
Ansuel Smith <ansuelsmth@gmail.com> wrote:
> This is another attempt in adding support for PHY LEDs. Most of the
> times Switch/PHY have connected multiple LEDs that are controlled by HW
> based on some rules/event. Currently we lack any support for a generic
> way to control the HW part and normally we either never implement the
> feature or only add control for brightness or hw blink.
>
> This is based on Marek idea of providing some API to cled but use a
> different implementation that in theory should be more generilized.
>
> The current idea is:
> - LED driver implement 3 API (hw_control_status/start/stop).
> They are used to put the LED in hardware mode and to configure the
> various trigger.
> - We have hardware triggers that are used to expose to userspace the
> supported hardware mode and set the hardware mode on trigger
> activation.
> - We can also have triggers that both support hardware and software mode.
> - The LED driver will declare each supported hardware blink mode and
> communicate with the trigger all the supported blink modes that will
> be available by sysfs.
> - A trigger will use blink_set to configure the blink mode to active
> in hardware mode.
> - On hardware trigger activation, only the hardware mode is enabled but
> the blink modes are not configured. The LED driver should reset any
> link mode active by default.
>
> Each LED driver will have to declare explicit support for the offload
> trigger (or return not supported error code) as we the trigger_data that
> the LED driver will elaborate and understand what is referring to (based
> on the current active trigger).
>
> I posted a user for this new implementation that will benefit from this
> and will add a big feature to it. Currently qca8k can have up to 3 LEDs
> connected to each PHY port and we have some device that have only one of
> them connected and the default configuration won't work for that.
>
> I also posted the netdev trigger expanded with the hardware support.
>
> More polish is required but this is just to understand if I'm taking
> the correct path with this implementation hoping we find a correct
> implementation and we start working on the ""small details""
Hello Ansuel,
besides other things, I am still against the idea of the
`hardware-phy-activity` trigger: I think that if the user wants the LED
to indicate network device's link status or activity, it should always
be done via the existing netdev trigger, and with that trigger only.
Yes, I know that netdev trigger does not currently support indicating
different link modes, only whether the link is up (in any mode). That
should be solved by extending the netdev trigger.
I am going to try to revive my last attempt and send my proposal again.
Hope you don't mind.
Marek
next prev parent reply other threads:[~2021-11-11 2:16 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-11 1:34 [RFC PATCH v4 0/8] Adds support for PHY LEDs with offload triggers Ansuel Smith
2021-11-11 1:34 ` [RFC PATCH v4 1/8] leds: add support for hardware driven LEDs Ansuel Smith
2021-11-11 2:24 ` Randy Dunlap
2021-11-11 1:34 ` [RFC PATCH v4 2/8] leds: document additional use of blink_set for hardware control Ansuel Smith
2021-11-11 2:28 ` Randy Dunlap
2021-11-11 1:34 ` [RFC PATCH v4 3/8] leds: trigger: netdev: drop NETDEV_LED_MODE_LINKUP from mode Ansuel Smith
2021-11-11 1:34 ` [RFC PATCH v4 4/8] leds: trigger: netdev: rename and expose NETDEV trigger enum and struct Ansuel Smith
2021-11-11 1:34 ` [RFC PATCH v4 5/8] leds: trigger: netdev: add hardware control support Ansuel Smith
2021-11-11 1:34 ` [RFC PATCH v4 6/8] leds: trigger: add hardware-phy-activity trigger Ansuel Smith
2021-11-11 2:18 ` Randy Dunlap
2021-11-11 1:34 ` [RFC PATCH v4 7/8] net: dsa: qca8k: add LEDs support Ansuel Smith
2021-11-11 2:19 ` Randy Dunlap
2021-11-11 1:35 ` [RFC PATCH v4 8/8] dt-bindings: net: dsa: qca8k: add LEDs definition example Ansuel Smith
2021-11-11 2:16 ` Marek Behún [this message]
2021-11-12 15:35 ` [RFC PATCH v4 0/8] Adds support for PHY LEDs with offload triggers Ansuel Smith
2021-11-12 18:30 ` 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=20211111031608.11267828@thinkpad \
--to=kabel@kernel.org \
--cc=andrew@lunn.ch \
--cc=ansuelsmth@gmail.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=john@phrozen.org \
--cc=kuba@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pavel@ucw.cz \
--cc=robh+dt@kernel.org \
--cc=vivien.didelot@gmail.com \
/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).