From: "Alvin Šipraga" <alvin.sipraga@analog.com>
To: Stanislaw Pal <kuncy7@gmail.com>
Cc: Linus Walleij <linusw@kernel.org>,
Luiz Angelo Daros de Luca <luizluca@gmail.com>,
Andrew Lunn <andrew@lunn.ch>, Vladimir Oltean <olteanv@gmail.com>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v3] net: dsa: realtek: rtl8365mb: wait out the full chip reset time
Date: Wed, 9 Sep 2026 15:55:39 +0200 [thread overview]
Message-ID: <aqFaNZ8TqM9e76xN@analog.com> (raw)
In-Reply-To: <20260909120825.33350-1-kuncy7@gmail.com>
On Wed, Sep 09, 2026 at 02:08:25PM +0200, Stanislaw Pal wrote:
> Realtek documentation gives the chip 1 second to reset, and the driver
> says so in a comment, but it only sleeps 100 ms and then polls the reset
> bit and continues as soon as that bit clears. The bit reports that the
> register block is back, not that the chip has finished its internal
> bring-up:
This claim is still speculative IMO. There are a couple of other reasons
why it could be failing on your end - see below for some debugging
approaches.
> Luiz notes that older parts such as the RTL8367R need
> noticeable extra time after it clears, so a driver that keys off the bit
> alone is relying on that margin being zero.
I don't really understand this statement, or how this patch would help
such chips? FWIW I thought Luiz was talking about some older Realtek IP
which isn't compatible with this driver.
If you or Luiz could point me to the vendor API sources, it would be
great, because I can't remember much of my thought process when I wrote
the original reset code. Maybe there are some additional hints in there.
And a reference to the 1 second reset time would be good for this patch
regardless. IIRC it's part of the vendor API documentation?
Unfortunately I don't have access to these things on the current PC.
> Sleep out the documented second before touching anything, then read the
> bit once and fail with -ETIMEDOUT if the chip has not come out of reset.
>
> Probe therefore takes 1 s on every chip the driver supports, not only on
> the part this was found on.
Increasing the probe time so significantly for all users without hard
evidence of its necessity is a hard sell IMO :(
[...]
> Alvin put the dilemma precisely: either the supplies are stable, in
> which case a cold probe and an unbind/rebind should behave the same -
> and on my board they do not - or they are not stable, in which case the
> board is out of spec and its behaviour proves nothing about the driver.
> I think the second branch is the likely one here. For what it is worth
> my device tree describes no regulators for the switch at all, so there
> is nothing for Oleksij's series to consume; I will follow that up
> separately rather than hold this patch to it. The device tree you asked
> about is not upstream yet - it is in the OpenWrt submission at
> https://github.com/openwrt/openwrt/pull/24197, file
> target/linux/qualcommax/dts/ipq5018-archer-ax55-v1.dts; the switch node
> has a reset GPIO and no supplies.
You don't necessarily need to describe the regulators perfectly in order
to test the validity of the claims at the top of this patch. It should
suffice to just introduce some artificial sleeps in the driver which
simulate blocking on regulator enablement.
Not knowing the power topology of the board, I think it would be
interesting to add the sleep in one of two places, and do some cold boot
tests for each:
1. Add an msleep(1000) right at the start of rtl83xx_reset_assert().
2. Add an msleep(1000) right at the end of rtl83xx_reset_deassert().
Don't add both, test each case independently.
If both (1) and (2) work, then I assume that it's a matter of some
always-on regulator(s) which need more time to ramp up.
If only (1) works, then I'm not sure why (2) wouldn't...
If only (2) works, then it could be that the reset signal is also
controlling some regulators that the switch consumes. In that case I
would suggest transforming the reset GPIO into a regulator enable GPIO
and using Oleksij's series to enable.
If neither work, then I wonder what else explains the unbind/bind
success case.
... ahh wait, now I see that you actually put the reset-gpios on the
MDIO bus' devicetree node. Well, in that case you also lose the
post-deassert delay here:
#define REALTEK_HW_START_DELAY 100 /* msecs */
...
if (priv->reset_ctl || priv->reset) {
rtl83xx_reset_assert(priv);
dev_dbg(dev, "asserted RESET\n");
msleep(REALTEK_HW_STOP_DELAY);
rtl83xx_reset_deassert(priv);
msleep(REALTEK_HW_START_DELAY);
dev_dbg(dev, "deasserted RESET\n");
}
Tests (1) and (2) are no-ops if you don't give the switch a reset, so
move the reset-gpios property into the switch node before performing the
tests.
Kind regards,
Alvin
prev parent reply other threads:[~2026-09-09 14:30 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-09 12:08 Stanislaw Pal
2026-09-09 12:30 ` Linus Walleij
2026-09-09 13:55 ` Alvin Šipraga [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aqFaNZ8TqM9e76xN@analog.com \
--to=alvin.sipraga@analog.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=kuncy7@gmail.com \
--cc=linusw@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luizluca@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®