Re: [PATCH net v2 2/2] net: phy: qcom: at803x: Apply IPQ5018 analog settings at probe

[Andrew Lunn]

On Tue, Sep 29, 2026 at 12:07:17AM +0200, Yongzhao Chen wrote:
> ipq5018_probe() pulses the PHY reset, and the IPQ5018 internal GE PHY
> then starts autonegotiation on its own with its reset-default analog
> settings. The LDO, EEE timer, MSE threshold and optional short-cable
> DAC values are only written by ipq5018_config_init(), which runs when
> the MAC attaches the PHY, usually when the interface is opened.
> 
> On a Redmi AX5400 board, the IPQ5018 PHY is connected without a cable
> to PHY4 of a QCA8337 switch, and "qcom,dac-preset-short-cable" is set.
> Between probe and attach, about 39 s in these boots, both PHYs
> resolved 1000BASE-T every 2.5 to 3 s, but the link did not come up.
> After about five attempts, SmartSpeed downshifted on both sides at the
> same time: the IPQ5018 PHY stopped advertising 1000BASE-T (CTRL1000
> 0x0200 -> 0x0000), and so did the QCA8337 PHY (0x0600 -> 0x0400). The
> soft reset at attach restores the IPQ5018 advertisement, but nothing
> restores the QCA8337 side, and the link stayed down, also after
> taking the interface down and up again.
> 
> Apply the analog settings in probe right after the reset and restart
> autonegotiation, so that negotiation runs with them from the start.
> Factor the settings into a helper that is also used by
> ipq5018_config_init(), and return MDIO errors from it instead of
> ignoring them. Probe fails with the error; config_init() returns it.
> 
> genphy_restart_aneg() sets ANENABLE and ANRESTART and clears ISOLATE.
> With the reset-default BMCR value of 0x1140 read on this board, this
> is the same write that was tested (BMCR | BMCR_ANRESTART).
> 
> The same values are written on every board with this PHY; only the
> time of the write changes. On boards without the DAC property, no
> DAC register is written.
> 
> The same writes and the autonegotiation restart were tested in probe
> on that board, in OpenWrt's Linux 6.18.52 kernel, over 3 warm boots
> with and 3 without them. Without them, the link did not come up in
> any boot, with both sides downshifted as described above. With them,
> 1000BASE-T came up in every boot less than 3 s after the QCA8337 PHY
> was reset, before the interface was opened, and the QCA8337 PHY kept
> advertising 1000BASE-T with SmartSpeed enabled. The DAC values written
> in probe were still in place after the BMCR soft reset at attach.
> 
> This patch, backported to the same kernel without the downstream
> SmartSpeed workaround, then kept the link at 1000BASE-T with SmartSpeed
> enabled on that board over a first boot, a power cycle, three reboots,
> three interface down/up cycles, three autonegotiation restarts, a
> network restart and 10 minutes of operation.

The usual problem with AI generated commit messages. They are too
verbose. 

  ipq5018_probe() pulses the PHY reset, and the IPQ5018 internal GE
  PHY then starts autonegotiation on its own with its reset-default
  analog settings. Refactor the config_init the pull the setting of
  the analogue front into a helper, and call it at probe, so the
  updated values are used, not the reset values.

Patch 0/X should contain the big picture, and there is no need to
repeat it in the individual patches. It will get included in the text
of the merge commit.

    Andrew

---
pw-bot: cr