| Message ID | 20230319191814.22067-16-ansuelsmth@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: Add basic LED support for switch/phy | expand |
Hi! > From: Andrew Lunn <andrew@lunn.ch> > > The WAN port of the 370-RD has a Marvell PHY, with one LED on > the front panel. List this LED in the device tree. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > @@ -135,6 +136,19 @@ &mdio { > pinctrl-names = "default"; > phy0: ethernet-phy@0 { > reg = <0>; > + leds { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@0 { > + reg = <0>; > + label = "WAN"; > + color = <LED_COLOR_ID_WHITE>; > + function = LED_FUNCTION_LAN; > + function-enumerator = <1>; > + linux,default-trigger = "netdev"; > + }; > + }; > }; > How will this end up looking in sysfs? Should documentation be added to Documentation/leds/leds-blinkm.rst ? BR, Pavel
On Thu, Mar 23, 2023 at 01:04:53PM +0100, Pavel Machek wrote: > Hi! > > > From: Andrew Lunn <andrew@lunn.ch> > > > > The WAN port of the 370-RD has a Marvell PHY, with one LED on > > the front panel. List this LED in the device tree. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > @@ -135,6 +136,19 @@ &mdio { > > pinctrl-names = "default"; > > phy0: ethernet-phy@0 { > > reg = <0>; > > + leds { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + led@0 { > > + reg = <0>; > > + label = "WAN"; > > + color = <LED_COLOR_ID_WHITE>; > > + function = LED_FUNCTION_LAN; > > + function-enumerator = <1>; > > + linux,default-trigger = "netdev"; > > + }; > > + }; > > }; > > > > How will this end up looking in sysfs? Hi Pavel It is just a plain boring LED, so it will look like all other LEDs. There is nothing special here. > Should documentation be added to Documentation/leds/leds-blinkm.rst > ? This has nothing to do with blinkm, which appears to be an i2c LED driver. Andrew
Hi! > > > The WAN port of the 370-RD has a Marvell PHY, with one LED on > > > the front panel. List this LED in the device tree. > > > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > > > @@ -135,6 +136,19 @@ &mdio { > > > pinctrl-names = "default"; > > > phy0: ethernet-phy@0 { > > > reg = <0>; > > > + leds { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + led@0 { > > > + reg = <0>; > > > + label = "WAN"; > > > + color = <LED_COLOR_ID_WHITE>; > > > + function = LED_FUNCTION_LAN; > > > + function-enumerator = <1>; > > > + linux,default-trigger = "netdev"; > > > + }; > > > + }; > > > }; > > > > > > > How will this end up looking in sysfs? > > Hi Pavel > > It is just a plain boring LED, so it will look like all other LEDs. > There is nothing special here. Well, AFAICT it will end up as /sys/class/leds/WAN, which is really not what we want. (Plus the netdev trigger should be tested; we'll need some kind of link to the ethernet device if we want this to work on multi-ethernet systems). > > Should documentation be added to Documentation/leds/leds-blinkm.rst > > ? > > This has nothing to do with blinkm, which appears to be an i2c LED > driver. Sorry, I meant Should documentation be added to Documentation/leds/well-known-leds.txt ? Best regards, Pavel
> > Hi Pavel > > > > It is just a plain boring LED, so it will look like all other LEDs. > > There is nothing special here. > > Well, AFAICT it will end up as /sys/class/leds/WAN, which is really > not what we want. Why not? It is just a plain boring LED. It can be used for anything, heartbeat, panic SOS in Morse code, shift lock, disk activity. Any of the triggers can be applied to it. It can be found in /sys/class/leds/f1072004.mdio-mii:00:WAN. But when we come to using it for ledtrig-netdev, the user is more likely to follow /sys/class/net/eth0/phydev/leds/f1072004.mdio-mii\:00\:WAN/ > (Plus the netdev trigger should be tested; we'll > need some kind of link to the ethernet device if we want this to work > on multi-ethernet systems). Since this is a plain boring LED, it could actually blink for any netdev. When we get to offloading blinking to hardware, then things change, we need to check the netdev which is configured in the ledtrig-netdev is the same one the PHY is associated to. But i have a patchset for that which will appear later. > Should documentation be added to Documentation/leds/well-known-leds.txt ? Saying what. That there might be LEDs in your RJ45 connector, which can be used for anything which is supported by an Linux LED trigger? Andrew
diff --git a/arch/arm/boot/dts/armada-370-rd.dts b/arch/arm/boot/dts/armada-370-rd.dts index be005c9f42ef..15b36aa34ef4 100644 --- a/arch/arm/boot/dts/armada-370-rd.dts +++ b/arch/arm/boot/dts/armada-370-rd.dts @@ -20,6 +20,7 @@ /dts-v1/; #include <dt-bindings/input/input.h> #include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/leds/common.h> #include <dt-bindings/gpio/gpio.h> #include "armada-370.dtsi" @@ -135,6 +136,19 @@ &mdio { pinctrl-names = "default"; phy0: ethernet-phy@0 { reg = <0>; + leds { + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0>; + label = "WAN"; + color = <LED_COLOR_ID_WHITE>; + function = LED_FUNCTION_LAN; + function-enumerator = <1>; + linux,default-trigger = "netdev"; + }; + }; }; switch: switch@10 {