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

[Huan Zhao <ms.huan.zhao@gmail.com>]

On Sun, Mar 15, 2026 at 07:06:58PM +0100, Heiner Kallweit wrote:
>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.
>
Confirmed. After unloading ledtrig_netdev and reloading it after r8169
was already running, assigning trigger=netdev to the LED classdev
immediately activated hw_control (offloaded=1) and all link_* files
appeared. Load order does not matter.

This means /etc/modules-load.d/ledtrig-netdev.conf is not needed.

>>
>> 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.
>
This is exactly what we observed in testing. When the script runs from
a udev RUN rule (ACTION=="move" on network interface rename), writing
trigger=netdev succeeds but the link_* files are not yet writable
immediately after. They become writable after a variable delay of up
to several seconds.

Our current workaround in the script is to poll link_100 until it
becomes writable:

   echo netdev > "$led/trigger"
   while ! echo 0 > "$led/link_100" 2>/dev/null; do
       sleep 0.1
   done

When the script runs as a systemd oneshot service after network.target,
this polling loop completes in zero iterations — link_* files are
immediately writable at that point.

So the udev timing issue is likely caused by trigger assignment being
asynchronous, as you suspected.

>>      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
>>>
>