Subject: Re: [PATCH RFC 2/2] net: dsa: bcm_sf2: implement HW bridging operations

From: Florian Fainelli <[email-masked]>
Date: Wed, 18 Feb 2015 18:48:19 -0800

On 17/02/15 11:26, Florian Fainelli wrote:
> Update the Broadcom Starfighter 2 switch driver to implement the
> join/leave/stp_update callbacks required for basic hardware bridging
> support.
> 
> There is not much to be done at the driver level but translating the
> STP state from Linux to their HW values.
> 
> Joining a bridge means that the joining port and the other port members
> need to be in the same VLAN membership as the CPU, while leaving the
> bridge puts the port back into a separate VLAN membership with only the
> CPU.

I found a couple additional issues while testing:

- manipulating UP/DOWN state of interfaces that are part of a bridge
would not restore their bridge membership

- removing an interface from a bridge and bringing it back up would
leave it in blocked state

> 
> Signed-off-by: Florian Fainelli <[email-masked]>
> ---
>  drivers/net/dsa/bcm_sf2.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
> index 4daffb284931..006c86a4df54 100644
> --- a/drivers/net/dsa/bcm_sf2.c
> +++ b/drivers/net/dsa/bcm_sf2.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_address.h>
>  #include <net/dsa.h>
>  #include <linux/ethtool.h>
> +#include <linux/if_bridge.h>
>  
>  #include "bcm_sf2.h"
>  #include "bcm_sf2_regs.h"
> @@ -400,6 +401,95 @@ static int bcm_sf2_sw_set_eee(struct dsa_switch *ds, int port,
>  	return 0;
>  }
>  
> +static int bcm_sf2_sw_br_join(struct dsa_switch *ds, int port,
> +			      u32 br_port_mask)
> +{
> +	struct bcm_sf2_priv *priv = ds_to_priv(ds);
> +	unsigned int i;
> +	u32 reg, p_ctl;
> +
> +	p_ctl = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(port));
> +
> +	for (i = 0; i < priv->hw_params.num_ports; i++) {
> +		if (!((1 << i) & br_port_mask))
> +			continue;
> +
> +		reg = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(i));
> +		reg |= 1 << port;
> +		core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(i));
> +
> +		p_ctl |= 1 << i;
> +	}
> +
> +	core_writel(priv, p_ctl, CORE_PORT_VLAN_CTL_PORT(port));
> +
> +	return 0;
> +}
> +
> +static int bcm_sf2_sw_br_leave(struct dsa_switch *ds, int port,
> +			       u32 br_port_mask)
> +{
> +	struct bcm_sf2_priv *priv = ds_to_priv(ds);
> +	unsigned int i;
> +	u32 reg, p_ctl;
> +
> +	p_ctl = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(port));
> +
> +	for (i = 0; i < priv->hw_params.num_ports; i++) {
> +		/* Don't touch the remaining ports */
> +		if (!((1 << i) & br_port_mask))
> +			continue;
> +
> +		reg = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(i));
> +		reg &= ~(1 << port);
> +		core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(i));
> +
> +		/* Prevent self removal to preserve isolation */
> +		if (port != i)
> +			p_ctl &= ~(1 << i);
> +	}
> +
> +	core_writel(priv, p_ctl, CORE_PORT_VLAN_CTL_PORT(port));
> +
> +	return 0;
> +}
> +
> +static int bcm_sf2_sw_br_stp_update(struct dsa_switch *ds, int port,
> +				    u8 state)
> +{
> +	struct bcm_sf2_priv *priv = ds_to_priv(ds);
> +	u32 reg;
> +	u8 hw_state;
> +
> +	switch (state) {
> +	case BR_STATE_DISABLED:
> +		hw_state = G_MISTP_DIS_STATE;
> +		break;
> +	case BR_STATE_LISTENING:
> +		hw_state = G_MISTP_LISTEN_STATE;
> +		break;
> +	case BR_STATE_LEARNING:
> +		hw_state = G_MISTP_LEARN_STATE;
> +		break;
> +	case BR_STATE_FORWARDING:
> +		hw_state = G_MISTP_FWD_STATE;
> +		break;
> +	case BR_STATE_BLOCKING:
> +		hw_state = G_MISTP_BLOCK_STATE;
> +		break;
> +	default:
> +		pr_err("%s: invalid STP state: %d\n", __func__, state);
> +		return -EINVAL;
> +	}
> +
> +	reg = core_readl(priv, CORE_G_PCTL_PORT(port));
> +	reg &= ~(G_MISTP_STATE_MASK << G_MISTP_STATE_SHIFT);
> +	reg |= hw_state;
> +	core_writel(priv, reg, CORE_G_PCTL_PORT(port));
> +
> +	return 0;
> +}
> +
>  static irqreturn_t bcm_sf2_switch_0_isr(int irq, void *dev_id)
>  {
>  	struct bcm_sf2_priv *priv = dev_id;
> @@ -916,6 +1006,9 @@ static struct dsa_switch_driver bcm_sf2_switch_driver = {
>  	.port_disable		= bcm_sf2_port_disable,
>  	.get_eee		= bcm_sf2_sw_get_eee,
>  	.set_eee		= bcm_sf2_sw_set_eee,
> +	.port_join_bridge	= bcm_sf2_sw_br_join,
> +	.port_leave_bridge	= bcm_sf2_sw_br_leave,
> +	.port_stp_update	= bcm_sf2_sw_br_stp_update,
>  };
>  
>  static int __init bcm_sf2_init(void)
> 

-- 
Florian