[PATCH net] net: dsa: realtek: rtl8365mb: wait out the full chip reset time

[PATCH net] net: dsa: realtek: rtl8365mb: wait out the full chip reset time

[Stanislaw Pal]

The reset bit clears well before the chip has finished its internal
bring-up, so configuring it right away makes register writes to blocks
that are not up yet get lost. The switch is then left half configured:
the CPU port link comes up and the switch still transmits towards the
CPU, but nothing the CPU sends is ever forwarded - no MIB TX counter
moves on any user port, while the MAC reports every frame as transmitted
without errors.

The driver already documents the 1 s reset time the chip needs and polls
with a 1 s timeout, but stops waiting as soon as the bit clears. Sleep
out the remainder of that second instead, measured from the reset write,
so the total wait stays at 1 s regardless of how long the poll took.

Seen on a TP-Link Archer AX55 v1 (IPQ5018 + RTL8367S, 2.5G HSGMII trunk)
on roughly three out of four boots. Unbinding and rebinding the driver
always fixed it at runtime. With this patch: 6 out of 6 clean boots.

Fixes: 4af2950c50c8 ("net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC")
Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
---
--- a/drivers/net/dsa/realtek/rtl8365mb_main.c
+++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
@@ -136,6 +136,9 @@
 #define RTL8365MB_CHIP_RESET_SW_MASK	0x0002
 #define RTL8365MB_CHIP_RESET_HW_MASK	0x0001
 
+/* Time the chip needs to complete a reset, per Realtek documentation */
+#define RTL8365MB_CHIP_RESET_TIME_MS	1000
+
 /* Interrupt polarity register */
 #define RTL8365MB_INTR_POLARITY_REG	0x1100
 #define   RTL8365MB_INTR_POLARITY_MASK	0x0001
@@ -2980,18 +2983,38 @@ static int rtl8365mb_switch_init(struct
 
 static int rtl8365mb_reset_chip(struct realtek_priv *priv)
 {
+	unsigned long deadline;
+	unsigned long remaining;
 	u32 val;
+	int ret;
 
 	priv->write_reg_noack(priv, RTL8365MB_CHIP_RESET_REG,
 			      FIELD_PREP(RTL8365MB_CHIP_RESET_HW_MASK, 1));
 
+	deadline = jiffies + msecs_to_jiffies(RTL8365MB_CHIP_RESET_TIME_MS);
+
 	/* Realtek documentation says the chip needs 1 second to reset. Sleep
 	 * for 100 ms before accessing any registers to prevent ACK timeouts.
 	 */
 	msleep(100);
-	return regmap_read_poll_timeout(priv->map, RTL8365MB_CHIP_RESET_REG, val,
-					!(val & RTL8365MB_CHIP_RESET_HW_MASK),
-					20000, 1e6);
+	ret = regmap_read_poll_timeout(priv->map, RTL8365MB_CHIP_RESET_REG, val,
+				       !(val & RTL8365MB_CHIP_RESET_HW_MASK),
+				       20000, 1e6);
+	if (ret)
+		return ret;
+
+	/* The bit clearing only means the reset was accepted, not that the
+	 * chip is ready: register writes issued before the documented reset
+	 * time has elapsed are silently dropped by blocks that are still
+	 * coming up, which leaves the switch half configured. Wait out
+	 * whatever is left of that second, measured from the reset write, so
+	 * the poll above does not add to the total.
+	 */
+	remaining = deadline - jiffies;
+	if ((long)remaining > 0)
+		msleep(jiffies_to_msecs(remaining));
+
+	return 0;
 }
 
 static int rtl8365mb_setup(struct dsa_switch *ds)

Re: [PATCH net] net: dsa: realtek: rtl8365mb: wait out the full chip reset time

[Andrew Lunn]

>  static int rtl8365mb_reset_chip(struct realtek_priv *priv)
>  {
> +	unsigned long remaining;

> +	remaining = deadline - jiffies;
> +	if ((long)remaining > 0)
> +		msleep(jiffies_to_msecs(remaining));

Since remaining could be negative, it seems like long would be better
than unsigned long.

     Andrew