Re: [PATCH net-next v3] r8169: add support for RTL8125cp

[javen]

>On 2/3/2026 8:58 AM, javen wrote:
>>>> On 2/2/2026 11:58 AM, javen wrote:
>>
>>>> From: Javen Xu <javen_xu@xxxxxxxxxxxxxx>
>>
>>>>
>>
>>>> This patch adds support for chip RTL8125cp. Its XID is 0x708. We apply
>>
>>>> different and firmware for RTL8125cp.
>>
>>>>
>>
>>>> Signed-off-by: Javen Xu <javen_xu@xxxxxxxxxxxxxx>
>>
>>>>
>>
>>>> ---
>>
>>>> v2: This patch fix one mistake on phy_modify_paged(phydev, 0xa43,
>>
>>>> 0x10, 0x0000, 0x1001) which is phy_modify_paged(phydev, 0xa43, 0x00, 0x0000, 0x1001) on patch v1.
>>
>>>>
>>
>>>> v3: Set phy_modify_paged(phydev, 0xa43, 0x10, 0x0000, 0x0003), bit 0
>>
>>>> means 'link speed 10m PLL OFF', bit 1 means 'ALDPS PLL OFF', bit 2
>>
>>>> means 'ENABLE ALDPS', bit 12 means 'ALDPS XTAL OFF'.
>>
>>>
>>
>>> ALDPS is disabled two lines later in rtl8125cp_hw_phy_config().
>>
>>> So why bother with ALDPS settings if ALDPS is disabled anyway?
>>
>>
>>
>> Bit 2 means 'ENABLE ALDPS', we prefer to treat it as a master switch that controls
>>
>> some sub-features like bit 1(ALDPS PLL OFF) and bit 12(ALDPS XTAL OFF), instead of
>>
>> controlling these sub-features individually. And bit 0(link speed 10m PLL OFF)
>>
>> has noting to do with aldps.
>>
>I see, you changed the value of a43/10 to 0x0003 in v3. Then just this question:
>Few lines later ALDPS gets disabled, so is it beneficial to set bit 1 here?
>Thanks for mentioning that bit 0 isn't related to ALDPS.
>
Bit 2 acts like a lock that controls bit 1. If bit 2 = 0, bit 1 has no effect even if set,
so pre-setting bit 1 causes no side effects. The reason why we set bit 1 here is that If we 
need to enable bit1(aldps pll off) later, just set bit 2 to enable aldps without any additional
bits write(means we do not need to set bit 1 again). 
Alternatively, you can view this as initialization related to aldps. if aldps(bit 2) is enabled later,
these related functions will be enabled at the same time(like bit 1).

Thanks,
Javen Xu