* [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 13:49 ` Frank Wunderlich
0 siblings, 0 replies; 48+ messages in thread
From: Frank Wunderlich @ 2022-04-26 13:49 UTC (permalink / raw)
To: linux-mediatek, linux-rockchip
Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
From: Frank Wunderlich <frank-w@public-files.de>
Currently CPU-Port is hardcoded to Port 6.
On BPI-R2-Pro board this port is not connected and only Port 5 is
connected to gmac of SoC.
Replace this hardcoded CPU-Port with a member in mt7530_priv struct
which is set in mt753x_cpu_port_enable to the right port.
I defined a default in probe (in case no CPU-Port will be setup) and
if both cpu-port were setup port 6 will be used like the const prior
this patch.
In mt7531_setup first access is before we know which port should be used
(mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
down.
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
drivers/net/dsa/mt7530.h | 2 +-
2 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ccf4cb944167..4789105b8137 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
return ret;
}
+ priv->cpu_port = port;
/* Enable Mediatek header mode on the cpu port */
mt7530_write(priv, MT7530_PVC_P(port),
PORT_SPEC_TAG);
@@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
* restore the port matrix if the port is the member of a certain
* bridge.
*/
- priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
+ priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
priv->ports[port].enable = true;
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
priv->ports[port].pm);
@@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
struct netlink_ext_ack *extack)
{
struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
- u32 port_bitmap = BIT(MT7530_CPU_PORT);
struct mt7530_priv *priv = ds->priv;
+ u32 port_bitmap = BIT(priv->cpu_port);
mutex_lock(&priv->reg_mutex);
@@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
* the CPU port get out of VLAN filtering mode.
*/
if (all_user_ports_removed) {
- mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
+ mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
PCR_MATRIX(dsa_user_ports(priv->ds)));
- mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
+ mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
| PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
}
}
@@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
*/
if (priv->ports[port].enable)
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
- PCR_MATRIX(BIT(MT7530_CPU_PORT)));
- priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
+ PCR_MATRIX(BIT(priv->cpu_port)));
+ priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
/* When a port is removed from the bridge, the port would be set up
* back to the default as is at initial boot which is a VLAN-unaware
@@ -1503,6 +1504,7 @@ static int
mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
struct netlink_ext_ack *extack)
{
+ struct mt7530_priv *priv = ds->priv;
if (vlan_filtering) {
/* The port is being kept as VLAN-unaware port when bridge is
* set up with vlan_filtering not being set, Otherwise, the
@@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
* for becoming a VLAN-aware port.
*/
mt7530_port_set_vlan_aware(ds, port);
- mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
+ mt7530_port_set_vlan_aware(ds, priv->cpu_port);
} else {
mt7530_port_set_vlan_unaware(ds, port);
}
@@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
u32 val;
new_members = entry->old_members | BIT(entry->port) |
- BIT(MT7530_CPU_PORT);
+ BIT(priv->cpu_port);
/* Validate the entry with independent learning, create egress tag per
* VLAN and joining the port as one of the port members.
@@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
* DSA tag.
*/
mt7530_rmw(priv, MT7530_VAWD2,
- ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
- ETAG_CTRL_P(MT7530_CPU_PORT,
+ ETAG_CTRL_P_MASK(priv->cpu_port),
+ ETAG_CTRL_P(priv->cpu_port,
MT7530_VLAN_EGRESS_STACK));
}
@@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
* the entry would be kept valid. Otherwise, the entry is got to be
* disabled.
*/
- if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
+ if (new_members && new_members != BIT(priv->cpu_port)) {
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
@@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
* controller also is the container for two GMACs nodes representing
* as two netdev instances.
*/
- dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
+ dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
ds->assisted_learning_on_cpu_port = true;
ds->mtu_enforcement_ingress = true;
@@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
CORE_PLL_GROUP4, val);
- /* BPDU to CPU port */
- mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
- BIT(MT7530_CPU_PORT));
- mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
- MT753X_BPDU_CPU_ONLY);
-
- /* Enable and reset MIB counters */
- mt7530_mib_reset(ds);
-
for (i = 0; i < MT7530_NUM_PORTS; i++) {
/* Disable forwarding by default on all ports */
mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
@@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
}
+ /* BPDU to CPU port */
+ mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
+ BIT(priv->cpu_port));
+ mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
+ MT753X_BPDU_CPU_ONLY);
+
+ /* Enable and reset MIB counters */
+ mt7530_mib_reset(ds);
+
/* Setup VLAN ID 0 for VLAN-unaware bridges */
ret = mt7530_setup_vlan0(priv);
if (ret)
@@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
if (!priv)
return -ENOMEM;
+ priv->cpu_port = 6;
+
priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
if (!priv->ds)
return -ENOMEM;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 91508e2feef9..62df8d10f6d4 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -8,7 +8,6 @@
#define MT7530_NUM_PORTS 7
#define MT7530_NUM_PHYS 5
-#define MT7530_CPU_PORT 6
#define MT7530_NUM_FDB_RECORDS 2048
#define MT7530_ALL_MEMBERS 0xff
@@ -823,6 +822,7 @@ struct mt7530_priv {
u8 mirror_tx;
struct mt7530_port ports[MT7530_NUM_PORTS];
+ int cpu_port;
/* protect among processes for registers access*/
struct mutex reg_mutex;
int irq;
--
2.25.1
^ permalink raw reply related [flat|nested] 48+ messages in thread* [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 13:49 ` Frank Wunderlich
0 siblings, 0 replies; 48+ messages in thread
From: Frank Wunderlich @ 2022-04-26 13:49 UTC (permalink / raw)
To: linux-mediatek, linux-rockchip
Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
From: Frank Wunderlich <frank-w@public-files.de>
Currently CPU-Port is hardcoded to Port 6.
On BPI-R2-Pro board this port is not connected and only Port 5 is
connected to gmac of SoC.
Replace this hardcoded CPU-Port with a member in mt7530_priv struct
which is set in mt753x_cpu_port_enable to the right port.
I defined a default in probe (in case no CPU-Port will be setup) and
if both cpu-port were setup port 6 will be used like the const prior
this patch.
In mt7531_setup first access is before we know which port should be used
(mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
down.
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
drivers/net/dsa/mt7530.h | 2 +-
2 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ccf4cb944167..4789105b8137 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
return ret;
}
+ priv->cpu_port = port;
/* Enable Mediatek header mode on the cpu port */
mt7530_write(priv, MT7530_PVC_P(port),
PORT_SPEC_TAG);
@@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
* restore the port matrix if the port is the member of a certain
* bridge.
*/
- priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
+ priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
priv->ports[port].enable = true;
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
priv->ports[port].pm);
@@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
struct netlink_ext_ack *extack)
{
struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
- u32 port_bitmap = BIT(MT7530_CPU_PORT);
struct mt7530_priv *priv = ds->priv;
+ u32 port_bitmap = BIT(priv->cpu_port);
mutex_lock(&priv->reg_mutex);
@@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
* the CPU port get out of VLAN filtering mode.
*/
if (all_user_ports_removed) {
- mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
+ mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
PCR_MATRIX(dsa_user_ports(priv->ds)));
- mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
+ mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
| PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
}
}
@@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
*/
if (priv->ports[port].enable)
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
- PCR_MATRIX(BIT(MT7530_CPU_PORT)));
- priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
+ PCR_MATRIX(BIT(priv->cpu_port)));
+ priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
/* When a port is removed from the bridge, the port would be set up
* back to the default as is at initial boot which is a VLAN-unaware
@@ -1503,6 +1504,7 @@ static int
mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
struct netlink_ext_ack *extack)
{
+ struct mt7530_priv *priv = ds->priv;
if (vlan_filtering) {
/* The port is being kept as VLAN-unaware port when bridge is
* set up with vlan_filtering not being set, Otherwise, the
@@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
* for becoming a VLAN-aware port.
*/
mt7530_port_set_vlan_aware(ds, port);
- mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
+ mt7530_port_set_vlan_aware(ds, priv->cpu_port);
} else {
mt7530_port_set_vlan_unaware(ds, port);
}
@@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
u32 val;
new_members = entry->old_members | BIT(entry->port) |
- BIT(MT7530_CPU_PORT);
+ BIT(priv->cpu_port);
/* Validate the entry with independent learning, create egress tag per
* VLAN and joining the port as one of the port members.
@@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
* DSA tag.
*/
mt7530_rmw(priv, MT7530_VAWD2,
- ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
- ETAG_CTRL_P(MT7530_CPU_PORT,
+ ETAG_CTRL_P_MASK(priv->cpu_port),
+ ETAG_CTRL_P(priv->cpu_port,
MT7530_VLAN_EGRESS_STACK));
}
@@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
* the entry would be kept valid. Otherwise, the entry is got to be
* disabled.
*/
- if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
+ if (new_members && new_members != BIT(priv->cpu_port)) {
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
@@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
* controller also is the container for two GMACs nodes representing
* as two netdev instances.
*/
- dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
+ dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
ds->assisted_learning_on_cpu_port = true;
ds->mtu_enforcement_ingress = true;
@@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
CORE_PLL_GROUP4, val);
- /* BPDU to CPU port */
- mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
- BIT(MT7530_CPU_PORT));
- mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
- MT753X_BPDU_CPU_ONLY);
-
- /* Enable and reset MIB counters */
- mt7530_mib_reset(ds);
-
for (i = 0; i < MT7530_NUM_PORTS; i++) {
/* Disable forwarding by default on all ports */
mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
@@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
}
+ /* BPDU to CPU port */
+ mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
+ BIT(priv->cpu_port));
+ mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
+ MT753X_BPDU_CPU_ONLY);
+
+ /* Enable and reset MIB counters */
+ mt7530_mib_reset(ds);
+
/* Setup VLAN ID 0 for VLAN-unaware bridges */
ret = mt7530_setup_vlan0(priv);
if (ret)
@@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
if (!priv)
return -ENOMEM;
+ priv->cpu_port = 6;
+
priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
if (!priv->ds)
return -ENOMEM;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 91508e2feef9..62df8d10f6d4 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -8,7 +8,6 @@
#define MT7530_NUM_PORTS 7
#define MT7530_NUM_PHYS 5
-#define MT7530_CPU_PORT 6
#define MT7530_NUM_FDB_RECORDS 2048
#define MT7530_ALL_MEMBERS 0xff
@@ -823,6 +822,7 @@ struct mt7530_priv {
u8 mirror_tx;
struct mt7530_port ports[MT7530_NUM_PORTS];
+ int cpu_port;
/* protect among processes for registers access*/
struct mutex reg_mutex;
int irq;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 48+ messages in thread* [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 13:49 ` Frank Wunderlich
0 siblings, 0 replies; 48+ messages in thread
From: Frank Wunderlich @ 2022-04-26 13:49 UTC (permalink / raw)
To: linux-mediatek, linux-rockchip
Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
From: Frank Wunderlich <frank-w@public-files.de>
Currently CPU-Port is hardcoded to Port 6.
On BPI-R2-Pro board this port is not connected and only Port 5 is
connected to gmac of SoC.
Replace this hardcoded CPU-Port with a member in mt7530_priv struct
which is set in mt753x_cpu_port_enable to the right port.
I defined a default in probe (in case no CPU-Port will be setup) and
if both cpu-port were setup port 6 will be used like the const prior
this patch.
In mt7531_setup first access is before we know which port should be used
(mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
down.
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
drivers/net/dsa/mt7530.h | 2 +-
2 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ccf4cb944167..4789105b8137 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
return ret;
}
+ priv->cpu_port = port;
/* Enable Mediatek header mode on the cpu port */
mt7530_write(priv, MT7530_PVC_P(port),
PORT_SPEC_TAG);
@@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
* restore the port matrix if the port is the member of a certain
* bridge.
*/
- priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
+ priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
priv->ports[port].enable = true;
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
priv->ports[port].pm);
@@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
struct netlink_ext_ack *extack)
{
struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
- u32 port_bitmap = BIT(MT7530_CPU_PORT);
struct mt7530_priv *priv = ds->priv;
+ u32 port_bitmap = BIT(priv->cpu_port);
mutex_lock(&priv->reg_mutex);
@@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
* the CPU port get out of VLAN filtering mode.
*/
if (all_user_ports_removed) {
- mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
+ mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
PCR_MATRIX(dsa_user_ports(priv->ds)));
- mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
+ mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
| PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
}
}
@@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
*/
if (priv->ports[port].enable)
mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
- PCR_MATRIX(BIT(MT7530_CPU_PORT)));
- priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
+ PCR_MATRIX(BIT(priv->cpu_port)));
+ priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
/* When a port is removed from the bridge, the port would be set up
* back to the default as is at initial boot which is a VLAN-unaware
@@ -1503,6 +1504,7 @@ static int
mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
struct netlink_ext_ack *extack)
{
+ struct mt7530_priv *priv = ds->priv;
if (vlan_filtering) {
/* The port is being kept as VLAN-unaware port when bridge is
* set up with vlan_filtering not being set, Otherwise, the
@@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
* for becoming a VLAN-aware port.
*/
mt7530_port_set_vlan_aware(ds, port);
- mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
+ mt7530_port_set_vlan_aware(ds, priv->cpu_port);
} else {
mt7530_port_set_vlan_unaware(ds, port);
}
@@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
u32 val;
new_members = entry->old_members | BIT(entry->port) |
- BIT(MT7530_CPU_PORT);
+ BIT(priv->cpu_port);
/* Validate the entry with independent learning, create egress tag per
* VLAN and joining the port as one of the port members.
@@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
* DSA tag.
*/
mt7530_rmw(priv, MT7530_VAWD2,
- ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
- ETAG_CTRL_P(MT7530_CPU_PORT,
+ ETAG_CTRL_P_MASK(priv->cpu_port),
+ ETAG_CTRL_P(priv->cpu_port,
MT7530_VLAN_EGRESS_STACK));
}
@@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
* the entry would be kept valid. Otherwise, the entry is got to be
* disabled.
*/
- if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
+ if (new_members && new_members != BIT(priv->cpu_port)) {
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
@@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
* controller also is the container for two GMACs nodes representing
* as two netdev instances.
*/
- dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
+ dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
ds->assisted_learning_on_cpu_port = true;
ds->mtu_enforcement_ingress = true;
@@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
CORE_PLL_GROUP4, val);
- /* BPDU to CPU port */
- mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
- BIT(MT7530_CPU_PORT));
- mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
- MT753X_BPDU_CPU_ONLY);
-
- /* Enable and reset MIB counters */
- mt7530_mib_reset(ds);
-
for (i = 0; i < MT7530_NUM_PORTS; i++) {
/* Disable forwarding by default on all ports */
mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
@@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
}
+ /* BPDU to CPU port */
+ mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
+ BIT(priv->cpu_port));
+ mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
+ MT753X_BPDU_CPU_ONLY);
+
+ /* Enable and reset MIB counters */
+ mt7530_mib_reset(ds);
+
/* Setup VLAN ID 0 for VLAN-unaware bridges */
ret = mt7530_setup_vlan0(priv);
if (ret)
@@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
if (!priv)
return -ENOMEM;
+ priv->cpu_port = 6;
+
priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
if (!priv->ds)
return -ENOMEM;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 91508e2feef9..62df8d10f6d4 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -8,7 +8,6 @@
#define MT7530_NUM_PORTS 7
#define MT7530_NUM_PHYS 5
-#define MT7530_CPU_PORT 6
#define MT7530_NUM_FDB_RECORDS 2048
#define MT7530_ALL_MEMBERS 0xff
@@ -823,6 +822,7 @@ struct mt7530_priv {
u8 mirror_tx;
struct mt7530_port ports[MT7530_NUM_PORTS];
+ int cpu_port;
/* protect among processes for registers access*/
struct mutex reg_mutex;
int irq;
--
2.25.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
2022-04-26 13:49 ` Frank Wunderlich
(?)
(?)
@ 2022-04-26 15:45 ` Florian Fainelli
-1 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:45 UTC (permalink / raw)
To: Frank Wunderlich, linux-mediatek, linux-rockchip
Cc: Andrew Lunn, Landen Chao, Paolo Abeni, Heiko Stuebner, netdev,
Sean Wang, linux-kernel, David S. Miller, DENG Qingfang,
devicetree, Rob Herring, linux-arm-kernel, Krzysztof Kozlowski,
Matthias Brugger, Jakub Kicinski, Vladimir Oltean, Vivien Didelot,
Peter Geis
On 4/26/22 06:49, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
No need to re-order these two lines.
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
Add a space to separate declaration from code.
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
something and in m7530_probe() use that newly renamed constant to
illustrate that we have a default value assigned, just in case.
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
This can be an unsigned integer since you do not assign negative values.
With that fixes, this looks good to me.
--
Florian
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:45 ` Florian Fainelli
0 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:45 UTC (permalink / raw)
To: Frank Wunderlich, linux-mediatek, linux-rockchip
Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni, Matthias Brugger, Peter Geis,
devicetree, linux-arm-kernel, linux-kernel, netdev
On 4/26/22 06:49, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
No need to re-order these two lines.
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
Add a space to separate declaration from code.
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
something and in m7530_probe() use that newly renamed constant to
illustrate that we have a default value assigned, just in case.
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
This can be an unsigned integer since you do not assign negative values.
With that fixes, this looks good to me.
--
Florian
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:45 ` Florian Fainelli
0 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:45 UTC (permalink / raw)
To: Frank Wunderlich, linux-mediatek, linux-rockchip
Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni, Matthias Brugger, Peter Geis,
devicetree, linux-arm-kernel, linux-kernel, netdev
On 4/26/22 06:49, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
No need to re-order these two lines.
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
Add a space to separate declaration from code.
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
something and in m7530_probe() use that newly renamed constant to
illustrate that we have a default value assigned, just in case.
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
This can be an unsigned integer since you do not assign negative values.
With that fixes, this looks good to me.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:45 ` Florian Fainelli
0 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:45 UTC (permalink / raw)
To: Frank Wunderlich, linux-mediatek, linux-rockchip
Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni, Matthias Brugger, Peter Geis,
devicetree, linux-arm-kernel, linux-kernel, netdev
On 4/26/22 06:49, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
No need to re-order these two lines.
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
Add a space to separate declaration from code.
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
something and in m7530_probe() use that newly renamed constant to
illustrate that we have a default value assigned, just in case.
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
This can be an unsigned integer since you do not assign negative values.
With that fixes, this looks good to me.
--
Florian
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 48+ messages in thread* Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
2022-04-26 15:45 ` Florian Fainelli
(?)
(?)
@ 2022-04-26 15:54 ` Frank Wunderlich
-1 siblings, 0 replies; 48+ messages in thread
From: Frank Wunderlich @ 2022-04-26 15:54 UTC (permalink / raw)
To: Florian Fainelli
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
Hi
thanks for fast review.
> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
> Von: "Florian Fainelli" <f.fainelli@gmail.com>
> On 4/26/22 06:49, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> > struct netlink_ext_ack *extack)
> > {
> > struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> > - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> > struct mt7530_priv *priv = ds->priv;
> > + u32 port_bitmap = BIT(priv->cpu_port);
>
> No need to re-order these two lines.
imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
> >
> > mutex_lock(&priv->reg_mutex);
> > @@ -1503,6 +1504,7 @@ static int
> > mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> > struct netlink_ext_ack *extack)
> > {
> > + struct mt7530_priv *priv = ds->priv;
>
> Add a space to separate declaration from code.
OK
> > --- a/drivers/net/dsa/mt7530.h
> > +++ b/drivers/net/dsa/mt7530.h
> > @@ -8,7 +8,6 @@
> >
> > #define MT7530_NUM_PORTS 7
> > #define MT7530_NUM_PHYS 5
> > -#define MT7530_CPU_PORT 6
>
> We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
> something and in m7530_probe() use that newly renamed constant to
> illustrate that we have a default value assigned, just in case.
i do
> > #define MT7530_NUM_FDB_RECORDS 2048
> > #define MT7530_ALL_MEMBERS 0xff
> >
> > @@ -823,6 +822,7 @@ struct mt7530_priv {
> > u8 mirror_tx;
> >
> > struct mt7530_port ports[MT7530_NUM_PORTS];
> > + int cpu_port;
>
> This can be an unsigned integer since you do not assign negative values.
> With that fixes, this looks good to me.
ok, i change to unsigned int
regards Frank
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 48+ messages in thread* Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:54 ` Frank Wunderlich
0 siblings, 0 replies; 48+ messages in thread
From: Frank Wunderlich @ 2022-04-26 15:54 UTC (permalink / raw)
To: Florian Fainelli
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
Hi
thanks for fast review.
> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
> Von: "Florian Fainelli" <f.fainelli@gmail.com>
> On 4/26/22 06:49, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> > struct netlink_ext_ack *extack)
> > {
> > struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> > - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> > struct mt7530_priv *priv = ds->priv;
> > + u32 port_bitmap = BIT(priv->cpu_port);
>
> No need to re-order these two lines.
imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
> >
> > mutex_lock(&priv->reg_mutex);
> > @@ -1503,6 +1504,7 @@ static int
> > mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> > struct netlink_ext_ack *extack)
> > {
> > + struct mt7530_priv *priv = ds->priv;
>
> Add a space to separate declaration from code.
OK
> > --- a/drivers/net/dsa/mt7530.h
> > +++ b/drivers/net/dsa/mt7530.h
> > @@ -8,7 +8,6 @@
> >
> > #define MT7530_NUM_PORTS 7
> > #define MT7530_NUM_PHYS 5
> > -#define MT7530_CPU_PORT 6
>
> We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
> something and in m7530_probe() use that newly renamed constant to
> illustrate that we have a default value assigned, just in case.
i do
> > #define MT7530_NUM_FDB_RECORDS 2048
> > #define MT7530_ALL_MEMBERS 0xff
> >
> > @@ -823,6 +822,7 @@ struct mt7530_priv {
> > u8 mirror_tx;
> >
> > struct mt7530_port ports[MT7530_NUM_PORTS];
> > + int cpu_port;
>
> This can be an unsigned integer since you do not assign negative values.
> With that fixes, this looks good to me.
ok, i change to unsigned int
regards Frank
^ permalink raw reply [flat|nested] 48+ messages in thread* Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:54 ` Frank Wunderlich
0 siblings, 0 replies; 48+ messages in thread
From: Frank Wunderlich @ 2022-04-26 15:54 UTC (permalink / raw)
To: Florian Fainelli
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
Hi
thanks for fast review.
> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
> Von: "Florian Fainelli" <f.fainelli@gmail.com>
> On 4/26/22 06:49, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> > struct netlink_ext_ack *extack)
> > {
> > struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> > - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> > struct mt7530_priv *priv = ds->priv;
> > + u32 port_bitmap = BIT(priv->cpu_port);
>
> No need to re-order these two lines.
imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
> >
> > mutex_lock(&priv->reg_mutex);
> > @@ -1503,6 +1504,7 @@ static int
> > mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> > struct netlink_ext_ack *extack)
> > {
> > + struct mt7530_priv *priv = ds->priv;
>
> Add a space to separate declaration from code.
OK
> > --- a/drivers/net/dsa/mt7530.h
> > +++ b/drivers/net/dsa/mt7530.h
> > @@ -8,7 +8,6 @@
> >
> > #define MT7530_NUM_PORTS 7
> > #define MT7530_NUM_PHYS 5
> > -#define MT7530_CPU_PORT 6
>
> We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
> something and in m7530_probe() use that newly renamed constant to
> illustrate that we have a default value assigned, just in case.
i do
> > #define MT7530_NUM_FDB_RECORDS 2048
> > #define MT7530_ALL_MEMBERS 0xff
> >
> > @@ -823,6 +822,7 @@ struct mt7530_priv {
> > u8 mirror_tx;
> >
> > struct mt7530_port ports[MT7530_NUM_PORTS];
> > + int cpu_port;
>
> This can be an unsigned integer since you do not assign negative values.
> With that fixes, this looks good to me.
ok, i change to unsigned int
regards Frank
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 48+ messages in thread* Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:54 ` Frank Wunderlich
0 siblings, 0 replies; 48+ messages in thread
From: Frank Wunderlich @ 2022-04-26 15:54 UTC (permalink / raw)
To: Florian Fainelli
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
Hi
thanks for fast review.
> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
> Von: "Florian Fainelli" <f.fainelli@gmail.com>
> On 4/26/22 06:49, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> > struct netlink_ext_ack *extack)
> > {
> > struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> > - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> > struct mt7530_priv *priv = ds->priv;
> > + u32 port_bitmap = BIT(priv->cpu_port);
>
> No need to re-order these two lines.
imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
> >
> > mutex_lock(&priv->reg_mutex);
> > @@ -1503,6 +1504,7 @@ static int
> > mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> > struct netlink_ext_ack *extack)
> > {
> > + struct mt7530_priv *priv = ds->priv;
>
> Add a space to separate declaration from code.
OK
> > --- a/drivers/net/dsa/mt7530.h
> > +++ b/drivers/net/dsa/mt7530.h
> > @@ -8,7 +8,6 @@
> >
> > #define MT7530_NUM_PORTS 7
> > #define MT7530_NUM_PHYS 5
> > -#define MT7530_CPU_PORT 6
>
> We could have kept this define and rename it MT7530_DEFAULT_CPU_PORT or
> something and in m7530_probe() use that newly renamed constant to
> illustrate that we have a default value assigned, just in case.
i do
> > #define MT7530_NUM_FDB_RECORDS 2048
> > #define MT7530_ALL_MEMBERS 0xff
> >
> > @@ -823,6 +822,7 @@ struct mt7530_priv {
> > u8 mirror_tx;
> >
> > struct mt7530_port ports[MT7530_NUM_PORTS];
> > + int cpu_port;
>
> This can be an unsigned integer since you do not assign negative values.
> With that fixes, this looks good to me.
ok, i change to unsigned int
regards Frank
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
2022-04-26 15:54 ` Frank Wunderlich
(?)
(?)
@ 2022-04-26 15:56 ` Florian Fainelli
-1 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:56 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
On 4/26/22 08:54, Frank Wunderlich wrote:
> Hi
>
> thanks for fast review.
>
>> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
>> Von: "Florian Fainelli" <f.fainelli@gmail.com>
>> On 4/26/22 06:49, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>
>>> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
>>> struct netlink_ext_ack *extack)
>>> {
>>> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
>>> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
>>> struct mt7530_priv *priv = ds->priv;
>>> + u32 port_bitmap = BIT(priv->cpu_port);
>>
>> No need to re-order these two lines.
>
> imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
My bad, yes, I was concerned with preserving the reserve christmas tree
style, never mind.
--
Florian
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:56 ` Florian Fainelli
0 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:56 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
On 4/26/22 08:54, Frank Wunderlich wrote:
> Hi
>
> thanks for fast review.
>
>> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
>> Von: "Florian Fainelli" <f.fainelli@gmail.com>
>> On 4/26/22 06:49, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>
>>> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
>>> struct netlink_ext_ack *extack)
>>> {
>>> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
>>> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
>>> struct mt7530_priv *priv = ds->priv;
>>> + u32 port_bitmap = BIT(priv->cpu_port);
>>
>> No need to re-order these two lines.
>
> imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
My bad, yes, I was concerned with preserving the reserve christmas tree
style, never mind.
--
Florian
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:56 ` Florian Fainelli
0 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:56 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
On 4/26/22 08:54, Frank Wunderlich wrote:
> Hi
>
> thanks for fast review.
>
>> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
>> Von: "Florian Fainelli" <f.fainelli@gmail.com>
>> On 4/26/22 06:49, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>
>>> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
>>> struct netlink_ext_ack *extack)
>>> {
>>> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
>>> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
>>> struct mt7530_priv *priv = ds->priv;
>>> + u32 port_bitmap = BIT(priv->cpu_port);
>>
>> No need to re-order these two lines.
>
> imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
My bad, yes, I was concerned with preserving the reserve christmas tree
style, never mind.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: Aw: Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-26 15:56 ` Florian Fainelli
0 siblings, 0 replies; 48+ messages in thread
From: Florian Fainelli @ 2022-04-26 15:56 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Frank Wunderlich, linux-mediatek, linux-rockchip, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
On 4/26/22 08:54, Frank Wunderlich wrote:
> Hi
>
> thanks for fast review.
>
>> Gesendet: Dienstag, 26. April 2022 um 17:45 Uhr
>> Von: "Florian Fainelli" <f.fainelli@gmail.com>
>> On 4/26/22 06:49, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>
>>> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
>>> struct netlink_ext_ack *extack)
>>> {
>>> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
>>> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
>>> struct mt7530_priv *priv = ds->priv;
>>> + u32 port_bitmap = BIT(priv->cpu_port);
>>
>> No need to re-order these two lines.
>
> imho it is needed as i now access priv-struct now ;) which is not available at the "old" position
My bad, yes, I was concerned with preserving the reserve christmas tree
style, never mind.
--
Florian
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
2022-04-26 13:49 ` Frank Wunderlich
(?)
(?)
@ 2022-04-27 11:28 ` Vladimir Oltean
-1 siblings, 0 replies; 48+ messages in thread
From: Vladimir Oltean @ 2022-04-27 11:28 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Andrew Lunn, Landen Chao, Florian Fainelli, DENG Qingfang,
Heiko Stuebner, netdev, Sean Wang, linux-kernel, David S. Miller,
linux-rockchip, devicetree, Rob Herring, linux-mediatek,
linux-arm-kernel, Krzysztof Kozlowski, Matthias Brugger,
Jakub Kicinski, Paolo Abeni, Vivien Didelot, Peter Geis
On Tue, Apr 26, 2022 at 03:49:23PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
This is called with an "int port" argument, so could you please do:
struct dsa_port *dp = dsa_to_port(ds, port);
if (dsa_port_is_user(dp)) {
struct dsa_port *cpu_dp = dp->cpu_dp;
priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));
}
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
Here we know for certain that "port" is a user port, so could you please:
struct dsa_port *cpu_dp = dp->cpu_dp;
u32 port_bitmap = BIT(cpu_dp->index);
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
Same idea here, go through dp->cpu_dp.
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
Same.
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
Same.
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
This one is interesting. For some reason, the driver author felt the
need to explicitly add BIT(MT7530_CPU_PORT) to new_members, even though
mt7530_port_vlan_add() will be called on the CPU port too
bridge vlan add dev swp0 vid 100
bridge vlan add dev br0 vid 100 self # here
I would first make a patch that leaves it up to DSA to call
port_vlan_add for the CPU port, rather than doing it implicitly.
There is a certain advantage to that too. You can do autonomous
forwarding in a certain VLAN, but not add br0 to that VLAN, and then,
you could avoid flooding the CPU with those packets, if you know that
software doesn't need to process them.
The modified function would look like this:
static void
mt7530_hw_vlan_add(struct mt7530_priv *priv,
struct mt7530_hw_vlan_entry *entry)
{
struct dsa_port *dp = dsa_to_port(priv->ds, entry->port);
u8 new_members;
u32 val;
new_members = entry->old_members | BIT(entry->port);
/* Validate the entry with independent learning, create egress tag per
* VLAN and joining the port as one of the port members.
*/
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | FID(FID_BRIDGED) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
/* Decide whether adding tag or not for those outgoing packets from the
* port inside the VLAN.
* CPU port is always taken as a tagged port for serving more than one
* VLANs across and also being applied with egress type stack mode for
* that VLAN tags would be appended after hardware special tag used as
* DSA tag.
*/
if (dsa_port_is_cpu(dp))
val = MT7530_VLAN_EGRESS_STACK;
else if (entry->untagged)
val = MT7530_VLAN_EGRESS_UNTAG;
else
val = MT7530_VLAN_EGRESS_TAG;
mt7530_rmw(priv, MT7530_VAWD2,
ETAG_CTRL_P_MASK(entry->port),
ETAG_CTRL_P(entry->port, val));
}
Then please confirm that there aren't regressions in local traffic
termination for a VLAN-aware bridge. Pinging over a bridge with
vlan_filtering=1 should be sufficient, since that is done in the default
pvid of 1.
With this change, you no longer need to concern yourself about the fixed
value of MT7530_CPU_PORT.
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
Here the logic is again too convoluted for its own good. Let's think
before changing.
What I think was happening was this. DSA used to be super blind when
adding a VLAN to the CPU port. Since there is no netdev for the CPU
port, then what DSA used to do was to implicitly program a bridge VLAN to
the CPU port whenever that VLAN was programmed on a user port. The
downside is that you never know when you can remove that VLAN, since the
CPU port is shared between multiple user ports and refcounting was
impossible due to technical switchdev API details.
Here, the mt7530 driver tries to outsmart DSA by saying "hey, if the CPU
port is the only remaining member of the VLAN, this VLAN is garbage,
let's remove it". Which, I mean, fair point.
But since commit 134ef2388e7f ("net: dsa: add explicit support for host
bridge VLANs"), DSA does a better job of keeping track of VLANs added on
CPU ports, and it doesn't leave dangling VLANs behind. You need to trust it.
So in this case, you would have to change the code like this:
if (new_members) {
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
} else {
mt7530_write(priv, MT7530_VAWD1, 0);
mt7530_write(priv, MT7530_VAWD2, 0);
}
and then delete the comment, it isn't relevant any longer.
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
Holy ####, this is convoluted. This gets us the OF node of the container
of the DSA master, alright.
Could you do something more generic? Something like this:
struct device_node *dn = NULL;
struct dsa_port *cpu_dp;
dsa_switch_for_each_cpu_port(cpu_dp, ds) {
dn = cpu_dp->master->dev.of_node->parent;
/* It doesn't matter which CPU port is found first,
* their masters should share the same parent OF node
*/
break;
}
if (!dn) {
ERROR ERROR ERROR, no CPU port
}
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
And this should no longer be needed.
> /* protect among processes for registers access*/
> struct mutex reg_mutex;
> int irq;
> --
> 2.25.1
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-27 11:28 ` Vladimir Oltean
0 siblings, 0 replies; 48+ messages in thread
From: Vladimir Oltean @ 2022-04-27 11:28 UTC (permalink / raw)
To: Frank Wunderlich
Cc: linux-mediatek, linux-rockchip, Frank Wunderlich, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
On Tue, Apr 26, 2022 at 03:49:23PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
This is called with an "int port" argument, so could you please do:
struct dsa_port *dp = dsa_to_port(ds, port);
if (dsa_port_is_user(dp)) {
struct dsa_port *cpu_dp = dp->cpu_dp;
priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));
}
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
Here we know for certain that "port" is a user port, so could you please:
struct dsa_port *cpu_dp = dp->cpu_dp;
u32 port_bitmap = BIT(cpu_dp->index);
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
Same idea here, go through dp->cpu_dp.
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
Same.
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
Same.
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
This one is interesting. For some reason, the driver author felt the
need to explicitly add BIT(MT7530_CPU_PORT) to new_members, even though
mt7530_port_vlan_add() will be called on the CPU port too
bridge vlan add dev swp0 vid 100
bridge vlan add dev br0 vid 100 self # here
I would first make a patch that leaves it up to DSA to call
port_vlan_add for the CPU port, rather than doing it implicitly.
There is a certain advantage to that too. You can do autonomous
forwarding in a certain VLAN, but not add br0 to that VLAN, and then,
you could avoid flooding the CPU with those packets, if you know that
software doesn't need to process them.
The modified function would look like this:
static void
mt7530_hw_vlan_add(struct mt7530_priv *priv,
struct mt7530_hw_vlan_entry *entry)
{
struct dsa_port *dp = dsa_to_port(priv->ds, entry->port);
u8 new_members;
u32 val;
new_members = entry->old_members | BIT(entry->port);
/* Validate the entry with independent learning, create egress tag per
* VLAN and joining the port as one of the port members.
*/
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | FID(FID_BRIDGED) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
/* Decide whether adding tag or not for those outgoing packets from the
* port inside the VLAN.
* CPU port is always taken as a tagged port for serving more than one
* VLANs across and also being applied with egress type stack mode for
* that VLAN tags would be appended after hardware special tag used as
* DSA tag.
*/
if (dsa_port_is_cpu(dp))
val = MT7530_VLAN_EGRESS_STACK;
else if (entry->untagged)
val = MT7530_VLAN_EGRESS_UNTAG;
else
val = MT7530_VLAN_EGRESS_TAG;
mt7530_rmw(priv, MT7530_VAWD2,
ETAG_CTRL_P_MASK(entry->port),
ETAG_CTRL_P(entry->port, val));
}
Then please confirm that there aren't regressions in local traffic
termination for a VLAN-aware bridge. Pinging over a bridge with
vlan_filtering=1 should be sufficient, since that is done in the default
pvid of 1.
With this change, you no longer need to concern yourself about the fixed
value of MT7530_CPU_PORT.
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
Here the logic is again too convoluted for its own good. Let's think
before changing.
What I think was happening was this. DSA used to be super blind when
adding a VLAN to the CPU port. Since there is no netdev for the CPU
port, then what DSA used to do was to implicitly program a bridge VLAN to
the CPU port whenever that VLAN was programmed on a user port. The
downside is that you never know when you can remove that VLAN, since the
CPU port is shared between multiple user ports and refcounting was
impossible due to technical switchdev API details.
Here, the mt7530 driver tries to outsmart DSA by saying "hey, if the CPU
port is the only remaining member of the VLAN, this VLAN is garbage,
let's remove it". Which, I mean, fair point.
But since commit 134ef2388e7f ("net: dsa: add explicit support for host
bridge VLANs"), DSA does a better job of keeping track of VLANs added on
CPU ports, and it doesn't leave dangling VLANs behind. You need to trust it.
So in this case, you would have to change the code like this:
if (new_members) {
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
} else {
mt7530_write(priv, MT7530_VAWD1, 0);
mt7530_write(priv, MT7530_VAWD2, 0);
}
and then delete the comment, it isn't relevant any longer.
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
Holy ####, this is convoluted. This gets us the OF node of the container
of the DSA master, alright.
Could you do something more generic? Something like this:
struct device_node *dn = NULL;
struct dsa_port *cpu_dp;
dsa_switch_for_each_cpu_port(cpu_dp, ds) {
dn = cpu_dp->master->dev.of_node->parent;
/* It doesn't matter which CPU port is found first,
* their masters should share the same parent OF node
*/
break;
}
if (!dn) {
ERROR ERROR ERROR, no CPU port
}
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
And this should no longer be needed.
> /* protect among processes for registers access*/
> struct mutex reg_mutex;
> int irq;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-27 11:28 ` Vladimir Oltean
0 siblings, 0 replies; 48+ messages in thread
From: Vladimir Oltean @ 2022-04-27 11:28 UTC (permalink / raw)
To: Frank Wunderlich
Cc: linux-mediatek, linux-rockchip, Frank Wunderlich, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
On Tue, Apr 26, 2022 at 03:49:23PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
This is called with an "int port" argument, so could you please do:
struct dsa_port *dp = dsa_to_port(ds, port);
if (dsa_port_is_user(dp)) {
struct dsa_port *cpu_dp = dp->cpu_dp;
priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));
}
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
Here we know for certain that "port" is a user port, so could you please:
struct dsa_port *cpu_dp = dp->cpu_dp;
u32 port_bitmap = BIT(cpu_dp->index);
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
Same idea here, go through dp->cpu_dp.
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
Same.
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
Same.
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
This one is interesting. For some reason, the driver author felt the
need to explicitly add BIT(MT7530_CPU_PORT) to new_members, even though
mt7530_port_vlan_add() will be called on the CPU port too
bridge vlan add dev swp0 vid 100
bridge vlan add dev br0 vid 100 self # here
I would first make a patch that leaves it up to DSA to call
port_vlan_add for the CPU port, rather than doing it implicitly.
There is a certain advantage to that too. You can do autonomous
forwarding in a certain VLAN, but not add br0 to that VLAN, and then,
you could avoid flooding the CPU with those packets, if you know that
software doesn't need to process them.
The modified function would look like this:
static void
mt7530_hw_vlan_add(struct mt7530_priv *priv,
struct mt7530_hw_vlan_entry *entry)
{
struct dsa_port *dp = dsa_to_port(priv->ds, entry->port);
u8 new_members;
u32 val;
new_members = entry->old_members | BIT(entry->port);
/* Validate the entry with independent learning, create egress tag per
* VLAN and joining the port as one of the port members.
*/
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | FID(FID_BRIDGED) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
/* Decide whether adding tag or not for those outgoing packets from the
* port inside the VLAN.
* CPU port is always taken as a tagged port for serving more than one
* VLANs across and also being applied with egress type stack mode for
* that VLAN tags would be appended after hardware special tag used as
* DSA tag.
*/
if (dsa_port_is_cpu(dp))
val = MT7530_VLAN_EGRESS_STACK;
else if (entry->untagged)
val = MT7530_VLAN_EGRESS_UNTAG;
else
val = MT7530_VLAN_EGRESS_TAG;
mt7530_rmw(priv, MT7530_VAWD2,
ETAG_CTRL_P_MASK(entry->port),
ETAG_CTRL_P(entry->port, val));
}
Then please confirm that there aren't regressions in local traffic
termination for a VLAN-aware bridge. Pinging over a bridge with
vlan_filtering=1 should be sufficient, since that is done in the default
pvid of 1.
With this change, you no longer need to concern yourself about the fixed
value of MT7530_CPU_PORT.
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
Here the logic is again too convoluted for its own good. Let's think
before changing.
What I think was happening was this. DSA used to be super blind when
adding a VLAN to the CPU port. Since there is no netdev for the CPU
port, then what DSA used to do was to implicitly program a bridge VLAN to
the CPU port whenever that VLAN was programmed on a user port. The
downside is that you never know when you can remove that VLAN, since the
CPU port is shared between multiple user ports and refcounting was
impossible due to technical switchdev API details.
Here, the mt7530 driver tries to outsmart DSA by saying "hey, if the CPU
port is the only remaining member of the VLAN, this VLAN is garbage,
let's remove it". Which, I mean, fair point.
But since commit 134ef2388e7f ("net: dsa: add explicit support for host
bridge VLANs"), DSA does a better job of keeping track of VLANs added on
CPU ports, and it doesn't leave dangling VLANs behind. You need to trust it.
So in this case, you would have to change the code like this:
if (new_members) {
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
} else {
mt7530_write(priv, MT7530_VAWD1, 0);
mt7530_write(priv, MT7530_VAWD2, 0);
}
and then delete the comment, it isn't relevant any longer.
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
Holy ####, this is convoluted. This gets us the OF node of the container
of the DSA master, alright.
Could you do something more generic? Something like this:
struct device_node *dn = NULL;
struct dsa_port *cpu_dp;
dsa_switch_for_each_cpu_port(cpu_dp, ds) {
dn = cpu_dp->master->dev.of_node->parent;
/* It doesn't matter which CPU port is found first,
* their masters should share the same parent OF node
*/
break;
}
if (!dn) {
ERROR ERROR ERROR, no CPU port
}
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
And this should no longer be needed.
> /* protect among processes for registers access*/
> struct mutex reg_mutex;
> int irq;
> --
> 2.25.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [RFC v1 2/3] net: dsa: mt753x: make CPU-Port dynamic
@ 2022-04-27 11:28 ` Vladimir Oltean
0 siblings, 0 replies; 48+ messages in thread
From: Vladimir Oltean @ 2022-04-27 11:28 UTC (permalink / raw)
To: Frank Wunderlich
Cc: linux-mediatek, linux-rockchip, Frank Wunderlich, Rob Herring,
Krzysztof Kozlowski, Heiko Stuebner, Sean Wang, Landen Chao,
DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S. Miller, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
Peter Geis, devicetree, linux-arm-kernel, linux-kernel, netdev
On Tue, Apr 26, 2022 at 03:49:23PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> Currently CPU-Port is hardcoded to Port 6.
>
> On BPI-R2-Pro board this port is not connected and only Port 5 is
> connected to gmac of SoC.
>
> Replace this hardcoded CPU-Port with a member in mt7530_priv struct
> which is set in mt753x_cpu_port_enable to the right port.
>
> I defined a default in probe (in case no CPU-Port will be setup) and
> if both cpu-port were setup port 6 will be used like the const prior
> this patch.
>
> In mt7531_setup first access is before we know which port should be used
> (mt753x_cpu_port_enable) so section "BPDU to CPU port" needs to be moved
> down.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> drivers/net/dsa/mt7530.c | 46 ++++++++++++++++++++++------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ccf4cb944167..4789105b8137 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1004,6 +1004,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
> return ret;
> }
>
> + priv->cpu_port = port;
> /* Enable Mediatek header mode on the cpu port */
> mt7530_write(priv, MT7530_PVC_P(port),
> PORT_SPEC_TAG);
> @@ -1041,7 +1042,7 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
> * restore the port matrix if the port is the member of a certain
> * bridge.
> */
> - priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + priv->ports[port].pm |= PCR_MATRIX(BIT(priv->cpu_port));
This is called with an "int port" argument, so could you please do:
struct dsa_port *dp = dsa_to_port(ds, port);
if (dsa_port_is_user(dp)) {
struct dsa_port *cpu_dp = dp->cpu_dp;
priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));
}
> priv->ports[port].enable = true;
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> priv->ports[port].pm);
> @@ -1190,8 +1191,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> struct netlink_ext_ack *extack)
> {
> struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
> - u32 port_bitmap = BIT(MT7530_CPU_PORT);
> struct mt7530_priv *priv = ds->priv;
> + u32 port_bitmap = BIT(priv->cpu_port);
Here we know for certain that "port" is a user port, so could you please:
struct dsa_port *cpu_dp = dp->cpu_dp;
u32 port_bitmap = BIT(cpu_dp->index);
>
> mutex_lock(&priv->reg_mutex);
>
> @@ -1267,9 +1268,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
> * the CPU port get out of VLAN filtering mode.
> */
> if (all_user_ports_removed) {
> - mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
> + mt7530_write(priv, MT7530_PCR_P(priv->cpu_port),
> PCR_MATRIX(dsa_user_ports(priv->ds)));
> - mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
> + mt7530_write(priv, MT7530_PVC_P(priv->cpu_port), PORT_SPEC_TAG
> | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
> }
Same idea here, go through dp->cpu_dp.
> @@ -1335,8 +1336,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> */
> if (priv->ports[port].enable)
> mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
> - PCR_MATRIX(BIT(MT7530_CPU_PORT)));
> - priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
> + PCR_MATRIX(BIT(priv->cpu_port)));
> + priv->ports[port].pm = PCR_MATRIX(BIT(priv->cpu_port));
Same.
>
> /* When a port is removed from the bridge, the port would be set up
> * back to the default as is at initial boot which is a VLAN-unaware
> @@ -1503,6 +1504,7 @@ static int
> mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> struct netlink_ext_ack *extack)
> {
> + struct mt7530_priv *priv = ds->priv;
> if (vlan_filtering) {
> /* The port is being kept as VLAN-unaware port when bridge is
> * set up with vlan_filtering not being set, Otherwise, the
> @@ -1510,7 +1512,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
> * for becoming a VLAN-aware port.
> */
> mt7530_port_set_vlan_aware(ds, port);
> - mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
> + mt7530_port_set_vlan_aware(ds, priv->cpu_port);
> } else {
> mt7530_port_set_vlan_unaware(ds, port);
> }
Same.
> @@ -1526,7 +1528,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> u32 val;
>
> new_members = entry->old_members | BIT(entry->port) |
> - BIT(MT7530_CPU_PORT);
> + BIT(priv->cpu_port);
>
> /* Validate the entry with independent learning, create egress tag per
> * VLAN and joining the port as one of the port members.
> @@ -1550,8 +1552,8 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
> * DSA tag.
> */
> mt7530_rmw(priv, MT7530_VAWD2,
> - ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
> - ETAG_CTRL_P(MT7530_CPU_PORT,
> + ETAG_CTRL_P_MASK(priv->cpu_port),
> + ETAG_CTRL_P(priv->cpu_port,
> MT7530_VLAN_EGRESS_STACK));
> }
This one is interesting. For some reason, the driver author felt the
need to explicitly add BIT(MT7530_CPU_PORT) to new_members, even though
mt7530_port_vlan_add() will be called on the CPU port too
bridge vlan add dev swp0 vid 100
bridge vlan add dev br0 vid 100 self # here
I would first make a patch that leaves it up to DSA to call
port_vlan_add for the CPU port, rather than doing it implicitly.
There is a certain advantage to that too. You can do autonomous
forwarding in a certain VLAN, but not add br0 to that VLAN, and then,
you could avoid flooding the CPU with those packets, if you know that
software doesn't need to process them.
The modified function would look like this:
static void
mt7530_hw_vlan_add(struct mt7530_priv *priv,
struct mt7530_hw_vlan_entry *entry)
{
struct dsa_port *dp = dsa_to_port(priv->ds, entry->port);
u8 new_members;
u32 val;
new_members = entry->old_members | BIT(entry->port);
/* Validate the entry with independent learning, create egress tag per
* VLAN and joining the port as one of the port members.
*/
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | FID(FID_BRIDGED) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
/* Decide whether adding tag or not for those outgoing packets from the
* port inside the VLAN.
* CPU port is always taken as a tagged port for serving more than one
* VLANs across and also being applied with egress type stack mode for
* that VLAN tags would be appended after hardware special tag used as
* DSA tag.
*/
if (dsa_port_is_cpu(dp))
val = MT7530_VLAN_EGRESS_STACK;
else if (entry->untagged)
val = MT7530_VLAN_EGRESS_UNTAG;
else
val = MT7530_VLAN_EGRESS_TAG;
mt7530_rmw(priv, MT7530_VAWD2,
ETAG_CTRL_P_MASK(entry->port),
ETAG_CTRL_P(entry->port, val));
}
Then please confirm that there aren't regressions in local traffic
termination for a VLAN-aware bridge. Pinging over a bridge with
vlan_filtering=1 should be sufficient, since that is done in the default
pvid of 1.
With this change, you no longer need to concern yourself about the fixed
value of MT7530_CPU_PORT.
>
> @@ -1575,7 +1577,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
> * the entry would be kept valid. Otherwise, the entry is got to be
> * disabled.
> */
> - if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
> + if (new_members && new_members != BIT(priv->cpu_port)) {
> val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
> VLAN_VALID;
> mt7530_write(priv, MT7530_VAWD1, val);
Here the logic is again too convoluted for its own good. Let's think
before changing.
What I think was happening was this. DSA used to be super blind when
adding a VLAN to the CPU port. Since there is no netdev for the CPU
port, then what DSA used to do was to implicitly program a bridge VLAN to
the CPU port whenever that VLAN was programmed on a user port. The
downside is that you never know when you can remove that VLAN, since the
CPU port is shared between multiple user ports and refcounting was
impossible due to technical switchdev API details.
Here, the mt7530 driver tries to outsmart DSA by saying "hey, if the CPU
port is the only remaining member of the VLAN, this VLAN is garbage,
let's remove it". Which, I mean, fair point.
But since commit 134ef2388e7f ("net: dsa: add explicit support for host
bridge VLANs"), DSA does a better job of keeping track of VLANs added on
CPU ports, and it doesn't leave dangling VLANs behind. You need to trust it.
So in this case, you would have to change the code like this:
if (new_members) {
val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
VLAN_VALID;
mt7530_write(priv, MT7530_VAWD1, val);
} else {
mt7530_write(priv, MT7530_VAWD1, 0);
mt7530_write(priv, MT7530_VAWD2, 0);
}
and then delete the comment, it isn't relevant any longer.
> @@ -2105,7 +2107,7 @@ mt7530_setup(struct dsa_switch *ds)
> * controller also is the container for two GMACs nodes representing
> * as two netdev instances.
> */
> - dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
> + dn = dsa_to_port(ds, priv->cpu_port)->master->dev.of_node->parent;
Holy ####, this is convoluted. This gets us the OF node of the container
of the DSA master, alright.
Could you do something more generic? Something like this:
struct device_node *dn = NULL;
struct dsa_port *cpu_dp;
dsa_switch_for_each_cpu_port(cpu_dp, ds) {
dn = cpu_dp->master->dev.of_node->parent;
/* It doesn't matter which CPU port is found first,
* their masters should share the same parent OF node
*/
break;
}
if (!dn) {
ERROR ERROR ERROR, no CPU port
}
> ds->assisted_learning_on_cpu_port = true;
> ds->mtu_enforcement_ingress = true;
>
> @@ -2337,15 +2339,6 @@ mt7531_setup(struct dsa_switch *ds)
> mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> CORE_PLL_GROUP4, val);
>
> - /* BPDU to CPU port */
> - mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> - BIT(MT7530_CPU_PORT));
> - mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> - MT753X_BPDU_CPU_ONLY);
> -
> - /* Enable and reset MIB counters */
> - mt7530_mib_reset(ds);
> -
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
> /* Disable forwarding by default on all ports */
> mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> @@ -2373,6 +2366,15 @@ mt7531_setup(struct dsa_switch *ds)
> PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> }
>
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(priv->cpu_port));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> /* Setup VLAN ID 0 for VLAN-unaware bridges */
> ret = mt7530_setup_vlan0(priv);
> if (ret)
> @@ -3213,6 +3215,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> if (!priv)
> return -ENOMEM;
>
> + priv->cpu_port = 6;
> +
> priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
> if (!priv->ds)
> return -ENOMEM;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 91508e2feef9..62df8d10f6d4 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -8,7 +8,6 @@
>
> #define MT7530_NUM_PORTS 7
> #define MT7530_NUM_PHYS 5
> -#define MT7530_CPU_PORT 6
> #define MT7530_NUM_FDB_RECORDS 2048
> #define MT7530_ALL_MEMBERS 0xff
>
> @@ -823,6 +822,7 @@ struct mt7530_priv {
> u8 mirror_tx;
>
> struct mt7530_port ports[MT7530_NUM_PORTS];
> + int cpu_port;
And this should no longer be needed.
> /* protect among processes for registers access*/
> struct mutex reg_mutex;
> int irq;
> --
> 2.25.1
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 48+ messages in thread