Re: [PATCH net-next v1 7/9] net: dsa: qca: ar9331: add bridge support

2021-04-03 Thread Florian Fainelli




On 4/3/2021 04:48, Oleksij Rempel wrote:

This switch is providing forwarding matrix, with it we can configure
individual bridges. Potentially we can configure more then one not VLAN


s/then/than/


based bridge on this HW.

Signed-off-by: Oleksij Rempel 
---
  drivers/net/dsa/qca/ar9331.c | 73 
  1 file changed, 73 insertions(+)

diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index b2c22ba924f0..bf9588574205 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -40,6 +40,7 @@
   */
  
  #include 

+#include 
  #include 
  #include 
  #include 
@@ -1134,6 +1135,76 @@ static int ar9331_sw_set_ageing_time(struct dsa_switch 
*ds,
  val);
  }
  
+static int ar9331_sw_port_bridge_join(struct dsa_switch *ds, int port,

+ struct net_device *br)
+{
+   struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+   struct regmap *regmap = priv->regmap;
+   int port_mask = BIT(priv->cpu_port);
+   int i, ret;
+   u32 val;
+
+   for (i = 0; i < ds->num_ports; i++) {
+   if (dsa_to_port(ds, i)->bridge_dev != br)
+   continue;
+
+   if (!dsa_is_user_port(ds, port))
+   continue;
+
+   val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, 
BIT(port));
+   ret = regmap_set_bits(regmap, AR9331_SW_REG_PORT_VLAN(i), val);
+   if (ret)
+   goto error;
+
+   if (i != port)
+   port_mask |= BIT(i);
+   }
+
+   val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, port_mask);
+   ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
+AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, val);
+   if (ret)
+   goto error;
+
+   return 0;
+error:
+   dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);


This is not called more than once per port and per bridge join operation 
so I would drop the rate limiting here. With that fixed:


Reviewed-by: Florian Fainelli 
--
Florian


Re: [PATCH net-next v1 7/9] net: dsa: qca: ar9331: add bridge support

2021-04-03 Thread Andrew Lunn
On Sat, Apr 03, 2021 at 01:48:46PM +0200, Oleksij Rempel wrote:
> This switch is providing forwarding matrix, with it we can configure
> individual bridges. Potentially we can configure more then one not VLAN
> based bridge on this HW.
> 
> Signed-off-by: Oleksij Rempel 
> ---
>  drivers/net/dsa/qca/ar9331.c | 73 
>  1 file changed, 73 insertions(+)
> 
> diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
> index b2c22ba924f0..bf9588574205 100644
> --- a/drivers/net/dsa/qca/ar9331.c
> +++ b/drivers/net/dsa/qca/ar9331.c
> @@ -40,6 +40,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1134,6 +1135,76 @@ static int ar9331_sw_set_ageing_time(struct dsa_switch 
> *ds,
> val);
>  }
>  
> +static int ar9331_sw_port_bridge_join(struct dsa_switch *ds, int port,
> +   struct net_device *br)
> +{
> + struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
> + struct regmap *regmap = priv->regmap;
> + int port_mask = BIT(priv->cpu_port);
> + int i, ret;
> + u32 val;
> +
> + for (i = 0; i < ds->num_ports; i++) {
> + if (dsa_to_port(ds, i)->bridge_dev != br)
> + continue;
> +
> + if (!dsa_is_user_port(ds, port))
> + continue;
> +
> + val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, 
> BIT(port));
> + ret = regmap_set_bits(regmap, AR9331_SW_REG_PORT_VLAN(i), val);
> + if (ret)
> + goto error;
> +
> + if (i != port)
> + port_mask |= BIT(i);
> + }
> +
> + val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, port_mask);
> + ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
> +  AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, val);
> + if (ret)
> + goto error;
> +
> + return 0;
> +error:
> + dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);
> +
> + return ret;
> +}
> +
> +static void ar9331_sw_port_bridge_leave(struct dsa_switch *ds, int port,
> + struct net_device *br)
> +{
> + struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
> + struct regmap *regmap = priv->regmap;
> + int i, ret;
> + u32 val;
> +
> + for (i = 0; i < ds->num_ports; i++) {
> + if (dsa_to_port(ds, i)->bridge_dev != br)
> + continue;
> +
> + if (!dsa_is_user_port(ds, port))
> + continue;
> +
> + val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, 
> BIT(port));
> + ret = regmap_clear_bits(regmap, AR9331_SW_REG_PORT_VLAN(i), 
> val);
> + if (ret)
> + goto error;
> + }

Join and leave only seems to differ by:

> + if (i != port)
> + port_mask |= BIT(i);

Maybe refactor the code to add a helper for the identical parts?

  Andrew


[PATCH net-next v1 7/9] net: dsa: qca: ar9331: add bridge support

2021-04-03 Thread Oleksij Rempel
This switch is providing forwarding matrix, with it we can configure
individual bridges. Potentially we can configure more then one not VLAN
based bridge on this HW.

Signed-off-by: Oleksij Rempel 
---
 drivers/net/dsa/qca/ar9331.c | 73 
 1 file changed, 73 insertions(+)

diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index b2c22ba924f0..bf9588574205 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -40,6 +40,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1134,6 +1135,76 @@ static int ar9331_sw_set_ageing_time(struct dsa_switch 
*ds,
  val);
 }
 
+static int ar9331_sw_port_bridge_join(struct dsa_switch *ds, int port,
+ struct net_device *br)
+{
+   struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+   struct regmap *regmap = priv->regmap;
+   int port_mask = BIT(priv->cpu_port);
+   int i, ret;
+   u32 val;
+
+   for (i = 0; i < ds->num_ports; i++) {
+   if (dsa_to_port(ds, i)->bridge_dev != br)
+   continue;
+
+   if (!dsa_is_user_port(ds, port))
+   continue;
+
+   val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, 
BIT(port));
+   ret = regmap_set_bits(regmap, AR9331_SW_REG_PORT_VLAN(i), val);
+   if (ret)
+   goto error;
+
+   if (i != port)
+   port_mask |= BIT(i);
+   }
+
+   val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, port_mask);
+   ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
+AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, val);
+   if (ret)
+   goto error;
+
+   return 0;
+error:
+   dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);
+
+   return ret;
+}
+
+static void ar9331_sw_port_bridge_leave(struct dsa_switch *ds, int port,
+   struct net_device *br)
+{
+   struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+   struct regmap *regmap = priv->regmap;
+   int i, ret;
+   u32 val;
+
+   for (i = 0; i < ds->num_ports; i++) {
+   if (dsa_to_port(ds, i)->bridge_dev != br)
+   continue;
+
+   if (!dsa_is_user_port(ds, port))
+   continue;
+
+   val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, 
BIT(port));
+   ret = regmap_clear_bits(regmap, AR9331_SW_REG_PORT_VLAN(i), 
val);
+   if (ret)
+   goto error;
+   }
+
+   val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, 
BIT(priv->cpu_port));
+   ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_VLAN(port),
+AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, val);
+   if (ret)
+   goto error;
+
+   return;
+error:
+   dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);
+}
+
 static const struct dsa_switch_ops ar9331_sw_ops = {
.get_tag_protocol   = ar9331_sw_get_tag_protocol,
.setup  = ar9331_sw_setup,
@@ -1150,6 +1221,8 @@ static const struct dsa_switch_ops ar9331_sw_ops = {
.port_mdb_add   = ar9331_sw_port_mdb_add,
.port_mdb_del   = ar9331_sw_port_mdb_del,
.set_ageing_time= ar9331_sw_set_ageing_time,
+   .port_bridge_join   = ar9331_sw_port_bridge_join,
+   .port_bridge_leave  = ar9331_sw_port_bridge_leave,
 };
 
 static irqreturn_t ar9331_sw_irq(int irq, void *data)
-- 
2.29.2