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

[Hayes Wang]

Linmao Li <lilinmao@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Linmao Li <lilinmao@xxxxxxxxxx>
> ---
> 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