| Message ID | 9e5682c68a4930dae2e277b9cecc8b8ec97ba2af.1718899575.git.mschiffer@universe-factory.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e85d3e6fea05c8ae21a40809a3c6b7adc97411c7 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: qca8k: cleanup and port isolation | expand |
On 20.06.2024 19:25, Matthias Schiffer wrote: > qca8k_port_bridge_join() set QCA8K_PORT_LOOKUP_CTRL() for i == port twice, > once in the loop handling all other port's masks, and finally at the end > with the accumulated port_mask. > > The first time it would incorrectly set the port's own bit in the mask, > only to correct the mistake a moment later. qca8k_port_bridge_leave() had > the same issue, but here the regmap_clear_bits() was a no-op rather than > setting an unintended value. > > Remove the duplicate assignment by skipping the whole loop iteration for > i == port. The unintended bit setting doesn't seem to have any negative > effects (even when not reverted right away), so the change is submitted > as a simple cleanup rather than a fix. > > Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> > --- Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > drivers/net/dsa/qca/qca8k-common.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c > index 7f80035c5441..b33df84070d3 100644 > --- a/drivers/net/dsa/qca/qca8k-common.c > +++ b/drivers/net/dsa/qca/qca8k-common.c > @@ -653,6 +653,8 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port, > port_mask = BIT(cpu_port); > > for (i = 0; i < QCA8K_NUM_PORTS; i++) { > + if (i == port) > + continue; > if (dsa_is_cpu_port(ds, i)) > continue; > if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge)) > @@ -665,8 +667,7 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port, > BIT(port)); > if (ret) > return ret; > - if (i != port) > - port_mask |= BIT(i); > + port_mask |= BIT(i); > } > > /* Add all other ports to this ports portvlan mask */ > @@ -685,6 +686,8 @@ void qca8k_port_bridge_leave(struct dsa_switch *ds, int port, > cpu_port = dsa_to_port(ds, port)->cpu_dp->index; > > for (i = 0; i < QCA8K_NUM_PORTS; i++) { > + if (i == port) > + continue; > if (dsa_is_cpu_port(ds, i)) > continue; > if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c index 7f80035c5441..b33df84070d3 100644 --- a/drivers/net/dsa/qca/qca8k-common.c +++ b/drivers/net/dsa/qca/qca8k-common.c @@ -653,6 +653,8 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port, port_mask = BIT(cpu_port); for (i = 0; i < QCA8K_NUM_PORTS; i++) { + if (i == port) + continue; if (dsa_is_cpu_port(ds, i)) continue; if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge)) @@ -665,8 +667,7 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port, BIT(port)); if (ret) return ret; - if (i != port) - port_mask |= BIT(i); + port_mask |= BIT(i); } /* Add all other ports to this ports portvlan mask */ @@ -685,6 +686,8 @@ void qca8k_port_bridge_leave(struct dsa_switch *ds, int port, cpu_port = dsa_to_port(ds, port)->cpu_dp->index; for (i = 0; i < QCA8K_NUM_PORTS; i++) { + if (i == port) + continue; if (dsa_is_cpu_port(ds, i)) continue; if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
qca8k_port_bridge_join() set QCA8K_PORT_LOOKUP_CTRL() for i == port twice, once in the loop handling all other port's masks, and finally at the end with the accumulated port_mask. The first time it would incorrectly set the port's own bit in the mask, only to correct the mistake a moment later. qca8k_port_bridge_leave() had the same issue, but here the regmap_clear_bits() was a no-op rather than setting an unintended value. Remove the duplicate assignment by skipping the whole loop iteration for i == port. The unintended bit setting doesn't seem to have any negative effects (even when not reverted right away), so the change is submitted as a simple cleanup rather than a fix. Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> --- drivers/net/dsa/qca/qca8k-common.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)