diff mbox series

[v3,net-next] net: phy: realtek: read actual speed on rtl8211f to detect downshift

Message ID 20201124230756.887925-1-antonio.borneo@st.com
State Superseded
Headers show
Series [v3,net-next] net: phy: realtek: read actual speed on rtl8211f to detect downshift | expand

Commit Message

Antonio Borneo Nov. 24, 2020, 11:07 p.m. UTC 
The rtl8211f supports downshift and before commit 5502b218e001
("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
the read-back of register MII_CTRL1000 was used to detect the
negotiated link speed.
The code added in commit d445dff2df60 ("net: phy: realtek: read
actual speed to detect downshift") is working fine also for this
phy and it's trivial re-using it to restore the downshift
detection on rtl8211f.

Add the phy specific read_status() pointing to the existing
function rtlgen_read_status().

Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Link: https://lore.kernel.org/r/478f871a-583d-01f1-9cc5-2eea56d8c2a7@huawei.com
---
To: Andrew Lunn <andrew@lunn.ch>
To: Heiner Kallweit <hkallweit1@gmail.com>
To: Russell King <linux@armlinux.org.uk>
To: "David S. Miller" <davem@davemloft.net>
To: Jakub Kicinski <kuba@kernel.org>
To: netdev@vger.kernel.org
To: Yonglong Liu <liuyonglong@huawei.com>
To: Willy Liu <willy.liu@realtek.com>
Cc: linuxarm@huawei.com
Cc: Salil Mehta <salil.mehta@huawei.com>
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <20201124143848.874894-1-antonio.borneo@st.com>

V1 => V2
	move from a generic implementation affecting every phy
	to a rtl8211f specific implementation
V2 => V3
	rebase on netdev-next, resolving minor conflict after
	merge of 8b43357fff61
---
 drivers/net/phy/realtek.c | 1 +
 1 file changed, 1 insertion(+)


base-commit: 1d155dfdf50efc2b0793bce93c06d1a5b23d0877

Comments

Yonglong Liu Nov. 25, 2020, 3:03 p.m. UTC | #1 
Tested-by: Yonglong Liu <liuyonglong@huawei.com>

On 2020/11/25 7:07, Antonio Borneo wrote:
> The rtl8211f supports downshift and before commit 5502b218e001
> ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
> the read-back of register MII_CTRL1000 was used to detect the
> negotiated link speed.
> The code added in commit d445dff2df60 ("net: phy: realtek: read
> actual speed to detect downshift") is working fine also for this
> phy and it's trivial re-using it to restore the downshift
> detection on rtl8211f.
>
> Add the phy specific read_status() pointing to the existing
> function rtlgen_read_status().
>
> Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
> Link: https://lore.kernel.org/r/478f871a-583d-01f1-9cc5-2eea56d8c2a7@huawei.com
> ---
> To: Andrew Lunn <andrew@lunn.ch>
> To: Heiner Kallweit <hkallweit1@gmail.com>
> To: Russell King <linux@armlinux.org.uk>
> To: "David S. Miller" <davem@davemloft.net>
> To: Jakub Kicinski <kuba@kernel.org>
> To: netdev@vger.kernel.org
> To: Yonglong Liu <liuyonglong@huawei.com>
> To: Willy Liu <willy.liu@realtek.com>
> Cc: linuxarm@huawei.com
> Cc: Salil Mehta <salil.mehta@huawei.com>
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-kernel@vger.kernel.org
> In-Reply-To: <20201124143848.874894-1-antonio.borneo@st.com>
>
> V1 => V2
> 	move from a generic implementation affecting every phy
> 	to a rtl8211f specific implementation
> V2 => V3
> 	rebase on netdev-next, resolving minor conflict after
> 	merge of 8b43357fff61
> ---
>   drivers/net/phy/realtek.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index f71eda945c6a..99ecd6c4c15a 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -729,6 +729,7 @@ static struct phy_driver realtek_drvs[] = {
>   		PHY_ID_MATCH_EXACT(0x001cc916),
>   		.name		= "RTL8211F Gigabit Ethernet",
>   		.config_init	= &rtl8211f_config_init,
> +		.read_status	= rtlgen_read_status,
>   		.config_intr	= &rtl8211f_config_intr,
>   		.handle_interrupt = rtl8211f_handle_interrupt,
>   		.suspend	= genphy_suspend,
>
> base-commit: 1d155dfdf50efc2b0793bce93c06d1a5b23d0877
Yonglong Liu Nov. 25, 2020, 4:57 p.m. UTC | #2 
Hi, Antonio:

     Could you help to provide a downshift warning message when this 
happen?

     It's a little strange that the adv and the lpa support 1000M, but 
finally the link speed is 100M.

Settings for eth5:
         Supported ports: [ TP ]
         Supported link modes:   10baseT/Half 10baseT/Full
                                 100baseT/Half 100baseT/Full
                                 1000baseT/Full
         Supported pause frame use: Symmetric Receive-only
         Supports auto-negotiation: Yes
         Supported FEC modes: Not reported
         *Advertised link modes:  10baseT/Half 10baseT/Full
                                 100baseT/Half 100baseT/Full
                                 1000baseT/Full*
         Advertised pause frame use: Symmetric
         Advertised auto-negotiation: Yes
         Advertised FEC modes: Not reported
         *Link partner advertised link modes:  10baseT/Half 10baseT/Full
                                              100baseT/Half 100baseT/Full
                                              1000baseT/Full*
         Link partner advertised pause frame use: Symmetric
         Link partner advertised auto-negotiation: Yes
         Link partner advertised FEC modes: Not reported
         *Speed: 100Mb/s*
         Duplex: Full
         Port: MII
         PHYAD: 3
         Transceiver: internal
         Auto-negotiation: on
         Current message level: 0x00000036 (54)
                                probe link ifdown ifup
         Link detected: yes


     

On 2020/11/25 23:03, Yonglong Liu wrote:
> Tested-by: Yonglong Liu <liuyonglong@huawei.com>
>
> On 2020/11/25 7:07, Antonio Borneo wrote:
>> The rtl8211f supports downshift and before commit 5502b218e001
>> ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
>> the read-back of register MII_CTRL1000 was used to detect the
>> negotiated link speed.
>> The code added in commit d445dff2df60 ("net: phy: realtek: read
>> actual speed to detect downshift") is working fine also for this
>> phy and it's trivial re-using it to restore the downshift
>> detection on rtl8211f.
>>
>> Add the phy specific read_status() pointing to the existing
>> function rtlgen_read_status().
>>
>> Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
>> Link: 
>> https://lore.kernel.org/r/478f871a-583d-01f1-9cc5-2eea56d8c2a7@huawei.com
>> ---
>> To: Andrew Lunn <andrew@lunn.ch>
>> To: Heiner Kallweit <hkallweit1@gmail.com>
>> To: Russell King <linux@armlinux.org.uk>
>> To: "David S. Miller" <davem@davemloft.net>
>> To: Jakub Kicinski <kuba@kernel.org>
>> To: netdev@vger.kernel.org
>> To: Yonglong Liu <liuyonglong@huawei.com>
>> To: Willy Liu <willy.liu@realtek.com>
>> Cc: linuxarm@huawei.com
>> Cc: Salil Mehta <salil.mehta@huawei.com>
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> Cc: linux-kernel@vger.kernel.org
>> In-Reply-To: <20201124143848.874894-1-antonio.borneo@st.com>
>>
>> V1 => V2
>>     move from a generic implementation affecting every phy
>>     to a rtl8211f specific implementation
>> V2 => V3
>>     rebase on netdev-next, resolving minor conflict after
>>     merge of 8b43357fff61
>> ---
>>   drivers/net/phy/realtek.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
>> index f71eda945c6a..99ecd6c4c15a 100644
>> --- a/drivers/net/phy/realtek.c
>> +++ b/drivers/net/phy/realtek.c
>> @@ -729,6 +729,7 @@ static struct phy_driver realtek_drvs[] = {
>>           PHY_ID_MATCH_EXACT(0x001cc916),
>>           .name        = "RTL8211F Gigabit Ethernet",
>>           .config_init    = &rtl8211f_config_init,
>> +        .read_status    = rtlgen_read_status,
>>           .config_intr    = &rtl8211f_config_intr,
>>           .handle_interrupt = rtl8211f_handle_interrupt,
>>           .suspend    = genphy_suspend,
>>
>> base-commit: 1d155dfdf50efc2b0793bce93c06d1a5b23d0877
>
> _______________________________________________
> Linuxarm mailing list
> Linuxarm@huawei.com
> http://hulk.huawei.com/mailman/listinfo/linuxarm
>
> .
Russell King (Oracle) Nov. 25, 2020, 5:07 p.m. UTC | #3 
On Thu, Nov 26, 2020 at 12:57:37AM +0800, Yonglong Liu wrote:
> Hi, Antonio:
> 
>     Could you help to provide a downshift warning message when this happen?
> 
>     It's a little strange that the adv and the lpa support 1000M, but
> finally the link speed is 100M.

That is an identifying feature of downshift.

Downshift can happen at either end of the link, and since we must not
change the "Advertised link modes" since this is what userspace
configured, if a downshift occurs at the local end, then you will get
the ethtool output you provide, where the speed does not agree with
the reported advertisements.

You should already be getting a warning in the kernel log when this
happens; phy_check_downshift() which is part of the phylib core code
will check this every time the link comes up. You should already
have a message "Downshift occurred ..." in your kernel log. Please
check.
Jakub Kicinski Nov. 25, 2020, 8:30 p.m. UTC | #4 
On Wed, 25 Nov 2020 00:07:56 +0100 Antonio Borneo wrote:
> The rtl8211f supports downshift and before commit 5502b218e001
> ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
> the read-back of register MII_CTRL1000 was used to detect the
> negotiated link speed.
> The code added in commit d445dff2df60 ("net: phy: realtek: read
> actual speed to detect downshift") is working fine also for this
> phy and it's trivial re-using it to restore the downshift
> detection on rtl8211f.
> 
> Add the phy specific read_status() pointing to the existing
> function rtlgen_read_status().
> 
> Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
> Link: https://lore.kernel.org/r/478f871a-583d-01f1-9cc5-2eea56d8c2a7@huawei.com

Applied, thanks everyone!
Yonglong Liu Nov. 26, 2020, 1:15 a.m. UTC | #5 
Hi, Russell:

     I found this message in kernel log, thanks!

On 2020/11/26 1:07, Russell King - ARM Linux admin wrote:
> On Thu, Nov 26, 2020 at 12:57:37AM +0800, Yonglong Liu wrote:
>> Hi, Antonio:
>>
>>      Could you help to provide a downshift warning message when this happen?
>>
>>      It's a little strange that the adv and the lpa support 1000M, but
>> finally the link speed is 100M.
> That is an identifying feature of downshift.
>
> Downshift can happen at either end of the link, and since we must not
> change the "Advertised link modes" since this is what userspace
> configured, if a downshift occurs at the local end, then you will get
> the ethtool output you provide, where the speed does not agree with
> the reported advertisements.
>
> You should already be getting a warning in the kernel log when this
> happens; phy_check_downshift() which is part of the phylib core code
> will check this every time the link comes up. You should already
> have a message "Downshift occurred ..." in your kernel log. Please
> check.
>
diff mbox series

Patch

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index f71eda945c6a..99ecd6c4c15a 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -729,6 +729,7 @@  static struct phy_driver realtek_drvs[] = {
 		PHY_ID_MATCH_EXACT(0x001cc916),
 		.name		= "RTL8211F Gigabit Ethernet",
 		.config_init	= &rtl8211f_config_init,
+		.read_status	= rtlgen_read_status,
 		.config_intr	= &rtl8211f_config_intr,
 		.handle_interrupt = rtl8211f_handle_interrupt,
 		.suspend	= genphy_suspend,