Re: r8169: RTL8125B restores default LEDSEL values on Link Up, preventing persistent LED configuration

[Heiner Kallweit <hkallweit1@gmail.com>]

On 15.03.2026 18:41, Huan Zhao wrote:
> You are correct. I apologize for the incorrect analysis in my previous
> email. Further testing has clarified the actual behavior.
> 
> Summary of findings after additional testing:
> 
> 1. The LEDSEL registers do NOT reset on Link Up.
> 
>    After writing 0x0000 to all LEDSEL registers via BAR0 MMIO and
>    triggering multiple Link Up/Down cycles, the registers retained
>    their zero values throughout. The hardware does not restore any
>    defaults on link state changes.
> 
> 2. The real problem is CONFIG_LEDS_TRIGGER_NETDEV=m on Arch Linux.
> 
>    As you clarified, the LED classdev registers successfully without
>    ledtrig_netdev present — just without a trigger assigned. However,
>    the link_* sysfs files (link_100, link_2500, etc.) do not appear
>    until the netdev trigger is explicitly assigned. Without these
>    files, there is no way to configure the LED modes through the
>    standard interface.
> 
>    The workaround is to ensure ledtrig_netdev loads before r8169
>    via /etc/modules-load.d/.

The netdev trigger module can be loaded also after r8169, and then
be assigned as trigger.

> 
> 3. Persistent LED disable works correctly via the standard interface.
> 
>    With ledtrig_netdev loaded before r8169, the following sequence
>    successfully and persistently disables all LEDs:
> 
>      echo netdev > /sys/class/leds/enp2s0-N::lan/trigger

How is it if you add a small delay here? I'd have check whether
assigning a trigger is fully synchronous.

>      echo 0 > /sys/class/leds/enp2s0-N::lan/link_10
>      echo 0 > /sys/class/leds/enp2s0-N::lan/link_100
>      echo 0 > /sys/class/leds/enp2s0-N::lan/link_1000
>      echo 0 > /sys/class/leds/enp2s0-N::lan/link_2500
>      echo 0 > /sys/class/leds/enp2s0-N::lan/tx
>      echo 0 > /sys/class/leds/enp2s0-N::lan/rx
> 
>    This survives Link Up/Down cycles and network cable
>    insertion/removal without any further intervention.
> 
>    This is implemented as a systemd oneshot service running after
>    network.target, at which point all link_* files are immediately
>    writable with no waiting required.
> 
> 4. Remaining question: udev timing issue.
> 
>    When the same script runs from a udev RUN rule (triggered on
>    ACTION=="move" for the network interface rename), the link_*
>    files are not yet writable at that point, even after
>    trigger=netdev is set. The files only become writable after
>    a delay of up to several seconds. Is there a reliable udev
>    event or sysfs condition that indicates hw_control is fully
>    ready for writing?
> 
> Thanks,
> Huan Zhao
> 
> On Sun, Mar 15, 2026 at 02:55:17PM +0100, Heiner Kallweit wrote:
>> On 15.03.2026 02:46, Huan Zhao wrote:
>>> Hardware: Beelink SER8 mini PC
>>> NIC: Realtek RTL8125B (XID 0x641, RTL_GIGA_MAC_VER_63, rev 05)
>>> Kernel: 6.19.6-arch1-1
>>> Driver: r8169 (in-tree)
>>> CONFIG_R8169_LEDS=y
>>> CONFIG_LEDS_TRIGGER_NETDEV=m
>>>
>>> Problem:
>>> The RTL8125B chip restores its LEDSEL registers to hardware default values
>>> on every Link Up event. The r8169 driver does not reapply the LED
>>> configuration after Link Up, making it impossible to persistently configure
>>> LEDs (e.g. disable them) through the standard kernel LED subsystem.
>>>
>>> Details:
>>>
>>> The Link Up path in the driver is:
>>>
>>>   r8169_phylink_handler()       [r8169_main.c:4757]
>>>     -> rtl_link_chg_patch()     [r8169_main.c:1597]
>>>     -> rtl_enable_tx_lpi()
>>>     -> pm_request_resume()
>>>
>>> rtl_link_chg_patch() has no handling for RTL_GIGA_MAC_VER_63 and does not
>>> touch the LEDSEL registers. Similarly, rtl_hw_start_8125b() and
>>> rtl_hw_start_8125_common(), which are called on hardware initialization,
>>> do not write to any LEDSEL register (LEDSEL0=0x18, LEDSEL1=0x86,
>>> LEDSEL2=0x84, LEDSEL3=0x96).
>>>
>>> The only driver code that writes LEDSEL registers is rtl8125_set_led_mode()
>>> in r8169_leds.c, called exclusively from the LED classdev hw_control
>>> callbacks. Therefore, any LED configuration applied via sysfs is silently
>>> overwritten by the hardware chip's own firmware on every Link Up event,
>>> before the driver has a chance to reapply it.
>>>
>>> This was verified by observing that:
>>> 1. Writing 0x0000 to all LEDSEL registers (via BAR0 MMIO) before Link Up
>>>    successfully turns off all LEDs.
>>> 2. After Link Up completes, the LEDSEL registers revert to their hardware
>>>    defaults, turning the LEDs back on.
>>> 3. Writing 0x0000 to LEDSEL registers immediately after Link Up (via
>>>    NetworkManager dispatcher triggered on dhcp4-change) successfully keeps
>>>    the LEDs off persistently.
>>>
>>> Original LEDSEL register values (hardware defaults) on this machine:
>>>   LEDSEL0 (0x18) = 0x0002  (LINK_100)
>>>   LEDSEL1 (0x86) = 0x0028  (LINK_2500 + LINK_1000)
>>>   LEDSEL2 (0x84) = 0x022b  (ACT + LINK_2500 + LINK_1000 + LINK_100 + LINK_10)
>>>   LEDSEL3 (0x96) = 0x0020  (LINK_2500)
>>>
>>> Secondary issue: CONFIG_LEDS_TRIGGER_NETDEV=m causes silent failure
>>>
>>> On Arch Linux, CONFIG_LEDS_TRIGGER_NETDEV=m (module, not built-in).
>>> The r8169 driver uses /* ignore errors */ when calling
>>> led_classdev_register() in rtl8125_setup_led_ldev(). If ledtrig_netdev
>>> is not loaded when r8169 initializes, the registration fails silently.
>>
>> No, it doesn't fail. Just that no trigger is assigned by default.
>> Once you load ledtrig-netdev module and assign netdev trigger to
>> the LED, you can control LED hw modes.
>>
>>
>>> The result is that /sys/class/leds/enp2s0-* entries appear to exist but
>>> are dummy stubs with no hardware control capability. Writing to their
>>> trigger or brightness attributes has no effect on the physical LEDs.
>>>
>> Setting brightness manually isn't supported by HW.
>>
>>> This was verified by manually loading ledtrig_netdev before r8169, which
>>> resulted in functional LED classdev entries. The fix was to add
>>> ledtrig_netdev to /etc/modules-load.d/ to ensure it loads before r8169.
>>>
>>> However, even with ledtrig_netdev loaded correctly and offloaded=1
>>> confirmed, the LED configuration is still lost on every Link Up due to
>>> the hardware reset described above.
>>>
>>> Suggested fix:
>>>
>>> In r8169_phylink_handler(), after Link Up is confirmed, reapply the
>>> current LED configuration by notifying the LED classdevs. For example:
>>>
>>>   if (netif_carrier_ok(ndev)) {
>>>       rtl_link_chg_patch(tp);
>>>       rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
>>>       /* Reapply LED configuration after hardware resets LEDSEL on Link Up */
>>>       if (tp->leds)
>>>           led_classdev_notify_clients(tp->leds);  /* or equivalent */
>>>       pm_request_resume(d);
>>>   }
>>>
>>> The exact mechanism for reapplying the LED configuration (whether through
>>> the LED classdev framework or by directly re-calling rtl8125_set_led_mode()
>>> with saved values) is left to your discretion.
>>>
>>> Thanks,
>>> Huan Zhao
>>