[RFC PATCH 6/7] realtek: Zyxel GS1900-8: define port LEDs

[Olliver Schinagl]

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 at 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 at 8.0 {
>>> +                       reg = <8 0>;
>>> +                       label = "lan1";
>>> +               };
>>> +               led at 9.0 {
>>> +                       reg = <9 0>;
>>> +                       label = "lan2";
>>> +               };
>>> +               led at 10.0 {
>>> +                       reg = <10 0>;
>>> +                       label = "lan3";
>>> +               };
>>> +               led at 11.0 {
>>> +                       reg = <11 0>;
>>> +                       label = "lan4";
>>> +               };
>>> +               led at 12.0 {
>>> +                       reg = <12 0>;
>>> +                       label = "lan5";
>>> +               };
>>> +               led at 13.0 {
>>> +                       reg = <13 0>;
>>> +                       label = "lan6";
>>> +               };
>>> +               led at 14.0 {
>>> +                       reg = <14 0>;
>>> +                       label = "lan7";
>>> +               };
>>> +               led at 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
>