[PATCH net v2 0/2] net: phy: qcom: at803x: IPQ5018 analog initialization fixes

[PATCH net v2 0/2] net: phy: qcom: at803x: IPQ5018 analog initialization fixes

[Yongzhao Chen]

When the IPQ5018 internal GE PHY is connected to another PHY without a
cable, its analog setup matters for the 1000BASE-T link. Two problems
in that setup are fixed here.

Patch 1 fixes the short-cable DAC values, which were written to the
wrong bits. It is the patch sent as v1 [1], unchanged except for
Andrew's Reviewed-by.

Patch 2 applies the analog settings in probe, right after the PHY reset,
instead of only when the MAC attaches the PHY. The PHY starts
autonegotiating when it leaves reset, so until attach it negotiated
with its reset defaults. On a Redmi AX5400, where it connects to PHY4
of a QCA8337, 1000BASE-T never came up in that state and SmartSpeed on
the switch PHY dropped its 1000BASE-T advertisement for good. With
patch 2 the link came up at 1 Gb/s with SmartSpeed left enabled, and
the SmartSpeed workaround discussed in [2] is no longer needed.

On that board, with both patches backported to OpenWrt's Linux 6.18.52,
the link was verified at 1 Gb/s after a first boot, three reboots, a
power-off cold boot, interface down/up cycles, renegotiations and a
network restart. During a separate 10-minute observation, sampled link
status remained at 1 Gb/s and no new switch-side CPU PHY link-down
events were logged. After the cold boot the switch side first reported
1 Gb/s at 4.4 s; it went down at MAC attach and recovered at 25.2 s.

Patch 2 accesses the PHY right after reset_control_reset(), which
pulses GCC_GEPHY_MISC_ARES for about 1 us. The vendor SDK waits 200 ms
after deasserting each Ethernet reset, but it does so for every block
alike, so that does not establish a GE PHY-specific minimum delay.
This patch adds no post-reset delay. Diagnostic warm-boot tests on
this board read back the values correctly after writing them in probe.
I have no specification for the required post-reset interval. George,
does the GE PHY require a minimum delay or a readiness check after
ARES is deasserted, before its analog settings are written and
autonegotiation is restarted?

Thanks to Ziyang Huang for asking whether the DAC settings had been
corrected [3], which is how the first problem was found, and to Andrew
Lunn for his reviews in the v3 thread, which kept the investigation
going until the cause was found.

Changes since v1:
- Added patch 2.
- Patch 1: added Andrew's Reviewed-by and Ziyang's Suggested-by.

[1] https://lore.kernel.org/netdev/20260927155136.2489-1-yongzhao.derek@gmail.com/
[2] https://lore.kernel.org/netdev/20260923215858.1653-1-yongzhao.derek@gmail.com/
[3] https://lore.kernel.org/netdev/SEYPR01MB58827E0D18ACC93AF98A4109C98E2@SEYPR01MB5882.apcprd01.prod.exchangelabs.com/

Yongzhao Chen (2):
  net: phy: qcom: at803x: Fix IPQ5018 short-cable DAC values
  net: phy: qcom: at803x: Apply IPQ5018 analog settings at probe

 drivers/net/phy/qcom/at803x.c | 94 ++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 24 deletions(-)


base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.43.0

[PATCH net v2 1/2] net: phy: qcom: at803x: Fix IPQ5018 short-cable DAC values

[Yongzhao Chen]

When "qcom,dac-preset-short-cable" is set, ipq5018_config_init()
programs the MDAC (MMD1 0x8100) and EDAC (debug 0x4380) fields. Both
fields occupy bits 15:8 (IPQ5018_PHY_DAC_MASK), but the value 0x10 is
passed unshifted as the set argument of phy_modify_mmd() and
at803x_debug_reg_mask(). Neither helper shifts or masks that argument,
so both fields are cleared to 0x00 instead of being set to 0x10, and
bit 4 of the low byte, which is outside the field, is set.

Use FIELD_PREP() to place the value in the field. This matches the
vendor SDK, which clears bits 15:8 and ORs in the value shifted left
by 8.

On a Redmi AX5400 board, where the IPQ5018 internal PHY connects to a
QCA8337 switch PHY without a cable, MDAC and EDAC read 0x6868 and
0x7800 before the write. With this change they read back 0x1068 and
0x1000, with the low byte preserved. Without it, the same writes would
leave 0x0078 and 0x0010.

No in-tree DTS sets this property yet, but it is documented in
qca,ar803x.yaml and used by several IPQ5018 boards in OpenWrt.

Fixes: d46502279a11 ("net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support")
Suggested-by: Ziyang Huang <hzyitc@outlook.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
Assisted-by: LLM
---
 drivers/net/phy/qcom/at803x.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
index 6872dbf7785..cacbadf1f48 100644
--- a/drivers/net/phy/qcom/at803x.c
+++ b/drivers/net/phy/qcom/at803x.c
@@ -1051,11 +1051,15 @@ static int ipq5018_config_init(struct phy_device *phydev)
 	if (priv->set_short_cable_dac) {
 		/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
 		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
-			       IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_MMD1_MDAC_VAL);
+			       IPQ5018_PHY_DAC_MASK,
+			       FIELD_PREP(IPQ5018_PHY_DAC_MASK,
+					  IPQ5018_PHY_MMD1_MDAC_VAL));
 
 		/* setting EDAC (Error-detection and Correction) in debug register */
 		at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
-				      IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_DEBUG_EDAC_VAL);
+				      IPQ5018_PHY_DAC_MASK,
+				      FIELD_PREP(IPQ5018_PHY_DAC_MASK,
+						 IPQ5018_PHY_DEBUG_EDAC_VAL));
 	}
 
 	return 0;

base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.43.0

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

[Yongzhao Chen]

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.

Fixes: d46502279a11 ("net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support")
Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
Assisted-by: LLM
---
 drivers/net/phy/qcom/at803x.c | 98 +++++++++++++++++++++++++----------
 1 file changed, 70 insertions(+), 28 deletions(-)

diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
index cacbadf1f48..09105f1e373 100644
--- a/drivers/net/phy/qcom/at803x.c
+++ b/drivers/net/phy/qcom/at803x.c
@@ -1019,10 +1019,10 @@ static int ipq5018_cable_test_start(struct phy_device *phydev)
 	return 0;
 }
 
-static int ipq5018_config_init(struct phy_device *phydev)
+static int ipq5018_analog_init(struct phy_device *phydev)
 {
 	struct ipq5018_priv *priv = phydev->priv;
-	u16 val;
+	int val, ret;
 
 	/*
 	 * set LDO efuse: first temporarily store ANA_DAC_FILTER value from
@@ -1030,39 +1030,66 @@ static int ipq5018_config_init(struct phy_device *phydev)
 	 * is written to
 	 */
 	val = at803x_debug_reg_read(phydev, IPQ5018_PHY_DEBUG_ANA_DAC_FILTER);
-	at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE,
-			      IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_MASK,
-			      IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_DEFAULT);
-	at803x_debug_reg_write(phydev, IPQ5018_PHY_DEBUG_ANA_DAC_FILTER, val);
+	if (val < 0)
+		return val;
+
+	ret = at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE,
+				    IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_MASK,
+				    IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_DEFAULT);
+	if (ret)
+		return ret;
+
+	ret = at803x_debug_reg_write(phydev, IPQ5018_PHY_DEBUG_ANA_DAC_FILTER,
+				     val);
+	if (ret)
+		return ret;
 
 	/* set 8023AZ EEE TX and RX timer values */
-	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_EEE_TX_TIMER,
-		      IPQ5018_PHY_PCS_EEE_TX_TIMER_VAL);
-	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_EEE_RX_TIMER,
-		      IPQ5018_PHY_PCS_EEE_RX_TIMER_VAL);
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_EEE_TX_TIMER,
+			    IPQ5018_PHY_PCS_EEE_TX_TIMER_VAL);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_EEE_RX_TIMER,
+			    IPQ5018_PHY_PCS_EEE_RX_TIMER_VAL);
+	if (ret)
+		return ret;
 
 	/* set MSE threshold values */
-	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MSE_THRESH1,
-		      IPQ5018_PHY_MMD1_MSE_THRESH1_VAL);
-	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MSE_THRESH2,
-		      IPQ5018_PHY_MMD1_MSE_THRESH2_VAL);
+	ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
+			    IPQ5018_PHY_MMD1_MSE_THRESH1,
+			    IPQ5018_PHY_MMD1_MSE_THRESH1_VAL);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
+			    IPQ5018_PHY_MMD1_MSE_THRESH2,
+			    IPQ5018_PHY_MMD1_MSE_THRESH2_VAL);
+	if (ret)
+		return ret;
 
 	/* PHY DAC values are optional and only set in a PHY to PHY link architecture */
-	if (priv->set_short_cable_dac) {
-		/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
-		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
-			       IPQ5018_PHY_DAC_MASK,
-			       FIELD_PREP(IPQ5018_PHY_DAC_MASK,
-					  IPQ5018_PHY_MMD1_MDAC_VAL));
-
-		/* setting EDAC (Error-detection and Correction) in debug register */
-		at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
-				      IPQ5018_PHY_DAC_MASK,
-				      FIELD_PREP(IPQ5018_PHY_DAC_MASK,
-						 IPQ5018_PHY_DEBUG_EDAC_VAL));
-	}
+	if (!priv->set_short_cable_dac)
+		return 0;
 
-	return 0;
+	/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
+	ret = phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
+			     IPQ5018_PHY_DAC_MASK,
+			     FIELD_PREP(IPQ5018_PHY_DAC_MASK,
+					IPQ5018_PHY_MMD1_MDAC_VAL));
+	if (ret)
+		return ret;
+
+	/* setting EDAC (Error-detection and Correction) in debug register */
+	return at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
+				     IPQ5018_PHY_DAC_MASK,
+				     FIELD_PREP(IPQ5018_PHY_DAC_MASK,
+						IPQ5018_PHY_DEBUG_EDAC_VAL));
+}
+
+static int ipq5018_config_init(struct phy_device *phydev)
+{
+	return ipq5018_analog_init(phydev);
 }
 
 static void ipq5018_link_change_notify(struct phy_device *phydev)
@@ -1111,6 +1138,21 @@ static int ipq5018_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
+	/*
+	 * The PHY starts autonegotiation as soon as it leaves reset. Apply the
+	 * analog settings now instead of waiting for config_init() at attach
+	 * time, and restart autonegotiation so that it uses them.
+	 */
+	ret = ipq5018_analog_init(phydev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to apply analog settings\n");
+
+	ret = genphy_restart_aneg(phydev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to restart autonegotiation\n");
+
 	return 0;
 }
 
-- 
2.43.0

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

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

[Yongzhao Chen]

Hi Andrew,

> Patch 0/X should contain the big picture, and there is no need to
> repeat it in the individual patches.

Agreed. In the next revision, the commit messages only describe the
problem and the change; the investigation and shared test results are
in the cover letter. The code is unchanged from v2.

Thanks,
Yongzhao Chen