| Message ID | 20220426134924.30372-3-linux@fw-web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add MT7531 switch to BPI-R2Pro Board | expand |
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.
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
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.
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 >
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;