drivers/net/dsa/mt7530.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
From: Richard van Schagen <richard@routerhints.com>
Add support for changing the master of a port on the MT7530 DSA subdriver.
[ arinc.unal@arinc9.com: Wrote subject and changelog ]
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Richard van Schagen <richard@routerhints.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
drivers/net/dsa/mt7530.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b5ad4b4fc00c..04bb4986454e 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1072,6 +1072,38 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
mutex_unlock(&priv->reg_mutex);
}
+static int
+mt7530_port_change_master(struct dsa_switch *ds, int port,
+ struct net_device *master,
+ struct netlink_ext_ack *extack)
+{
+ struct mt7530_priv *priv = ds->priv;
+ struct dsa_port *dp = dsa_to_port(ds, port);
+ struct dsa_port *cpu_dp = master->dsa_ptr;
+ int old_cpu = dp->cpu_dp->index;
+ int new_cpu = cpu_dp->index;
+
+ mutex_lock(&priv->reg_mutex);
+
+ /* Move old to new cpu on User port */
+ priv->ports[port].pm &= ~PCR_MATRIX(BIT(old_cpu));
+ priv->ports[port].pm |= PCR_MATRIX(BIT(new_cpu));
+
+ mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
+ priv->ports[port].pm);
+
+ /* Move user port from old cpu to new cpu */
+ priv->ports[old_cpu].pm &= ~PCR_MATRIX(BIT(port));
+ priv->ports[new_cpu].pm |= PCR_MATRIX(BIT(port));
+
+ mt7530_write(priv, MT7530_PCR_P(old_cpu), priv->ports[old_cpu].pm);
+ mt7530_write(priv, MT7530_PCR_P(new_cpu), priv->ports[new_cpu].pm);
+
+ mutex_unlock(&priv->reg_mutex);
+
+ return 0;
+}
+
static int
mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
{
@@ -3157,6 +3189,7 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
.set_ageing_time = mt7530_set_ageing_time,
.port_enable = mt7530_port_enable,
.port_disable = mt7530_port_disable,
+ .port_change_master = mt7530_port_change_master,
.port_change_mtu = mt7530_port_change_mtu,
.port_max_mtu = mt7530_port_max_mtu,
.port_stp_state_set = mt7530_stp_state_set,
--
2.37.2
On Fri, Feb 10, 2023 at 08:29:43PM +0300, arinc9.unal@gmail.com wrote: > From: Richard van Schagen <richard@routerhints.com> > > Add support for changing the master of a port on the MT7530 DSA subdriver. > > [ arinc.unal@arinc9.com: Wrote subject and changelog ] > > Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com> > Signed-off-by: Richard van Schagen <richard@routerhints.com> > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- > drivers/net/dsa/mt7530.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index b5ad4b4fc00c..04bb4986454e 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -1072,6 +1072,38 @@ mt7530_port_disable(struct dsa_switch *ds, int port) > mutex_unlock(&priv->reg_mutex); > } > > +static int > +mt7530_port_change_master(struct dsa_switch *ds, int port, > + struct net_device *master, > + struct netlink_ext_ack *extack) alignment > +{ > + struct mt7530_priv *priv = ds->priv; > + struct dsa_port *dp = dsa_to_port(ds, port); > + struct dsa_port *cpu_dp = master->dsa_ptr; > + int old_cpu = dp->cpu_dp->index; > + int new_cpu = cpu_dp->index; I believe you need to reject LAG DSA masters. > + > + mutex_lock(&priv->reg_mutex); > + > + /* Move old to new cpu on User port */ > + priv->ports[port].pm &= ~PCR_MATRIX(BIT(old_cpu)); > + priv->ports[port].pm |= PCR_MATRIX(BIT(new_cpu)); > + > + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK, > + priv->ports[port].pm); > + > + /* Move user port from old cpu to new cpu */ > + priv->ports[old_cpu].pm &= ~PCR_MATRIX(BIT(port)); > + priv->ports[new_cpu].pm |= PCR_MATRIX(BIT(port)); > + > + mt7530_write(priv, MT7530_PCR_P(old_cpu), priv->ports[old_cpu].pm); > + mt7530_write(priv, MT7530_PCR_P(new_cpu), priv->ports[new_cpu].pm); - who writes to the "pm" field of CPU ports? - how does this line up with your other patch which said (AFAIU) that the port matrix of CPU ports should be 0 and that should be fine? - read/modify/write (rmw) using PCR_MATRIX_MASK rather than mt7530_write(). That overwrites the other PCR fields. > + > + mutex_unlock(&priv->reg_mutex); > + > + return 0; > +} > + > static int > mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) > { > @@ -3157,6 +3189,7 @@ static const struct dsa_switch_ops mt7530_switch_ops = { > .set_ageing_time = mt7530_set_ageing_time, > .port_enable = mt7530_port_enable, > .port_disable = mt7530_port_disable, > + .port_change_master = mt7530_port_change_master, > .port_change_mtu = mt7530_port_change_mtu, > .port_max_mtu = mt7530_port_max_mtu, > .port_stp_state_set = mt7530_stp_state_set, > -- > 2.37.2 >
> On 10 Feb 2023, at 19:56, Vladimir Oltean <olteanv@gmail.com> wrote: > > On Fri, Feb 10, 2023 at 08:29:43PM +0300, arinc9.unal@gmail.com wrote: >> From: Richard van Schagen <richard@routerhints.com> >> >> Add support for changing the master of a port on the MT7530 DSA subdriver. >> >> [ arinc.unal@arinc9.com: Wrote subject and changelog ] >> >> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com> >> Signed-off-by: Richard van Schagen <richard@routerhints.com> >> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >> --- >> drivers/net/dsa/mt7530.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c >> index b5ad4b4fc00c..04bb4986454e 100644 >> --- a/drivers/net/dsa/mt7530.c >> +++ b/drivers/net/dsa/mt7530.c >> @@ -1072,6 +1072,38 @@ mt7530_port_disable(struct dsa_switch *ds, int port) >> mutex_unlock(&priv->reg_mutex); >> } >> >> +static int >> +mt7530_port_change_master(struct dsa_switch *ds, int port, >> + struct net_device *master, >> + struct netlink_ext_ack *extack) > > alignment > Will fix >> +{ >> + struct mt7530_priv *priv = ds->priv; >> + struct dsa_port *dp = dsa_to_port(ds, port); >> + struct dsa_port *cpu_dp = master->dsa_ptr; >> + int old_cpu = dp->cpu_dp->index; >> + int new_cpu = cpu_dp->index; > > I believe you need to reject LAG DSA masters. > Not sure what you mean: how is this different from the change_master in the Felix driver when using 8021q tags? But. Can add a check if you prefer. It might be a good idea anyway to be future proof. The MT7531 has support for LAG in hw. >> + >> + mutex_lock(&priv->reg_mutex); >> + >> + /* Move old to new cpu on User port */ >> + priv->ports[port].pm &= ~PCR_MATRIX(BIT(old_cpu)); >> + priv->ports[port].pm |= PCR_MATRIX(BIT(new_cpu)); >> + >> + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK, >> + priv->ports[port].pm); >> + >> + /* Move user port from old cpu to new cpu */ >> + priv->ports[old_cpu].pm &= ~PCR_MATRIX(BIT(port)); >> + priv->ports[new_cpu].pm |= PCR_MATRIX(BIT(port)); >> + >> + mt7530_write(priv, MT7530_PCR_P(old_cpu), priv->ports[old_cpu].pm); >> + mt7530_write(priv, MT7530_PCR_P(new_cpu), priv->ports[new_cpu].pm); > > - who writes to the "pm" field of CPU ports? Nobody actually writes to cpu.pm. I “fixed” that, and dropped that later. This is left over. > - how does this line up with your other patch which said (AFAIU) that > the port matrix of CPU ports should be 0 and that should be fine? Since cpu.pm was never user, and all user ports were added without using this field when enabling the cpu, I changed that to add user ports belonging to that CPU. Arinc reported that it didn’t work. Since the cpu.pm (empty) is writing during dsa_enable_port and that worked (for a long time already) the cpu.pm can be dropped. > - read/modify/write (rmw) using PCR_MATRIX_MASK rather than mt7530_write(). > That overwrites the other PCR fields. > Good catch. Will fix. >> + >> + mutex_unlock(&priv->reg_mutex); >> + >> + return 0; >> +} >> + >> static int >> mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) >> { >> @@ -3157,6 +3189,7 @@ static const struct dsa_switch_ops mt7530_switch_ops = { >> .set_ageing_time = mt7530_set_ageing_time, >> .port_enable = mt7530_port_enable, >> .port_disable = mt7530_port_disable, >> + .port_change_master = mt7530_port_change_master, >> .port_change_mtu = mt7530_port_change_mtu, >> .port_max_mtu = mt7530_port_max_mtu, >> .port_stp_state_set = mt7530_stp_state_set, >> -- >> 2.37.2
On Fri, Feb 10, 2023 at 09:41:06PM +0000, Richard van Schagen wrote: > > I believe you need to reject LAG DSA masters. > > Not sure what you mean: how is this different from the change_master in the Felix driver when using 8021q tags? > But. Can add a check if you prefer. It might be a good idea anyway to be future proof. The MT7531 has support for LAG in hw. I mean, like Documentation/networking/dsa/configuration.rst says, that the user can attempt to put the DSA masters in a LAG and create a larger DSA master which is that bonding device. The difference from the Felix driver is that Felix supports LAG DSA masters and this driver doesn't. I don't believe there is any other restriction in the code which would prevent a driver which implements port_change_master() from accepting that as a valid configuration, so it's going to be the mt7530 driver who acts as the final frontier in this case. An "if (netif_is_lag_master(master)) return -EOPNOTSUPP" will do. But it would always be good to check if it's really needed :)
© 2016 - 2024 Red Hat, Inc.