| Message ID | 328d5a61298abee3aac61eaa6000c84723345bed.1657998467.git.sander@svanheule.net |
|---|---|
| State | Superseded |
| Delegated to: | Sander Vanheule |
| Headers | show |
| Series | realtek: MFD for switch core | expand |
On 16-07-2022 21:09, Sander Vanheule wrote: > Add all port LEDs to the device tree for the GS1900-8. To reproduce the > same behaviour as stock firmware, the LEDs need to light up on all > link speeds, and blink on link activity: > > echo 1f > /sys/class/leds/lan?/rtl_hw_trigger > echo realtek-switchport > /sys/class/leds/lan?/trigger > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > --- > .../dts-5.10/rtl8380_zyxel_gs1900-8.dts | 41 +++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts > index e9c5efe60392..41266b701aca 100644 > --- a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts > +++ b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts > @@ -10,3 +10,44 @@ > &gpio1 { > /delete-node/ poe_enable; > }; > + > +&switchcore { > + port-leds { > + compatible = "realtek,rtl8380-port-led"; > + #address-cells = <2>; > + #size-cells = <0>; > + > + led@8.0 { > + reg = <8 0>; > + label = "lan1"; > + }; > + led@9.0 { > + reg = <9 0>; > + label = "lan2"; > + }; > + led@10.0 { > + reg = <10 0>; > + label = "lan3"; > + }; > + led@11.0 { > + reg = <11 0>; > + label = "lan4"; > + }; > + led@12.0 { > + reg = <12 0>; > + label = "lan5"; > + }; > + led@13.0 { > + reg = <13 0>; > + label = "lan6"; > + }; > + led@14.0 { > + reg = <14 0>; > + label = "lan7"; > + }; > + led@15.0 { > + reg = <15 0>; > + label = "lan8"; > + }; > + }; > +}; nothing else to say then 'love it' :) I'm not familiar with the GS1900; but it only has a single LED with one color? Can we set the trigger already here in the dt like we do with others? arch/arm/boot/dts/am335x-bone-common.dtsi:31: linux,default-trigger = "heartbeat"; for example ... >
Hi Oliver, Thanks for all the feedback, I'll go over it tonight. In the future, could you CC me (as the patch author), other any reviewers, and people who were CC-ed before? Not everybody may be subscribed to the mailing list (not even necessarily the patch author). On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote: > On 16-07-2022 21:09, Sander Vanheule wrote: > > Add all port LEDs to the device tree for the GS1900-8. To reproduce the > > same behaviour as stock firmware, the LEDs need to light up on all > > link speeds, and blink on link activity: > > > > echo 1f > /sys/class/leds/lan?/rtl_hw_trigger > > echo realtek-switchport > /sys/class/leds/lan?/trigger > > > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > > --- > > .../dts-5.10/rtl8380_zyxel_gs1900-8.dts | 41 +++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts > > b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts > > index e9c5efe60392..41266b701aca 100644 > > --- a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts > > +++ b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts > > @@ -10,3 +10,44 @@ > > &gpio1 { > > /delete-node/ poe_enable; > > }; > > + > > +&switchcore { > > + port-leds { > > + compatible = "realtek,rtl8380-port-led"; > > + #address-cells = <2>; > > + #size-cells = <0>; > > + > > + led@8.0 { > > + reg = <8 0>; > > + label = "lan1"; > > + }; > > + led@9.0 { > > + reg = <9 0>; > > + label = "lan2"; > > + }; > > + led@10.0 { > > + reg = <10 0>; > > + label = "lan3"; > > + }; > > + led@11.0 { > > + reg = <11 0>; > > + label = "lan4"; > > + }; > > + led@12.0 { > > + reg = <12 0>; > > + label = "lan5"; > > + }; > > + led@13.0 { > > + reg = <13 0>; > > + label = "lan6"; > > + }; > > + led@14.0 { > > + reg = <14 0>; > > + label = "lan7"; > > + }; > > + led@15.0 { > > + reg = <15 0>; > > + label = "lan8"; > > + }; > > + }; > > +}; > > nothing else to say then 'love it' :) I'm not familiar with the GS1900; > but it only has a single LED with one color? That's correct. Each port on this device has one green LED to indicate status. I was lazy and didn't add the 'color', 'function', 'function-enumerator' properties... > Can we set the trigger > already here in the dt like we do with others? > > arch/arm/boot/dts/am335x-bone-common.dtsi:31: linux,default-trigger = > "heartbeat"; > > for example ... You actually can. The trigger that reproduces stock behaviour is "realtek-switchport". But! Before you enable that trigger, you need to write an appropriate trigger mask to the custom config property in /sys/class/leds/$PORT_LED. Otherwise the trigger value will be "disabled". Setting that value from DT is not possible though. So I figured that the recommended way to configure this HW trigger would be to run a script at system init. Best, Sander
On 29-07-2022 15:27, Sander Vanheule wrote: > Hi Oliver, > > Thanks for all the feedback, I'll go over it tonight. In the future, could you CC me (as the patch > author), other any reviewers, and people who were CC-ed before? Not everybody may be subscribed to > the mailing list (not even necessarily the patch author). absolutly; I'm not subsribed either; so I downloaded the mbox file from patchwork; and did a 'reply-list'; guess thunderbird simply doesn't know about other reviewers based on the mbox file; which is obvious of course ... > > On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote: >> On 16-07-2022 21:09, Sander Vanheule wrote: >>> Add all port LEDs to the device tree for the GS1900-8. To reproduce the >>> same behaviour as stock firmware, the LEDs need to light up on all >>> link speeds, and blink on link activity: >>> >>> echo 1f > /sys/class/leds/lan?/rtl_hw_trigger >>> echo realtek-switchport > /sys/class/leds/lan?/trigger >>> >>> Signed-off-by: Sander Vanheule <sander@svanheule.net> >>> --- >>> .../dts-5.10/rtl8380_zyxel_gs1900-8.dts | 41 +++++++++++++++++++ >>> 1 file changed, 41 insertions(+) >>> >>> diff --git a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts >>> b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts >>> index e9c5efe60392..41266b701aca 100644 >>> --- a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts >>> +++ b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts >>> @@ -10,3 +10,44 @@ >>> &gpio1 { >>> /delete-node/ poe_enable; >>> }; >>> + >>> +&switchcore { >>> + port-leds { >>> + compatible = "realtek,rtl8380-port-led"; >>> + #address-cells = <2>; >>> + #size-cells = <0>; >>> + >>> + led@8.0 { >>> + reg = <8 0>; >>> + label = "lan1"; >>> + }; >>> + led@9.0 { >>> + reg = <9 0>; >>> + label = "lan2"; >>> + }; >>> + led@10.0 { >>> + reg = <10 0>; >>> + label = "lan3"; >>> + }; >>> + led@11.0 { >>> + reg = <11 0>; >>> + label = "lan4"; >>> + }; >>> + led@12.0 { >>> + reg = <12 0>; >>> + label = "lan5"; >>> + }; >>> + led@13.0 { >>> + reg = <13 0>; >>> + label = "lan6"; >>> + }; >>> + led@14.0 { >>> + reg = <14 0>; >>> + label = "lan7"; >>> + }; >>> + led@15.0 { >>> + reg = <15 0>; >>> + label = "lan8"; >>> + }; >>> + }; >>> +}; >> nothing else to say then 'love it' :) I'm not familiar with the GS1900; >> but it only has a single LED with one color? > That's correct. Each port on this device has one green LED to indicate status. I was lazy and didn't > add the 'color', 'function', 'function-enumerator' properties... heh, part of our job description right :) I'd still add them to make it more clear, also doesn't that help when they show up in sysfs? > >> Can we set the trigger >> already here in the dt like we do with others? >> >> arch/arm/boot/dts/am335x-bone-common.dtsi:31: linux,default-trigger = >> "heartbeat"; >> >> for example ... > You actually can. The trigger that reproduces stock behaviour is "realtek-switchport". But! Before > you enable that trigger, you need to write an appropriate trigger mask to the custom config property > in /sys/class/leds/$PORT_LED. Otherwise the trigger value will be "disabled". Setting that value > from DT is not possible though. So I figured that the recommended way to configure this HW trigger > would be to run a script at system init. the net-effect would still be the same right? no led would show up until the startup script launches, so having the `realtek-switchport` trigger makes it more descriptive? I think it's an oversight though of the dts parser, as if you define the trigger 'blink' you should be able to set up a delay_on/off from the dts as well. but that's a separate discussion :) > > Best, > Sander >
diff --git a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts index e9c5efe60392..41266b701aca 100644 --- a/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts +++ b/target/linux/realtek/dts-5.10/rtl8380_zyxel_gs1900-8.dts @@ -10,3 +10,44 @@ &gpio1 { /delete-node/ poe_enable; }; + +&switchcore { + port-leds { + compatible = "realtek,rtl8380-port-led"; + #address-cells = <2>; + #size-cells = <0>; + + led@8.0 { + reg = <8 0>; + label = "lan1"; + }; + led@9.0 { + reg = <9 0>; + label = "lan2"; + }; + led@10.0 { + reg = <10 0>; + label = "lan3"; + }; + led@11.0 { + reg = <11 0>; + label = "lan4"; + }; + led@12.0 { + reg = <12 0>; + label = "lan5"; + }; + led@13.0 { + reg = <13 0>; + label = "lan6"; + }; + led@14.0 { + reg = <14 0>; + label = "lan7"; + }; + led@15.0 { + reg = <15 0>; + label = "lan8"; + }; + }; +};
Add all port LEDs to the device tree for the GS1900-8. To reproduce the same behaviour as stock firmware, the LEDs need to light up on all link speeds, and blink on link activity: echo 1f > /sys/class/leds/lan?/rtl_hw_trigger echo realtek-switchport > /sys/class/leds/lan?/trigger Signed-off-by: Sander Vanheule <sander@svanheule.net> --- .../dts-5.10/rtl8380_zyxel_gs1900-8.dts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+)