From: "Arınç ÜNAL" <arinc.unal@arinc9.com>
To: stable@vger.kernel.org
Cc: "Daniel Golle" <daniel@makrotopia.org>,
"Arınç ÜNAL" <arinc.unal@arinc9.com>,
"Vladimir Oltean" <olteanv@gmail.com>,
"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
"Florian Fainelli" <florian.fainelli@broadcom.com>,
"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 2/4] net: dsa: introduce preferred_default_local_cpu_port and use on MT7530
Date: Sat, 20 Apr 2024 19:46:04 +0300 [thread overview]
Message-ID: <20240420-for-stable-5-15-backports-v1-2-007bfa19d044@arinc9.com> (raw)
In-Reply-To: <20240420-for-stable-5-15-backports-v1-0-007bfa19d044@arinc9.com>
From: Vladimir Oltean <olteanv@gmail.com>
[ Upstream commit b79d7c14f48083abb3fb061370c0c64a569edf4c ]
Since the introduction of the OF bindings, DSA has always had a policy that
in case multiple CPU ports are present in the device tree, the numerically
smallest one is always chosen.
The MT7530 switch family, except the switch on the MT7988 SoC, has 2 CPU
ports, 5 and 6, where port 6 is preferable on the MT7531BE switch because
it has higher bandwidth.
The MT7530 driver developers had 3 options:
- to modify DSA when the MT7531 switch support was introduced, such as to
prefer the better port
- to declare both CPU ports in device trees as CPU ports, and live with the
sub-optimal performance resulting from not preferring the better port
- to declare just port 6 in the device tree as a CPU port
Of course they chose the path of least resistance (3rd option), kicking the
can down the road. The hardware description in the device tree is supposed
to be stable - developers are not supposed to adopt the strategy of
piecemeal hardware description, where the device tree is updated in
lockstep with the features that the kernel currently supports.
Now, as a result of the fact that they did that, any attempts to modify the
device tree and describe both CPU ports as CPU ports would make DSA change
its default selection from port 6 to 5, effectively resulting in a
performance degradation visible to users with the MT7531BE switch as can be
seen below.
Without preferring port 6:
[ ID][Role] Interval Transfer Bitrate Retr
[ 5][TX-C] 0.00-20.00 sec 374 MBytes 157 Mbits/sec 734 sender
[ 5][TX-C] 0.00-20.00 sec 373 MBytes 156 Mbits/sec receiver
[ 7][RX-C] 0.00-20.00 sec 1.81 GBytes 778 Mbits/sec 0 sender
[ 7][RX-C] 0.00-20.00 sec 1.81 GBytes 777 Mbits/sec receiver
With preferring port 6:
[ ID][Role] Interval Transfer Bitrate Retr
[ 5][TX-C] 0.00-20.00 sec 1.99 GBytes 856 Mbits/sec 273 sender
[ 5][TX-C] 0.00-20.00 sec 1.99 GBytes 855 Mbits/sec receiver
[ 7][RX-C] 0.00-20.00 sec 1.72 GBytes 737 Mbits/sec 15 sender
[ 7][RX-C] 0.00-20.00 sec 1.71 GBytes 736 Mbits/sec receiver
Using one port for WAN and the other ports for LAN is a very popular use
case which is what this test emulates.
As such, this change proposes that we retroactively modify stable kernels
(which don't support the modification of the CPU port assignments, so as to
let user space fix the problem and restore the throughput) to keep the
mt7530 driver preferring port 6 even with device trees where the hardware
is more fully described.
Fixes: c288575f7810 ("net: dsa: mt7530: Add the support of MT7531 switch")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
drivers/net/dsa/mt7530.c | 15 +++++++++++++++
include/net/dsa.h | 8 ++++++++
net/dsa/dsa2.c | 24 +++++++++++++++++++++++-
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index e3852cb48369..c7192f8ba4ce 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -415,6 +415,20 @@ static void mt7530_pll_setup(struct mt7530_priv *priv)
core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
}
+/* If port 6 is available as a CPU port, always prefer that as the default,
+ * otherwise don't care.
+ */
+static struct dsa_port *
+mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds)
+{
+ struct dsa_port *cpu_dp = dsa_to_port(ds, 6);
+
+ if (dsa_port_is_cpu(cpu_dp))
+ return cpu_dp;
+
+ return NULL;
+}
+
/* Setup port 6 interface mode and TRGMII TX circuit */
static int
mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
@@ -3375,6 +3389,7 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
static const struct dsa_switch_ops mt7530_switch_ops = {
.get_tag_protocol = mtk_get_tag_protocol,
.setup = mt753x_setup,
+ .preferred_default_local_cpu_port = mt753x_preferred_default_local_cpu_port,
.get_strings = mt7530_get_strings,
.get_ethtool_stats = mt7530_get_ethtool_stats,
.get_sset_count = mt7530_get_sset_count,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index bec439c4a085..e57d6e65f27e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -705,6 +705,14 @@ struct dsa_switch_ops {
struct phy_device *phy);
void (*port_disable)(struct dsa_switch *ds, int port);
+ /*
+ * Compatibility between device trees defining multiple CPU ports and
+ * drivers which are not OK to use by default the numerically smallest
+ * CPU port of a switch for its local ports. This can return NULL,
+ * meaning "don't know/don't care".
+ */
+ struct dsa_port *(*preferred_default_local_cpu_port)(struct dsa_switch *ds);
+
/*
* Port's MAC EEE settings
*/
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 9751bee3fb2f..543834e31298 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -386,6 +386,24 @@ static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
return 0;
}
+static struct dsa_port *
+dsa_switch_preferred_default_local_cpu_port(struct dsa_switch *ds)
+{
+ struct dsa_port *cpu_dp;
+
+ if (!ds->ops->preferred_default_local_cpu_port)
+ return NULL;
+
+ cpu_dp = ds->ops->preferred_default_local_cpu_port(ds);
+ if (!cpu_dp)
+ return NULL;
+
+ if (WARN_ON(!dsa_port_is_cpu(cpu_dp) || cpu_dp->ds != ds))
+ return NULL;
+
+ return cpu_dp;
+}
+
/* Perform initial assignment of CPU ports to user ports and DSA links in the
* fabric, giving preference to CPU ports local to each switch. Default to
* using the first CPU port in the switch tree if the port does not have a CPU
@@ -393,12 +411,16 @@ static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
*/
static int dsa_tree_setup_cpu_ports(struct dsa_switch_tree *dst)
{
- struct dsa_port *cpu_dp, *dp;
+ struct dsa_port *preferred_cpu_dp, *cpu_dp, *dp;
list_for_each_entry(cpu_dp, &dst->ports, list) {
if (!dsa_port_is_cpu(cpu_dp))
continue;
+ preferred_cpu_dp = dsa_switch_preferred_default_local_cpu_port(cpu_dp->ds);
+ if (preferred_cpu_dp && preferred_cpu_dp != cpu_dp)
+ continue;
+
list_for_each_entry(dp, &dst->ports, list) {
/* Prefer a local CPU port */
if (dp->ds != cpu_dp->ds)
--
2.40.1
next prev parent reply other threads:[~2024-04-20 16:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-20 16:46 [PATCH 0/4] Please apply these MT7530 DSA subdriver patches to 5.15 Arınç ÜNAL
2024-04-20 16:46 ` [PATCH 1/4] net: dsa: mt7530: set all CPU ports in MT7531_CPU_PMAP Arınç ÜNAL
2024-04-20 16:46 ` Arınç ÜNAL [this message]
2024-04-20 16:46 ` [PATCH 3/4] net: dsa: mt7530: fix improper frames on all 25MHz and 40MHz XTAL MT7530 Arınç ÜNAL
2024-04-20 16:46 ` [PATCH 4/4] net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards Arınç ÜNAL
-- strict thread matches above, loose matches on Subject: below --
2024-04-20 15:59 [PATCH 0/4] Please apply these MT7530 DSA subdriver patches to 6.1 Arınç ÜNAL
2024-04-20 15:59 ` [PATCH 2/4] net: dsa: introduce preferred_default_local_cpu_port and use on MT7530 Arınç ÜNAL
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=20240420-for-stable-5-15-backports-v1-2-007bfa19d044@arinc9.com \
--to=arinc.unal@arinc9.com \
--cc=daniel@makrotopia.org \
--cc=davem@davemloft.net \
--cc=florian.fainelli@broadcom.com \
--cc=olteanv@gmail.com \
--cc=rmk+kernel@armlinux.org.uk \
--cc=stable@vger.kernel.org \
/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).