Re: [PATCH net-next] net: phy: qca83xx: read resolved QCA8337 link status

[Yongzhao Chen <yongzhao.derek@gmail.com>]

Hi Andrew,

> One thing you can try is to set the link partner to only advertise 1G,
> not the lower speeds. I've no idea what it will do then, but you might
> get into the situation you are interested in.

Thanks. I tried that with an Intel igb link partner and a two-pair
cable. With only 1000baseT/Full advertised, the QCA8337 set its
downshift bit at times, but the link did not come up during the
observation window. With the partner's default advertisement, though,
the QCA8337 side did downshift: it was not advertising 1000BASE-T
(CTRL1000 0x0400) while the partner was (STAT1000 0x2800), and register
0x11 showed the downshift bit set and a resolved 100 Mb/s full-duplex
link.

For the subsequent old/new comparison, I kept the partner's default
advertisement and used one OpenWrt Linux 6.18.52 test image. A selector
switched only the tested user PHY between genphy_read_status() and
at803x_read_status() followed by genphy_read_master_slave(), with the
same tracing in both paths.

In both paths, phydev->advertising and lp_advertising still had
1000baseT/Full set. With genphy_read_status(), the driver reported
1000 Mb/s, phylink passed that to the MAC, and the MAC port status
register read back 1000 Mb/s. With the new path, the driver reported
100 Mb/s, phylink passed 100 Mb/s, and the MAC register agreed.
Outside in-band mode, qca8k_phylink_mac_link_up() in net programs the
port speed the same way.

Each path was tested across two warm boots and three port down/up
cycles, and the downshift was present in every stage. Ping was 0/20
in each direction in every old-path stage and 20/20 in every new-path
stage.

This covers the resolved downshift case only. It does not show whether
BMSR can report link up while register 0x11 is still unresolved, and
it does not change the SPEED_UNKNOWN limitation from my previous reply.

Given the traffic failure, I would like to target net for v2. Three
questions:

1. Is net appropriate, or should this stay in net-next?
2. Is 272833b9b3b3 ("net: phy: add support for qca8k switch internal
   PHY in at803x") the right Fixes target? It added the QCA8337 entry
   without .read_status; I have not checked whether the problem
   predates it. Christian, as its author, is now on Cc.
3. Compared with genphy_read_status(), the helper takes the forced-mode
   speed from register 0x11 rather than BMCR and adds MDI-X reporting,
   and the added genphy_read_master_slave() call runs on every poll. Is
   that scope acceptable for net, or would you prefer a narrower fix?

Thanks,
Yongzhao Chen