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

[Linmao Li <lilinmao@kylinos.cn>]

在 2026/10/7 0:10, Andrew Lunn 写道:
> On Tue, Oct 06, 2026 at 08:28:21AM +0000, Hayes Wang wrote:
>> 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.
> But the link status in BMSR does change.
>
> You read it once and get the latched value. That clears the latch, so
> the link status changes to the current version.
>
> Now, 802.3 C22 has no support for interrupts, that is a vendor
> extension. But if you are not generating an interrupt when BMSR
> changes, i would say that is broken.
>
> 	 Andrew
Hi Hayes, Andrew,

Thanks. Since the newer ICs notify only when the link status changes,
the driver cannot rely on another link up notification after the link
has settled, so v2 can leave the carrier off when the first read
returns the latched link down.

Before sending v3, which approach would you prefer? Both fixed the
problem in my tests on an RTL8153B with v7.3-rc6:

a) Keep carrier detection on PLA_PHYSTATUS and read BMSR once before
    carrier on, only to clear the latch. This is the option I asked
    about on v1.

b) Detect carrier from BMSR and read it again when the first read
    returns link down, as genphy_update_link() does in interrupt mode.

Neither depends on another notification. Neither reports a short link
drop that recovers before the link work runs: with a) a later speed
query still sees the latched link down, with b) the latch is cleared.

Hayes, for b): is BMSR_LSTATUS reliable as the carrier source on all
chips supported by r8152?

Thanks,
Linmao

pw-bot: cr