diff mbox series

[RFC,7/7] realtek: Netgear GS110TPPv1: define port LEDs

Message ID ec556a32cb1e7cf09516738f27fa2967baa6a940.1657998467.git.sander@svanheule.net
State Superseded
Delegated to: Sander Vanheule
Headers show
Series realtek: MFD for switch core | expand

Commit Message

Sander Vanheule July 16, 2022, 7:09 p.m. UTC
Add the port LEDs for lan1-lan8 to the device tree for the GS110TPP v1.
To reproduce the same behaviour as stock firmware, green should be
LINK/ACT 1G, and amber should be LINK/ACT 100M/10M:

    for i in $(seq 1 8); do
        echo 13 > /sys/class/leds/green:lan-$i/rtl_hw_trigger
        echo realtek-switchport > /sys/class/leds/green:lan-$i/trigger
        echo f > /sys/class/leds/amber:lan-$i/rtl_hw_trigger
        echo realtek-switchport > /sys/class/leds/amber:lan-$i/trigger
    done

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 .../dts-5.10/rtl8380_netgear_gs110tpp-v1.dts  | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

Olliver Schinagl July 29, 2022, 12:58 p.m. UTC | #1
On 16-07-2022 21:09, Sander Vanheule wrote:
> Add the port LEDs for lan1-lan8 to the device tree for the GS110TPP v1.
> To reproduce the same behaviour as stock firmware, green should be
> LINK/ACT 1G, and amber should be LINK/ACT 100M/10M:
>
>      for i in $(seq 1 8); do
>          echo 13 > /sys/class/leds/green:lan-$i/rtl_hw_trigger
>          echo realtek-switchport > /sys/class/leds/green:lan-$i/trigger
>          echo f > /sys/class/leds/amber:lan-$i/rtl_hw_trigger
>          echo realtek-switchport > /sys/class/leds/amber:lan-$i/trigger
>      done
>
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---
>   .../dts-5.10/rtl8380_netgear_gs110tpp-v1.dts  | 30 +++++++++++++++++++
>   1 file changed, 30 insertions(+)
>
> diff --git a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
> index 1ff209cee363..897699bea2c3 100644
> --- a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
> +++ b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
> @@ -43,3 +43,33 @@
>   &uart1 {
>   	status = "okay";
>   };
> +
> +#define LAN_LED_LABEL(p, n)	STRINGIZE(p ## n)
> +#define LED_LABEL_GREEN(p)	LAN_LED_LABEL(green:lan-, p)
> +#define LED_LABEL_AMBER(p)	LAN_LED_LABEL(amber:lan-, p)
> +#define NETGEAR_LED(_phy, _port)			\
> +	led@ ## _phy ##.0 {				\
> +		reg = < _phy 0 >;			\
> +		label = LED_LABEL_GREEN(_port) ;	\
> +	};						\
> +	led@ ## _phy ## .1 {				\
> +		reg = < _phy 1 >;			\
> +		label = LED_LABEL_AMBER(_port) ;	\
> +	}
> +
> +&switchcore {
> +	port-leds {
> +		compatible = "realtek,rtl8380-port-led";
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		NETGEAR_LED(8,1);
> +		NETGEAR_LED(9,2);
> +		NETGEAR_LED(10,3);
> +		NETGEAR_LED(11,4);
> +		NETGEAR_LED(12,5);
> +		NETGEAR_LED(13,6);
> +		NETGEAR_LED(14,7);
> +		NETGEAR_LED(15,8);
> +	};
> +};
ugh, is it relaly worth it to do this? can we not just write it out; yes 
it's a lot of writing (you can use the define to generate it and copy 
paste it ;p)
Sander Vanheule July 29, 2022, 1:31 p.m. UTC | #2
On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote:
> On 16-07-2022 21:09, Sander Vanheule wrote:
> > Add the port LEDs for lan1-lan8 to the device tree for the GS110TPP v1.
> > To reproduce the same behaviour as stock firmware, green should be
> > LINK/ACT 1G, and amber should be LINK/ACT 100M/10M:
> > 
> >      for i in $(seq 1 8); do
> >          echo 13 > /sys/class/leds/green:lan-$i/rtl_hw_trigger
> >          echo realtek-switchport > /sys/class/leds/green:lan-$i/trigger
> >          echo f > /sys/class/leds/amber:lan-$i/rtl_hw_trigger
> >          echo realtek-switchport > /sys/class/leds/amber:lan-$i/trigger
> >      done
> > 
> > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> > ---
> >   .../dts-5.10/rtl8380_netgear_gs110tpp-v1.dts  | 30 +++++++++++++++++++
> >   1 file changed, 30 insertions(+)
> > 
> > diff --git a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
> > b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
> > index 1ff209cee363..897699bea2c3 100644
> > --- a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
> > +++ b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
> > @@ -43,3 +43,33 @@
> >   &uart1 {
> >         status = "okay";
> >   };
> > +
> > +#define LAN_LED_LABEL(p, n)    STRINGIZE(p ## n)
> > +#define LED_LABEL_GREEN(p)     LAN_LED_LABEL(green:lan-, p)
> > +#define LED_LABEL_AMBER(p)     LAN_LED_LABEL(amber:lan-, p)
> > +#define NETGEAR_LED(_phy, _port)                       \
> > +       led@ ## _phy ##.0 {                             \
> > +               reg = < _phy 0 >;                       \
> > +               label = LED_LABEL_GREEN(_port) ;        \
> > +       };                                              \
> > +       led@ ## _phy ## .1 {                            \
> > +               reg = < _phy 1 >;                       \
> > +               label = LED_LABEL_AMBER(_port) ;        \
> > +       }
> > +
> > +&switchcore {
> > +       port-leds {
> > +               compatible = "realtek,rtl8380-port-led";
> > +               #address-cells = <2>;
> > +               #size-cells = <0>;
> > +
> > +               NETGEAR_LED(8,1);
> > +               NETGEAR_LED(9,2);
> > +               NETGEAR_LED(10,3);
> > +               NETGEAR_LED(11,4);
> > +               NETGEAR_LED(12,5);
> > +               NETGEAR_LED(13,6);
> > +               NETGEAR_LED(14,7);
> > +               NETGEAR_LED(15,8);
> > +       };
> > +};
> ugh, is it relaly worth it to do this? can we not just write it out; yes 
> it's a lot of writing (you can use the define to generate it and copy 
> paste it ;p)

I agree it's a bit opaque, and it doesn't allow you to add a label to the second port LED.  But I
was writing this by hand and didn't want to do endless copy-pasting when modifying the LED list. 
The list also misses two LEDs, for device ports 10 and 11, so now I'm doubting if I actually tested
this...

Best,
Sander
Olliver Schinagl July 29, 2022, 1:38 p.m. UTC | #3
On 29-07-2022 15:31, Sander Vanheule wrote:
> On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote:
>> On 16-07-2022 21:09, Sander Vanheule wrote:
>>> Add the port LEDs for lan1-lan8 to the device tree for the GS110TPP v1.
>>> To reproduce the same behaviour as stock firmware, green should be
>>> LINK/ACT 1G, and amber should be LINK/ACT 100M/10M:
>>>
>>>       for i in $(seq 1 8); do
>>>           echo 13 > /sys/class/leds/green:lan-$i/rtl_hw_trigger
>>>           echo realtek-switchport > /sys/class/leds/green:lan-$i/trigger
>>>           echo f > /sys/class/leds/amber:lan-$i/rtl_hw_trigger
>>>           echo realtek-switchport > /sys/class/leds/amber:lan-$i/trigger
>>>       done
>>>
>>> Signed-off-by: Sander Vanheule <sander@svanheule.net>
>>> ---
>>>    .../dts-5.10/rtl8380_netgear_gs110tpp-v1.dts  | 30 +++++++++++++++++++
>>>    1 file changed, 30 insertions(+)
>>>
>>> diff --git a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
>>> b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
>>> index 1ff209cee363..897699bea2c3 100644
>>> --- a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
>>> +++ b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
>>> @@ -43,3 +43,33 @@
>>>    &uart1 {
>>>          status = "okay";
>>>    };
>>> +
>>> +#define LAN_LED_LABEL(p, n)    STRINGIZE(p ## n)
>>> +#define LED_LABEL_GREEN(p)     LAN_LED_LABEL(green:lan-, p)
>>> +#define LED_LABEL_AMBER(p)     LAN_LED_LABEL(amber:lan-, p)
>>> +#define NETGEAR_LED(_phy, _port)                       \
>>> +       led@ ## _phy ##.0 {                             \
>>> +               reg = < _phy 0 >;                       \
>>> +               label = LED_LABEL_GREEN(_port) ;        \
>>> +       };                                              \
>>> +       led@ ## _phy ## .1 {                            \
>>> +               reg = < _phy 1 >;                       \
>>> +               label = LED_LABEL_AMBER(_port) ;        \
>>> +       }
>>> +
>>> +&switchcore {
>>> +       port-leds {
>>> +               compatible = "realtek,rtl8380-port-led";
>>> +               #address-cells = <2>;
>>> +               #size-cells = <0>;
>>> +
>>> +               NETGEAR_LED(8,1);
>>> +               NETGEAR_LED(9,2);
>>> +               NETGEAR_LED(10,3);
>>> +               NETGEAR_LED(11,4);
>>> +               NETGEAR_LED(12,5);
>>> +               NETGEAR_LED(13,6);
>>> +               NETGEAR_LED(14,7);
>>> +               NETGEAR_LED(15,8);
>>> +       };
>>> +};
>> ugh, is it relaly worth it to do this? can we not just write it out; yes
>> it's a lot of writing (you can use the define to generate it and copy
>> paste it ;p)
> I agree it's a bit opaque, and it doesn't allow you to add a label to the second port LED.  But I
> was writing this by hand and didn't want to do endless copy-pasting when modifying the LED list.
> The list also misses two LEDs, for device ports 10 and 11, so now I'm doubting if I actually tested
> this...
Sadly, I can't test this until I had 93xx support to your patch series; 
which I'll try to do as time allows, as I really want this for my switch :)
>
> Best,
> Sander
diff mbox series

Patch

diff --git a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
index 1ff209cee363..897699bea2c3 100644
--- a/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
+++ b/target/linux/realtek/dts-5.10/rtl8380_netgear_gs110tpp-v1.dts
@@ -43,3 +43,33 @@ 
 &uart1 {
 	status = "okay";
 };
+
+#define LAN_LED_LABEL(p, n)	STRINGIZE(p ## n)
+#define LED_LABEL_GREEN(p)	LAN_LED_LABEL(green:lan-, p)
+#define LED_LABEL_AMBER(p)	LAN_LED_LABEL(amber:lan-, p)
+#define NETGEAR_LED(_phy, _port)			\
+	led@ ## _phy ##.0 {				\
+		reg = < _phy 0 >;			\
+		label = LED_LABEL_GREEN(_port) ;	\
+	};						\
+	led@ ## _phy ## .1 {				\
+		reg = < _phy 1 >;			\
+		label = LED_LABEL_AMBER(_port) ;	\
+	}
+
+&switchcore {
+	port-leds {
+		compatible = "realtek,rtl8380-port-led";
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		NETGEAR_LED(8,1);
+		NETGEAR_LED(9,2);
+		NETGEAR_LED(10,3);
+		NETGEAR_LED(11,4);
+		NETGEAR_LED(12,5);
+		NETGEAR_LED(13,6);
+		NETGEAR_LED(14,7);
+		NETGEAR_LED(15,8);
+	};
+};