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

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

[Stanislaw Pal]

The reset bit clears well before the RTL8365MB/RTL8367S 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>
---
Changes in v2:
- Declare "remaining" as long instead of unsigned long and drop the cast
  from the comparison, so the sign is carried by the type (Andrew Lunn).
- Name the affected parts in the opening sentence instead of "the chip".
- Link to v1: https://lore.kernel.org/all/20260907191940.806734-1-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_reset_chip(struct realtek_priv *priv)
 {
+	unsigned long deadline;
+	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 = (long)(deadline - jiffies);
+	if (remaining > 0)
+		msleep(jiffies_to_msecs(remaining));
+
+	return 0;
 }
 
 static int rtl8365mb_setup(struct dsa_switch *ds)

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

[Linus Walleij]

On Tue, Sep 8, 2026 at 7:44 PM Stanislaw Pal <kuncy7@gmail.com> wrote:

> The reset bit clears well before the RTL8365MB/RTL8367S 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>

Interesting fix, with an interesting optimization of the reset
deadline. I like it.
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

[Luiz Angelo Daros de Luca]

Hi Stanislaw,

Thanks for your patch.

> The reset bit clears well before the RTL8365MB/RTL8367S 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.

We discussed a similar issue previously:
https://lore.kernel.org/all/20260721115306.15144-1-kuncy7@gmail.com/

In that thread, you mentioned it might have been caused by a bad power
supply. Are there any updates or new findings regarding that?

>  Unbinding and rebinding the driver
> always fixed it at runtime. With this patch: 6 out of 6 clean boots.

This part is still puzzling. Why does an unbind/rebind cycle work
every time if a cold boot fails without the extra delay? After the
reset write, both execution paths should behave identically. Perhaps
the interface state during an unbind/rebind cycle slows down the
probe/setup path enough to give the chip the time it needs?

> Fixes: 4af2950c50c8 ("net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC")
> Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
> ---
> Changes in v2:
> - Declare "remaining" as long instead of unsigned long and drop the cast
>   from the comparison, so the sign is carried by the type (Andrew Lunn).
> - Name the affected parts in the opening sentence instead of "the chip".
> - Link to v1: https://lore.kernel.org/all/20260907191940.806734-1-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_reset_chip(struct realtek_priv *priv)
>  {
> +       unsigned long deadline;
> +       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 = (long)(deadline - jiffies);
> +       if (remaining > 0)
> +               msleep(jiffies_to_msecs(remaining));

Tracking deadline math across regmap_read_poll_timeout() adds
unnecessary complexity. If the chip reliably requires a full 1-second
delay, why not simply msleep(RTL8365MB_CHIP_RESET_TIME_MS) upfront? We
could perform a single regmap_read() check afterward to verify
RTL8365MB_CHIP_RESET_HW_MASK cleared, though even that might be overly
cautious.

FWIW, I noticed that older chips (like the RTL8367R) do require extra
time after the reset bit clears, whereas I haven't seen that on
RTL8367C devices. Checking if RTL8365MB_CHIP_RESET_HW_MASK was cleared
simply might not indicate that the chip has finished booting
internally.

> +
> +       return 0;
>  }
>
>  static int rtl8365mb_setup(struct dsa_switch *ds)
>

Best regards,

Luiz

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

[Stanislaw Pal]

Hi Luiz,

Thanks - all three points are fair, and the first one I owe you a
correction on.


The power supply
----------------

That July message was too broadly worded, and I should have followed up
in that thread rather than leaving it as the last word.

What the A/B/A established still holds, but for a narrower fault than I
claimed. The July signature was link up at 2.5G/Full with 326 FCS errors
and 326 drop events on the switch's CPU-facing port; swapping the supply
made it go away and putting the old one back brought it straight back.
I have no reason to doubt that part.

What I got wrong was declaring the cold-start problem closed. On the new
supply the board later came up broken again, several times, with a
different signature: no FCS errors, no drop events, no CRC or symbol
errors anywhere - the switch simply never forwards what the CPU sends,
and the MIB TX counters on the user ports stay at zero. A power cycle or
a driver re-probe clears it. That is the failure these patches address,
and it happens on a supply that is not faulty.

So: two faults, one supply-related and settled, one not. I conflated
them in July.


Why unbind/rebind always works
------------------------------

I do not have a measurement that settles this, so treat what follows as
a hypothesis.

At rebind the chip has been powered for minutes or hours, so whatever
internal power-on sequence it runs is long finished; the driver's reset
then only restarts the register blocks, and they come back quickly. From
cold, the reset lands while that power-on initialisation is still in
progress, and the two overlap. That would explain why the extra wait
only ever matters on the first probe after power-on, and why every warm
path - rebind, reboot, sysupgrade - is clean regardless.

It also fits your RTL8367R observation: if the reset bit reflects the
register-level reset rather than the completion of the internal boot,
then how much slack there is after the bit clears is a property of the
part, and a driver that keys off the bit alone is relying on that slack
being zero.


The deadline arithmetic
-----------------------

I have no attachment to the implementation, but I would rather not
change it on my own judgement here, because you and Linus have landed on
opposite sides: he reviewed this version specifically liking the
deadline optimisation, and you would rather see the complexity gone.

If you two settle on the simple form I will send a v3 with

	msleep(RTL8365MB_CHIP_RESET_TIME_MS);

up front and a single read of RTL8365MB_CHIP_RESET_HW_MASK afterwards,
returning -ETIMEDOUT if it is still set. Total probe time is the same
either way; what the current version buys is only that a chip whose poll
already took the full second does not wait twice, which on reflection is
not worth the arithmetic if it reads as complexity to the people
maintaining this.


Numbers
-------

One thing I should be straight about: the "one cold boot in seven"
figure was measured with both patches applied together, so it does not
attribute the failures to either one on its own. Johan raised the same
point on the OpenWrt PR. I am running per-arm cold-boot counts now -
neither patch, 714-01 alone, both - on the only board I have that
reproduces this, and I will report the counts when I have enough boots
for them to mean anything. If 714-01 alone turns out to be sufficient,
the second patch should be dropped rather than respun.

Best regards,
Stanislaw

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

[Alvin Šipraga]

Hi Stanislaw, Luiz,

On Wed, Sep 09, 2026 at 12:13:55PM +0200, Stanislaw Pal wrote:
> Hi Luiz,
> 
> Thanks - all three points are fair, and the first one I owe you a
> correction on.
> 
> 
> The power supply
> ----------------
> 
> That July message was too broadly worded, and I should have followed up
> in that thread rather than leaving it as the last word.
> 
> What the A/B/A established still holds, but for a narrower fault than I
> claimed. The July signature was link up at 2.5G/Full with 326 FCS errors
> and 326 drop events on the switch's CPU-facing port; swapping the supply
> made it go away and putting the old one back brought it straight back.
> I have no reason to doubt that part.
> 
> What I got wrong was declaring the cold-start problem closed. On the new
> supply the board later came up broken again, several times, with a
> different signature: no FCS errors, no drop events, no CRC or symbol
> errors anywhere - the switch simply never forwards what the CPU sends,
> and the MIB TX counters on the user ports stay at zero. A power cycle or
> a driver re-probe clears it. That is the failure these patches address,
> and it happens on a supply that is not faulty.
> 
> So: two faults, one supply-related and settled, one not. I conflated
> them in July.
> 
> 
> Why unbind/rebind always works
> ------------------------------
> 
> I do not have a measurement that settles this, so treat what follows as
> a hypothesis.
> 
> At rebind the chip has been powered for minutes or hours, so whatever
> internal power-on sequence it runs is long finished; the driver's reset
> then only restarts the register blocks, and they come back quickly. From
> cold, the reset lands while that power-on initialisation is still in
> progress, and the two overlap. That would explain why the extra wait
> only ever matters on the first probe after power-on, and why every warm
> path - rebind, reboot, sysupgrade - is clean regardless.

Can you share the device tree for the device you are testing on?

Your unbind/bind test observations seem to contradict the headline in
your patch - that the chip reset register bit gets cleared prematurely.
Either the supplies are stable - in which case the "cold probe" scenario
should behave the same as the "warm probe" (unbind/rebind) scenario - or
they're not, in which case we're out of spec and weird behavior is to be
expected.

Oleksij has a separate series recently applied to net-next [1] which
makes the driver consume the regulators. But since your patch is to net,
I assume that you've been testing without that. Maybe it's possible that
the driver probe races with power-up of the regulators? The management
interface supply (DVDDIO?) could accidentally be stable, so you can
still write to the register and observe the bit flipping, but other
random parts remain un(der)-powered?

Assuming you have the regulators described in your device tree, maybe
you can try applying Oleksij's series and seeing if it fixes the problem
for you? Note that some regulators take longer to ramp-up than others,
so this only helps if those timings are properly described. You can try
with some conservative values if you're not sure.

[1] https://lore.kernel.org/all/178839965088.3035212.12735726271200270390.git-patchwork-notify@kernel.org/

All that said, I'm not sure that backporting Oleksij's changes is really
a proper fix for stable, since the supplies were not even part of the
bindings to begin with.

> It also fits your RTL8367R observation: if the reset bit reflects the
> register-level reset rather than the completion of the internal boot,
> then how much slack there is after the bit clears is a property of the
> part, and a driver that keys off the bit alone is relying on that slack
> being zero.
> 
> 
> The deadline arithmetic
> -----------------------
> 
> I have no attachment to the implementation, but I would rather not
> change it on my own judgement here, because you and Linus have landed on
> opposite sides: he reviewed this version specifically liking the
> deadline optimisation, and you would rather see the complexity gone.
> 
> If you two settle on the simple form I will send a v3 with
> 
> 	msleep(RTL8365MB_CHIP_RESET_TIME_MS);
> 
> up front and a single read of RTL8365MB_CHIP_RESET_HW_MASK afterwards,
> returning -ETIMEDOUT if it is still set. Total probe time is the same
> either way; what the current version buys is only that a chip whose poll
> already took the full second does not wait twice, which on reflection is
> not worth the arithmetic if it reads as complexity to the people
> maintaining this.

I don't see much point in polling if you are going to sleep for the same
amount of time regardless. Why not just write the reset bit, msleep(),
and check the bit has cleared - returning -ETIMEDOUT if it hasn't?

> Numbers
> -------
> 
> One thing I should be straight about: the "one cold boot in seven"
> figure was measured with both patches applied together, so it does not
> attribute the failures to either one on its own. Johan raised the same
> point on the OpenWrt PR. I am running per-arm cold-boot counts now -
> neither patch, 714-01 alone, both - on the only board I have that
> reproduces this, and I will report the counts when I have enough boots
> for them to mean anything. If 714-01 alone turns out to be sufficient,
> the second patch should be dropped rather than respun.

Kind regards,
Alvin