From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>,
Alexei Starovoitov <ast@kernel.org>,
bpf@vger.kernel.org, Daniel Borkmann <daniel@iogearbox.net>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>,
Jesper Dangaard Brouer <hawk@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
Jose Abreu <joabreu@synopsys.com>,
linux-arm-kernel@lists.infradead.org,
linux-stm32@st-md-mailman.stormreply.com,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
netdev@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>
Subject: [PATCH RFC 0/6] net: stmmac: convert stmmac "pcs" to phylink
Date: Sun, 12 May 2024 17:28:20 +0100 [thread overview]
Message-ID: <ZkDuJAx7atDXjf5m@shell.armlinux.org.uk> (raw)
Hi,
As I noted recently in a thread (and was ignored) stmmac sucks. (I
won't hide my distain for drivers that make my life as phylink
maintainer more difficult!)
One of the contract conditions for using phylink is that the driver
will _not_ mess with the netif carrier. stmmac developers/maintainers
clearly didn't read that, because stmmac messes with the netif
carrier, which destroys phylink's guarantee that it'll make certain
calls in a particular order (e.g. it won't call mac_link_up() twice
in a row without an intervening mac_link_down().) This is clearly
stated in the phylink documentation.
Thus, this patch set attempts to fix this. Why does it mess with the
netif carrier? It has its own independent PCS implementation that
completely bypasses phylink _while_ phylink is still being used.
This is not acceptable. Either the driver uses phylink, or it doesn't
use phylink. There is no half-way house about this. Therefore, this
driver needs to either be fixed, or needs to stop using phylink.
Since I was ignored when I brought this up, I've hacked together the
following patch set - and it is hacky at the moment. It's also broken
because of recentl changes involving dwmac-qcom-ethqos.c - but there
isn't sufficient information in the driver for me to fix this. The
driver appears to use SGMII at 2500Mbps, which simply does not exist.
What interface mode (and neg_mode) does phylink pass to pcs_config()
in each of the speeds that dwmac-qcom-ethqos.c is interested in.
Without this information, I can't do that conversion. So for the
purposes of this, I've just ignored dwmac-qcom-ethqos.c (which means
it will fail to build.)
The patch splitup is not ideal, but that's not what I'm interested in
here. What I want to hear is the results of testing - does this switch
of the RGMII/SGMII "pcs" stuff to a phylink_pcs work for this driver?
Please don't review the patches, but you are welcome to send fixes to
them. Once we know that the overall implementation works, then I'll
look at how best to split the patches. In the mean time, the present
form is more convenient for making changes and fixing things.
There is still more improvement that's needed here.
Thanks.
drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
drivers/net/ethernet/stmicro/stmmac/common.h | 12 ++-
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 113 ++++++++++++---------
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 108 ++++++++++++--------
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 6 --
drivers/net/ethernet/stmicro/stmmac/hwif.h | 27 ++---
.../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 111 +-------------------
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++--
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c | 57 +++++++++++
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 84 ++-------------
10 files changed, 227 insertions(+), 312 deletions(-)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next reply other threads:[~2024-05-12 16:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-12 16:28 Russell King (Oracle) [this message]
2024-05-12 16:28 ` [PATCH RFC net-next 1/6] net: stmmac: convert sgmii/rgmii "pcs" to phylink Russell King (Oracle)
2024-05-13 23:04 ` [PATCH RFC 0/6] net: stmmac: convert stmmac " Serge Semin
2024-05-13 23:21 ` Russell King (Oracle)
2024-05-13 23:53 ` Serge Semin
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=ZkDuJAx7atDXjf5m@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=alexandre.torgue@foss.st.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=fancer.lancer@gmail.com \
--cc=hawk@kernel.org \
--cc=joabreu@synopsys.com \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=netdev@vger.kernel.org \
--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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).