[RFC,net-next,09/11] net: dsa: microchip: lan937x: update port membership with dsa port
Commit Message
Like cpu port, cascaded port will act as host port in second switch. And
all ports from both switches should be able to forward packets to cascaded
ports. Add cascaded port (dev->dsa_port) to each port membership.
Current design add bit map of user ports as cpu port membership. Include
cascaded port index as well to this group.
Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
drivers/net/dsa/microchip/ksz_common.c | 7 ++++---
drivers/net/dsa/microchip/lan937x_main.c | 2 +-
include/net/dsa.h | 15 +++++++++++++++
3 files changed, 20 insertions(+), 4 deletions(-)
Comments
On Thu, Feb 02, 2023 at 06:29:28PM +0530, Rakesh Sankaranarayanan wrote:
> Like cpu port, cascaded port will act as host port in second switch. And
> all ports from both switches should be able to forward packets to cascaded
> ports. Add cascaded port (dev->dsa_port) to each port membership.
>
> Current design add bit map of user ports as cpu port membership. Include
> cascaded port index as well to this group.
>
> Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
> ---
> drivers/net/dsa/microchip/ksz_common.c | 7 ++++---
> drivers/net/dsa/microchip/lan937x_main.c | 2 +-
> include/net/dsa.h | 15 +++++++++++++++
> 3 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 913296c5dd50..b8b7b5b7b52d 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1748,9 +1748,9 @@ static void ksz_get_strings(struct dsa_switch *ds, int port,
>
> static void ksz_update_port_member(struct ksz_device *dev, int port)
> {
> + u8 port_member = 0, cpu_port, dsa_port;
Don't need these unimaginative names ("cpu_port", "dsa_port" for what is
actually a port *mask*). You can bitwise-OR the upstream port and the
downstream cascade port directly into "port_member", and the code in
ksz_update_port_member() would tolerate it just fine.
> struct ksz_port *p = &dev->ports[port];
> struct dsa_switch *ds = dev->ds;
> - u8 port_member = 0, cpu_port;
> const struct dsa_port *dp;
> int i, j;
>
> @@ -1759,6 +1759,7 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
>
> dp = dsa_to_port(ds, port);
> cpu_port = BIT(dsa_upstream_port(ds, port));
> + dsa_port = BIT(dev->dsa_port);
If dev->dsa_port is 0xff, what is BIT(0xff)?
>
> for (i = 0; i < ds->num_ports; i++) {
> const struct dsa_port *other_dp = dsa_to_port(ds, i);
> @@ -1798,10 +1799,10 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
> val |= BIT(j);
> }
>
> - dev->dev_ops->cfg_port_member(dev, i, val | cpu_port);
> + dev->dev_ops->cfg_port_member(dev, i, val | cpu_port | dsa_port);
> }
>
> - dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port);
> + dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port | dsa_port);
> }
>
> static int ksz_sw_mdio_read(struct mii_bus *bus, int addr, int regnum)
> diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
> index 5108a3f4bf76..b17bb1ea2a4a 100644
> --- a/drivers/net/dsa/microchip/lan937x_main.c
> +++ b/drivers/net/dsa/microchip/lan937x_main.c
> @@ -198,7 +198,7 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
> true);
>
> if (cpu_port)
> - member = dsa_user_ports(ds);
> + member = dsa_user_ports(ds) | dsa_dsa_ports(ds);
I'm wondering if a single dsa_switch_for_each_port() list traversal plus
an "if ()" wouldn't be more efficient here.
> else
> member = BIT(dsa_upstream_port(ds, port));
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 55651ad29193..939aa6ff1a38 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -591,6 +591,10 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
> dsa_switch_for_each_port((_dp), (_ds)) \
> if (dsa_port_is_cpu((_dp)))
>
> +#define dsa_switch_for_each_dsa_port(_dp, _ds) \
> + dsa_switch_for_each_port((_dp), (_ds)) \
> + if (dsa_port_is_dsa((_dp)))
> +
> #define dsa_switch_for_each_cpu_port_continue_reverse(_dp, _ds) \
> dsa_switch_for_each_port_continue_reverse((_dp), (_ds)) \
> if (dsa_port_is_cpu((_dp)))
> @@ -617,6 +621,17 @@ static inline u32 dsa_cpu_ports(struct dsa_switch *ds)
> return mask;
> }
>
> +static inline u32 dsa_dsa_ports(struct dsa_switch *ds)
> +{
> + struct dsa_port *dsa_dp;
can name this just "dp"
> + u32 mask = 0;
> +
> + dsa_switch_for_each_dsa_port(dsa_dp, ds)
> + mask |= BIT(dsa_dp->index);
> +
> + return mask;
> +}
> +
> /* Return the local port used to reach an arbitrary switch device */
> static inline unsigned int dsa_routing_port(struct dsa_switch *ds, int device)
> {
> --
> 2.34.1
>
@@ -1748,9 +1748,9 @@ static void ksz_get_strings(struct dsa_switch *ds, int port,
static void ksz_update_port_member(struct ksz_device *dev, int port)
{
+ u8 port_member = 0, cpu_port, dsa_port;
struct ksz_port *p = &dev->ports[port];
struct dsa_switch *ds = dev->ds;
- u8 port_member = 0, cpu_port;
const struct dsa_port *dp;
int i, j;
@@ -1759,6 +1759,7 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
dp = dsa_to_port(ds, port);
cpu_port = BIT(dsa_upstream_port(ds, port));
+ dsa_port = BIT(dev->dsa_port);
for (i = 0; i < ds->num_ports; i++) {
const struct dsa_port *other_dp = dsa_to_port(ds, i);
@@ -1798,10 +1799,10 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
val |= BIT(j);
}
- dev->dev_ops->cfg_port_member(dev, i, val | cpu_port);
+ dev->dev_ops->cfg_port_member(dev, i, val | cpu_port | dsa_port);
}
- dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port);
+ dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port | dsa_port);
}
static int ksz_sw_mdio_read(struct mii_bus *bus, int addr, int regnum)
@@ -198,7 +198,7 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
true);
if (cpu_port)
- member = dsa_user_ports(ds);
+ member = dsa_user_ports(ds) | dsa_dsa_ports(ds);
else
member = BIT(dsa_upstream_port(ds, port));
@@ -591,6 +591,10 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
dsa_switch_for_each_port((_dp), (_ds)) \
if (dsa_port_is_cpu((_dp)))
+#define dsa_switch_for_each_dsa_port(_dp, _ds) \
+ dsa_switch_for_each_port((_dp), (_ds)) \
+ if (dsa_port_is_dsa((_dp)))
+
#define dsa_switch_for_each_cpu_port_continue_reverse(_dp, _ds) \
dsa_switch_for_each_port_continue_reverse((_dp), (_ds)) \
if (dsa_port_is_cpu((_dp)))
@@ -617,6 +621,17 @@ static inline u32 dsa_cpu_ports(struct dsa_switch *ds)
return mask;
}
+static inline u32 dsa_dsa_ports(struct dsa_switch *ds)
+{
+ struct dsa_port *dsa_dp;
+ u32 mask = 0;
+
+ dsa_switch_for_each_dsa_port(dsa_dp, ds)
+ mask |= BIT(dsa_dp->index);
+
+ return mask;
+}
+
/* Return the local port used to reach an arbitrary switch device */
static inline unsigned int dsa_routing_port(struct dsa_switch *ds, int device)
{