linux-kernel - Re: [net-next PATCH v6 16/16] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

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: <ebaedccc-a73b-48fc-8735-c0567d899d5c@lunn.ch>
Date:   Thu, 6 Apr 2023 15:54:14 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Pavel Machek <pavel@....cz>
Cc:     Christian Marangi <ansuelsmth@...il.com>,
        Lee Jones <lee@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        John Crispin <john@...ozen.org>, linux-leds@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [net-next PATCH v6 16/16] arm: mvebu: dt: Add PHY LED support
 for 370-rd WAN port

> I don't think basing stuff on position is reasonable. (And am not sure
> if making difference between MAC and PHY leds is good idea).
> 
> Normally, there's ethernet port with two LEDs, one is usually green
> and indicates link, second being yellow and indicates activity,
> correct?

Nope. I have machines with 1, 2 or 3 LEDs. I have green, yellow, white
and red LEDs.

Part of the problem is 802.3 says absolutely nothing about LEDs. So
every vendor is free to do whatever why want. There is no
standardisation at all. So we have to assume every vendor does
something different.

> On devices like ADSL modems, there is one LED per port, typically on
> with link and blinking with activity.  
> 
> Could we use that distinction instead? (id):green:link,
> (id):yellow:activity, (id):?:linkact -- for combined LED as it seems.
> 
> Are there any other common leds? I seem to remember "100mbps" lights
> from time where 100mbit was fast...?

But what about 2.5G, 5G, 10G, 40G... And 10Mbps for automotive. And
collision for 1/2 duplex, which is making a bit of a comeback in
automotive.

Plus, we are using ledtrig-netdev. A wifi device is a netdev. A CAN
bus devices is a netdev. Link speed has a totally different meaning
for 802.11 and CAN.

You are also assuming the LEDs have fixed meaning. But they are not
fixed, they mean whatever the ledtrig-netdev is configured to make
them blink.  I even have one of my boxes blinking heartbeat, because
if has a habit of crashing... And i think for Linux LEDs in general,
we should not really tie an LED to a meaning. Maybe tie it to a label
on the case, but the meaning of an LED is all about software, what
ledtrig- is controlling it.

As to differentiating MAC and PHY, we need to, because as i said, both
could offer LEDs. Generally, Ethernet switches have LED controllers
per MAC port. Most switches have internal PHYs, and those PHYs don't
have LED controllers. However, not all ports have internal PHYs, there
can be external PHYs with its own LED controller. So in that case,
both the MAC and the PHY could register an LED controller for the same
netdev. It comes down to DT to indicate what LED controllers are
actually wired to an LED.

	 Andrew