]>
Commit | Line | Data |
---|---|---|
5b8c8b93 GKH |
1 | From b79d7c14f48083abb3fb061370c0c64a569edf4c Mon Sep 17 00:00:00 2001 |
2 | From: Vladimir Oltean <olteanv@gmail.com> | |
3 | Date: Sat, 17 Jun 2023 09:26:48 +0300 | |
4 | Subject: net: dsa: introduce preferred_default_local_cpu_port and use on MT7530 | |
5 | MIME-Version: 1.0 | |
6 | Content-Type: text/plain; charset=UTF-8 | |
7 | Content-Transfer-Encoding: 8bit | |
8 | ||
9 | From: Vladimir Oltean <olteanv@gmail.com> | |
10 | ||
11 | commit b79d7c14f48083abb3fb061370c0c64a569edf4c upstream. | |
12 | ||
13 | Since the introduction of the OF bindings, DSA has always had a policy that | |
14 | in case multiple CPU ports are present in the device tree, the numerically | |
15 | smallest one is always chosen. | |
16 | ||
17 | The MT7530 switch family, except the switch on the MT7988 SoC, has 2 CPU | |
18 | ports, 5 and 6, where port 6 is preferable on the MT7531BE switch because | |
19 | it has higher bandwidth. | |
20 | ||
21 | The MT7530 driver developers had 3 options: | |
22 | - to modify DSA when the MT7531 switch support was introduced, such as to | |
23 | prefer the better port | |
24 | - to declare both CPU ports in device trees as CPU ports, and live with the | |
25 | sub-optimal performance resulting from not preferring the better port | |
26 | - to declare just port 6 in the device tree as a CPU port | |
27 | ||
28 | Of course they chose the path of least resistance (3rd option), kicking the | |
29 | can down the road. The hardware description in the device tree is supposed | |
30 | to be stable - developers are not supposed to adopt the strategy of | |
31 | piecemeal hardware description, where the device tree is updated in | |
32 | lockstep with the features that the kernel currently supports. | |
33 | ||
34 | Now, as a result of the fact that they did that, any attempts to modify the | |
35 | device tree and describe both CPU ports as CPU ports would make DSA change | |
36 | its default selection from port 6 to 5, effectively resulting in a | |
37 | performance degradation visible to users with the MT7531BE switch as can be | |
38 | seen below. | |
39 | ||
40 | Without preferring port 6: | |
41 | ||
42 | [ ID][Role] Interval Transfer Bitrate Retr | |
43 | [ 5][TX-C] 0.00-20.00 sec 374 MBytes 157 Mbits/sec 734 sender | |
44 | [ 5][TX-C] 0.00-20.00 sec 373 MBytes 156 Mbits/sec receiver | |
45 | [ 7][RX-C] 0.00-20.00 sec 1.81 GBytes 778 Mbits/sec 0 sender | |
46 | [ 7][RX-C] 0.00-20.00 sec 1.81 GBytes 777 Mbits/sec receiver | |
47 | ||
48 | With preferring port 6: | |
49 | ||
50 | [ ID][Role] Interval Transfer Bitrate Retr | |
51 | [ 5][TX-C] 0.00-20.00 sec 1.99 GBytes 856 Mbits/sec 273 sender | |
52 | [ 5][TX-C] 0.00-20.00 sec 1.99 GBytes 855 Mbits/sec receiver | |
53 | [ 7][RX-C] 0.00-20.00 sec 1.72 GBytes 737 Mbits/sec 15 sender | |
54 | [ 7][RX-C] 0.00-20.00 sec 1.71 GBytes 736 Mbits/sec receiver | |
55 | ||
56 | Using one port for WAN and the other ports for LAN is a very popular use | |
57 | case which is what this test emulates. | |
58 | ||
59 | As such, this change proposes that we retroactively modify stable kernels | |
60 | (which don't support the modification of the CPU port assignments, so as to | |
61 | let user space fix the problem and restore the throughput) to keep the | |
62 | mt7530 driver preferring port 6 even with device trees where the hardware | |
63 | is more fully described. | |
64 | ||
65 | Fixes: c288575f7810 ("net: dsa: mt7530: Add the support of MT7531 switch") | |
66 | Signed-off-by: Vladimir Oltean <olteanv@gmail.com> | |
67 | Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> | |
68 | Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> | |
69 | Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com> | |
70 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
71 | Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> | |
72 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
73 | --- | |
74 | drivers/net/dsa/mt7530.c | 15 +++++++++++++++ | |
75 | include/net/dsa.h | 8 ++++++++ | |
76 | net/dsa/dsa2.c | 24 +++++++++++++++++++++++- | |
77 | 3 files changed, 46 insertions(+), 1 deletion(-) | |
78 | ||
79 | --- a/drivers/net/dsa/mt7530.c | |
80 | +++ b/drivers/net/dsa/mt7530.c | |
81 | @@ -415,6 +415,20 @@ static void mt7530_pll_setup(struct mt75 | |
82 | core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN); | |
83 | } | |
84 | ||
85 | +/* If port 6 is available as a CPU port, always prefer that as the default, | |
86 | + * otherwise don't care. | |
87 | + */ | |
88 | +static struct dsa_port * | |
89 | +mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds) | |
90 | +{ | |
91 | + struct dsa_port *cpu_dp = dsa_to_port(ds, 6); | |
92 | + | |
93 | + if (dsa_port_is_cpu(cpu_dp)) | |
94 | + return cpu_dp; | |
95 | + | |
96 | + return NULL; | |
97 | +} | |
98 | + | |
99 | /* Setup port 6 interface mode and TRGMII TX circuit */ | |
100 | static int | |
101 | mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface) | |
102 | @@ -3381,6 +3395,7 @@ static int mt753x_set_mac_eee(struct dsa | |
103 | static const struct dsa_switch_ops mt7530_switch_ops = { | |
104 | .get_tag_protocol = mtk_get_tag_protocol, | |
105 | .setup = mt753x_setup, | |
106 | + .preferred_default_local_cpu_port = mt753x_preferred_default_local_cpu_port, | |
107 | .get_strings = mt7530_get_strings, | |
108 | .get_ethtool_stats = mt7530_get_ethtool_stats, | |
109 | .get_sset_count = mt7530_get_sset_count, | |
110 | --- a/include/net/dsa.h | |
111 | +++ b/include/net/dsa.h | |
112 | @@ -706,6 +706,14 @@ struct dsa_switch_ops { | |
113 | void (*port_disable)(struct dsa_switch *ds, int port); | |
114 | ||
115 | /* | |
116 | + * Compatibility between device trees defining multiple CPU ports and | |
117 | + * drivers which are not OK to use by default the numerically smallest | |
118 | + * CPU port of a switch for its local ports. This can return NULL, | |
119 | + * meaning "don't know/don't care". | |
120 | + */ | |
121 | + struct dsa_port *(*preferred_default_local_cpu_port)(struct dsa_switch *ds); | |
122 | + | |
123 | + /* | |
124 | * Port's MAC EEE settings | |
125 | */ | |
126 | int (*set_mac_eee)(struct dsa_switch *ds, int port, | |
127 | --- a/net/dsa/dsa2.c | |
128 | +++ b/net/dsa/dsa2.c | |
129 | @@ -386,6 +386,24 @@ static int dsa_tree_setup_default_cpu(st | |
130 | return 0; | |
131 | } | |
132 | ||
133 | +static struct dsa_port * | |
134 | +dsa_switch_preferred_default_local_cpu_port(struct dsa_switch *ds) | |
135 | +{ | |
136 | + struct dsa_port *cpu_dp; | |
137 | + | |
138 | + if (!ds->ops->preferred_default_local_cpu_port) | |
139 | + return NULL; | |
140 | + | |
141 | + cpu_dp = ds->ops->preferred_default_local_cpu_port(ds); | |
142 | + if (!cpu_dp) | |
143 | + return NULL; | |
144 | + | |
145 | + if (WARN_ON(!dsa_port_is_cpu(cpu_dp) || cpu_dp->ds != ds)) | |
146 | + return NULL; | |
147 | + | |
148 | + return cpu_dp; | |
149 | +} | |
150 | + | |
151 | /* Perform initial assignment of CPU ports to user ports and DSA links in the | |
152 | * fabric, giving preference to CPU ports local to each switch. Default to | |
153 | * using the first CPU port in the switch tree if the port does not have a CPU | |
154 | @@ -393,12 +411,16 @@ static int dsa_tree_setup_default_cpu(st | |
155 | */ | |
156 | static int dsa_tree_setup_cpu_ports(struct dsa_switch_tree *dst) | |
157 | { | |
158 | - struct dsa_port *cpu_dp, *dp; | |
159 | + struct dsa_port *preferred_cpu_dp, *cpu_dp, *dp; | |
160 | ||
161 | list_for_each_entry(cpu_dp, &dst->ports, list) { | |
162 | if (!dsa_port_is_cpu(cpu_dp)) | |
163 | continue; | |
164 | ||
165 | + preferred_cpu_dp = dsa_switch_preferred_default_local_cpu_port(cpu_dp->ds); | |
166 | + if (preferred_cpu_dp && preferred_cpu_dp != cpu_dp) | |
167 | + continue; | |
168 | + | |
169 | list_for_each_entry(dp, &dst->ports, list) { | |
170 | /* Prefer a local CPU port */ | |
171 | if (dp->ds != cpu_dp->ds) |