linux-kernel - Re: [PATCH v7 11/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

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: <f854183545a6ff55235c9f2264af97c1a7f530c3.camel@svanheule.net>
Date:   Mon, 30 Jan 2023 11:59:42 +0100
From:   Sander Vanheule <sander@...nheule.net>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Christian Marangi <ansuelsmth@...il.com>,
        Rob Herring <robh@...nel.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>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Jonathan Corbet <corbet@....net>, Pavel Machek <pavel@....cz>,
        "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
        John Crispin <john@...ozen.org>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-leds@...r.kernel.org,
        Tim Harvey <tharvey@...eworks.com>,
        Alexander Stein <alexander.stein@...tq-group.com>,
        Rasmus Villemoes <rasmus.villemoes@...vas.dk>
Subject: Re: [PATCH v7 11/11] dt-bindings: net: dsa: qca8k: add LEDs
 definition example

Hi Andrew,

On Sun, 2023-01-29 at 23:02 +0100, Andrew Lunn wrote:
> > > This is an example of the dt implemented on a real device.
> > > 
> > >                 mdio {
> > >                         #address-cells = <1>;
> > >                         #size-cells = <0>;
> > > 
> > >                         phy_port1: phy@0 {
> > >                                 reg = <0>;
> > > 
> > >                                 leds {
> > >                                         #address-cells = <1>;
> > >                                         #size-cells = <0>;
> > [...]
> > >                                 };
> > >                         };
> > [...]
> > >                 };
> > > 
> > > In the following implementation. Each port have 2 leds attached (out of
> > > 3) one white and one amber. The driver parse the reg and calculate the
> > > offset to set the correct option with the regs by also checking the phy
> > > number.
> > 
> > With switch silicon allowing user control of the LEDs, vendors can (and
> > will)
> > use the switch's LED peripheral to drive other LEDs (or worse). E.g. on a
> > Cisco
> > SG220-26 switch, using a Realtek RTL8382 SoC, the LEDs associated with some
> > unused switch ports are used to display a global device status. My concern
> > here
> > is that one would have to specify switch ports, that aren't connected to
> > anything, just to describe those non-ethernet LEDs.
> 
> Note that the binding is adding properties to the PHY nodes, not the
> switch port nodes. Is this how the RTL8382 works? Marvell Switches
> have LED registers which are not in the PHY register space.

Thanks for the quick clarification. Because you mention this, I realised that
the RTL8382's LED controller is actually not in the PHYs. These SoCs use
external PHYs, which may have their own, independent, LED controllers. For
example the RTL8212D [1].

[1]
https://datasheet.lcsc.com/lcsc/2203252253_Realtek-Semicon-RTL8218D-CG_C2901898.pdf

> 
> But the point is, the PHYs will probe if listed. They don't have to
> have a MAC pointing to them with a phandle. So the phydev will exist,
> and that should be enough to get the LED class device registered. If
> there is basic on/off support, that should be enough for you to attach
> the Morse code panic trigger, the heartbeat handler, or any other LED
> trigger.

OK, this makes sense for (external) PHYs which need to be probed anyway to have
access to the LEDs.

Looking at the RTL8212D's datasheet (Table 11, p. 24), it appears to be possible
to assign an LED to any of the eight PHYs. Perhaps to allow more freedom in the
board layout. Maybe I'm just not seeing it, but I don't think the example with
an 'leds' node under a PHY contains enough information to perform such a non-
trivial mapping. On the other hand, I'm not sure where else that info might go.
Maybe a 'trigger-sources' property cross-referencing another PHY in the same
package?

Best,
Sander