[PATCH net v2] r8152: Use BMSR to detect the link state

[PATCH net v2] r8152: Use BMSR to detect the link state

[Linmao Li]

r8152 detects carrier from PLA_PHYSTATUS without reading BMSR, so
BMSR_LSTATUS can still be latched low when the carrier comes up.
Since commit f6f2e946aa4d ("net: mii: Fix the Speed display when the
network cable is not connected"), the first speed query after link up
can then report SPEED_UNKNOWN, leaving NetworkManager at 0 Mb/s until
the next carrier change.

Use BMSR_LSTATUS in set_carrier() and rtl8152_runtime_resume(), so the
driver consumes the latched link down itself. If the first read still
reports link down, the next link-up notification triggers another read
and brings the carrier up.

Tested on an RTL8153B with a 6.6-based kernel. In 5 rebinds and 6 cable
replugs, the first read returned LSTATUS=0, a second link-up
notification came about 32 ms later, the second read returned
LSTATUS=1 and the carrier went up; NetworkManager reported 1000 Mb/s.
Runtime suspend/resume with the link up did not change the carrier.

Fixes: f6f2e946aa4d ("net: mii: Fix the Speed display when the network cable is not connected")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
v2:
- Fix it in r8152 instead of mii.c, detecting the link from BMSR as
  suggested by Andrew Lunn.
- Also use BMSR in rtl8152_runtime_resume().
v1: https://lore.kernel.org/netdev/20260930113842.2640928-1-lilinmao@kylinos.cn/

Only tested on an RTL8153B, with a 6.6-based kernel; the changed lines
are the same in net/main. I could not check whether BMSR_LSTATUS is
reliable on the 2.5G/5G/10G chips handled by this driver.
I could not reproduce a link drop that recovers while the device is
runtime suspended, so that case is only covered by code inspection.

r8152_mdio_read() cannot fail today. The pending RTL8157/RTL8159
series makes it return errors, so these two callers will need error
handling once that is merged.

 drivers/net/usb/r8152.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f61686433031..e3947eb796c5 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -6979,11 +6979,8 @@ static void set_carrier(struct r8152 *tp)
 {
 	struct net_device *netdev = tp->netdev;
 	struct napi_struct *napi = &tp->napi;
-	u16 speed;
-
-	speed = rtl8152_get_speed(tp);
 
-	if (speed & LINK_STATUS) {
+	if (r8152_mdio_read(tp, MII_BMSR) & BMSR_LSTATUS) {
 		if (!netif_carrier_ok(netdev)) {
 			tp->rtl_ops.enable(tp);
 			netif_stop_queue(netdev);
@@ -8653,7 +8650,7 @@ static int rtl8152_runtime_resume(struct r8152 *tp)
 		set_bit(WORK_ENABLE, &tp->flags);
 
 		if (netif_carrier_ok(netdev)) {
-			if (rtl8152_get_speed(tp) & LINK_STATUS) {
+			if (r8152_mdio_read(tp, MII_BMSR) & BMSR_LSTATUS) {
 				rtl_start_rx(tp);
 			} else {
 				netif_carrier_off(netdev);
-- 
2.25.1

Re: [PATCH net v2] r8152: Use BMSR to detect the link state

[Birger Koblitz]

Hi Linmao,

On 05/10/2026 12:52 pm, Linmao Li wrote:
> r8152 detects carrier from PLA_PHYSTATUS without reading BMSR, so
> BMSR_LSTATUS can still be latched low when the carrier comes up.
> Since commit f6f2e946aa4d ("net: mii: Fix the Speed display when the
> network cable is not connected"), the first speed query after link up
> can then report SPEED_UNKNOWN, leaving NetworkManager at 0 Mb/s until
> the next carrier change.
> 
> Use BMSR_LSTATUS in set_carrier() and rtl8152_runtime_resume(), so the
> driver consumes the latched link down itself. If the first read still
> reports link down, the next link-up notification triggers another read
> and brings the carrier up.
> 
> Tested on an RTL8153B with a 6.6-based kernel. In 5 rebinds and 6 cable
> replugs, the first read returned LSTATUS=0, a second link-up
> notification came about 32 ms later, the second read returned
> LSTATUS=1 and the carrier went up; NetworkManager reported 1000 Mb/s.
> Runtime suspend/resume with the link up did not change the carrier.

I have been trying to reproduce the issue with the following adapter:
driver: r8152
version: 7.1.8+deb13-amd64
firmware-version: rtl8153a-4 v2 02/07/20

But I am not able to do so. Both after a link-up after a plugin event 
and after a system resume, ethtool always reports that the correct link 
speed and state. Could you explain, what you actually do, exactly? Or is 
the wrong link information only there for 32ms and I am just too slow?

Birger

RE: [PATCH net v2] r8152: Use BMSR to detect the link state

[Hayes Wang]

Linmao Li <lilinmao@kylinos.cn>
> Sent: Monday, October 5, 2026 6:53 PM
[...]
> r8152 detects carrier from PLA_PHYSTATUS without reading BMSR, so
> BMSR_LSTATUS can still be latched low when the carrier comes up.
> Since commit f6f2e946aa4d ("net: mii: Fix the Speed display when the network
> cable is not connected"), the first speed query after link up can then report
> SPEED_UNKNOWN, leaving NetworkManager at 0 Mb/s until the next carrier
> change.
> 
> Use BMSR_LSTATUS in set_carrier() and rtl8152_runtime_resume(), so the
> driver consumes the latched link down itself. If the first read still reports link
> down, the next link-up notification triggers another read and brings the carrier
> up.
> 
> Tested on an RTL8153B with a 6.6-based kernel. In 5 rebinds and 6 cable
> replugs, the first read returned LSTATUS=0, a second link-up notification came
> about 32 ms later, the second read returned
> LSTATUS=1 and the carrier went up; NetworkManager reported 1000 Mb/s.
> Runtime suspend/resume with the link up did not change the carrier.

I think this patch may introduce a new issue.

Our newer ICs do not generate periodic link-status notifications. They generate a
notification only when the link status changes. Therefore, with your patch, BMSR
will not be read a second time until the next link-status change.

Best Regards,
Hayes

> Fixes: f6f2e946aa4d ("net: mii: Fix the Speed display when the network cable is
> not connected")
> Cc: stable@vger.kernel.org
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
> v2:
> - Fix it in r8152 instead of mii.c, detecting the link from BMSR as
>   suggested by Andrew Lunn.
> - Also use BMSR in rtl8152_runtime_resume().
> v1:
> https://lore.kernel.org/netdev/20260930113842.2640928-1-lilinmao@kylinos.c
> n/
> 
> Only tested on an RTL8153B, with a 6.6-based kernel; the changed lines are the
> same in net/main. I could not check whether BMSR_LSTATUS is reliable on the
> 2.5G/5G/10G chips handled by this driver.
> I could not reproduce a link drop that recovers while the device is runtime
> suspended, so that case is only covered by code inspection.
> 
> r8152_mdio_read() cannot fail today. The pending RTL8157/RTL8159 series
> makes it return errors, so these two callers will need error handling once that
> is merged.
> 
>  drivers/net/usb/r8152.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index
> f61686433031..e3947eb796c5 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -6979,11 +6979,8 @@ static void set_carrier(struct r8152 *tp)  {
>         struct net_device *netdev = tp->netdev;
>         struct napi_struct *napi = &tp->napi;
> -       u16 speed;
> -
> -       speed = rtl8152_get_speed(tp);
> 
> -       if (speed & LINK_STATUS) {
> +       if (r8152_mdio_read(tp, MII_BMSR) & BMSR_LSTATUS) {
>                 if (!netif_carrier_ok(netdev)) {
>                         tp->rtl_ops.enable(tp);
>                         netif_stop_queue(netdev); @@ -8653,7 +8650,7
> @@ static int rtl8152_runtime_resume(struct r8152 *tp)
>                 set_bit(WORK_ENABLE, &tp->flags);
> 
>                 if (netif_carrier_ok(netdev)) {
> -                       if (rtl8152_get_speed(tp) & LINK_STATUS) {
> +                       if (r8152_mdio_read(tp, MII_BMSR) &
> + BMSR_LSTATUS) {
>                                 rtl_start_rx(tp);
>                         } else {
>                                 netif_carrier_off(netdev);
> --
> 2.25.1