Re: [PATCH RFC leds + net-next v2 0/1] Add support for LEDs on Marvell PHYs

Pavel Machek
2020-07-24T10:29:07Z

[-- Attachment #1: Type: text/plain, Size: 2541 bytes --]

On Thu 2020-07-23 20:13:18, Marek Behún wrote:
> Hi,
>
> this is v2 of my RFC adding support for LEDs connected to Marvell PHYs.
>
> The LED subsystem patches are not contained:
> - the patch adding support for LED private triggers is already accepted
> in Pavel Machek's for-next tree.
> If you want to try this patch on top of net-next, please also apply
> https://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/commit/?h=for-next&id=93690cdf3060c61dfce813121d0bfc055e7fa30d
> - the other led-trigger patch is not needed in this version of the RFC
>
> The main difference from v1 is that only one trigger, named
> "hw-control", is added to the /sys/class/leds/<LED>/trigger file.
>
> When this trigger is activated, another file called "hw_control" is
> created in the /sys/class/leds/<LED>/ directory. This file lists
> available HW control modes for this LED in the same way the trigger
> file does for triggers.
>
> Example:
>
> # cd /sys/class/leds/eth0\:green\:link/
> # cat trigger
> [none] hw-control timer oneshot heartbeat ...
> # echo hw-control >trigger

Make this "hw-phy-mode" or something. hw-control is a bit too generic.

> # cat trigger
> none [hw-control] timer oneshot heartbeat ...
> # cat hw_control
> link/nolink link/act/nolink 1000/100/10/nolink act/noact
> blink-act/noact transmit/notransmit copperlink/else [1000/else]
> force-hi-z force-blink
> # echo 1000/100/10/nolink >hw_control
> # cat hw_control
> link/nolink link/act/nolink [1000/100/10/nolink] act/noact
> blink-act/noact transmit/notransmit copperlink/else 1000/else
> force-hi-z force-blink
>
> The benefit here is that only one trigger is registered via LED API.
> I guess there are other PHY drivers which too support HW controlled
> blinking modes. So of this way of controlling PHY LED HW controlled
> modes is accepted, the code creating the hw-control trigger and
> hw_control file should be made into library code so that it can be
> reused.
>
> What do you think?

So.. you have 10 of them right now. I guess both hw_control and making
it into the trigger directly is acceptable here.

In future, would you expect having software "1000/100/10/nolink"
triggers I could activate on my scrollock LED (or on GPIO controlled
LEDs) to indicate network activity?

Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]