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

[Stanislaw Pal <kuncy7@gmail.com>]

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