[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260204070955.1473-1-javen_xu@realsil.com.cn>
Date: Wed, 4 Feb 2026 15:09:55 +0800
From: javen <javen_xu@...lsil.com.cn>
To: <hkallweit1@...il.com>
CC: <andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
<horms@...nel.org>, <javen_xu@...lsil.com.cn>, <kuba@...nel.org>,
<linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
<nic_swsd@...ltek.com>, <pabeni@...hat.com>
Subject: Re: [PATCH net-next v3] r8169: add support for RTL8125cp
>On 2/3/2026 8:58 AM, javen wrote:
>>>> On 2/2/2026 11:58 AM, javen wrote:
>>
>>>> From: Javen Xu <javen_xu@...lsil.com.cn>
>>
>>>>
>>
>>>> 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@...lsil.com.cn>
>>
>>>>
>>
>>>> ---
>>
>>>> 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
Powered by blists - more mailing lists