diff mbox series

[net-next,v5,15/15] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

Message ID 20230319191814.22067-16-ansuelsmth@gmail.com
State New
Headers
Series net: Add basic LED support for switch/phy |

Commit Message

Christian Marangi March 19, 2023, 7:18 p.m. UTC 
  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>
---
 arch/arm/boot/dts/armada-370-rd.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Pavel Machek March 23, 2023, 12:04 p.m. UTC | #1 
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
Andrew Lunn March 23, 2023, 5:02 p.m. UTC | #2 
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
Pavel Machek March 23, 2023, 7:11 p.m. UTC | #3 
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
Andrew Lunn March 23, 2023, 7:53 p.m. UTC | #4 
> > 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 mbox series

Patch

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 {