From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Xiaolei Wang <xiaolei.wang@windriver.com>
Cc: alexandre.torgue@foss.st.com, joabreu@synopsys.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, mcoquelin.stm32@gmail.com,
netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [net PATCH] net: stmmac: update priv->speed to SPEED_UNKNOWN when link down
Date: Wed, 29 May 2024 09:46:55 +0100 [thread overview]
Message-ID: <Zlbrf8ixl9jeTTIv@shell.armlinux.org.uk> (raw)
In-Reply-To: <20240528092010.439089-1-xiaolei.wang@windriver.com>
On Tue, May 28, 2024 at 05:20:10PM +0800, Xiaolei Wang wrote:
> The CBS parameter can still be configured when the port is
> currently disconnected and link down. This is unreasonable.
> The current speed_div and ptr parameters depend on the negotiated
> speed after uplinking. So When the link is down, update priv->speed
> to SPEED_UNKNOWN and an error log should be added.
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
So what happens if stmmac is connected to a PHY that can negotiate with
the link partner, it has link up at e.g. 1G speed, one configures CBS,
and then the link goes down and comes up at a different speed?
I can't see any way in the stmmac driver that this is handled, which
makes this feature way more buggy than you're referring to here. It
also means that with your patch, if one attempts to configure CBS
when the link is down, it will fail.
To me, commit 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
just looks very buggy.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2024-05-29 8:47 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-28 9:20 [net PATCH] net: stmmac: update priv->speed to SPEED_UNKNOWN when link down Xiaolei Wang
2024-05-28 13:20 ` Andrew Lunn
2024-05-29 0:22 ` xiaolei wang
2024-05-29 0:57 ` Andrew Lunn
2024-05-29 8:48 ` Russell King (Oracle)
2024-05-29 8:46 ` Russell King (Oracle) [this message]
2024-05-29 10:55 ` xiaolei wang
2024-05-29 11:45 ` Russell King (Oracle)
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=Zlbrf8ixl9jeTTIv@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=alexandre.torgue@foss.st.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=joabreu@synopsys.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mcoquelin.stm32@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=xiaolei.wang@windriver.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;
as well as URLs for NNTP newsgroup(s).