Re: [PATCH net-next 5/8] net: dsa: lan9645x: add bridge support

[Vladimir Oltean <olteanv@gmail.com>]

On Tue, Mar 03, 2026 at 01:22:31PM +0100, Jens Emil Schulz Østergaard wrote:
> Add support for hardware offloading of the bridge. We support a single
> bridge device.
> 
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
> ---
>  drivers/net/dsa/microchip/lan9645x/lan9645x_main.c | 196 +++++++++++++++++++++
>  drivers/net/dsa/microchip/lan9645x/lan9645x_main.h |  11 ++
>  drivers/net/dsa/microchip/lan9645x/lan9645x_port.c |   2 +
>  3 files changed, 209 insertions(+)
> 
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 739013f049d0..b6efaf669a3f 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> @@ -171,6 +171,8 @@ static int lan9645x_setup(struct dsa_switch *ds)
>  		return err;
>  	}
>  
> +	mutex_init(&lan9645x->fwd_domain_lock);
> +
>  	/* Link Aggregation Mode: NETDEV_LAG_HASH_L2 */
>  	lan_wr(ANA_AGGR_CFG_AC_SMAC_ENA |
>  	       ANA_AGGR_CFG_AC_DMAC_ENA,
> @@ -288,6 +290,192 @@ static void lan9645x_port_phylink_get_caps(struct dsa_switch *ds, int port,
>  	lan9645x_phylink_get_caps(ds->priv, port, config);
>  }
>  
> +static int lan9645x_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
> +{
> +	u32 age_secs = max(1, msecs / 1000 / 2);

s/1000/MSEC_PER_SEC/

> +	struct lan9645x *lan9645x = ds->priv;
> +
> +	/* Entry is must suffer two aging scans before it is removed, so an

"An entry must suffer (...), so it is aged"

> +	 * entry is aged after 2*AGE_PERIOD, and the unit is in seconds.
> +	 * An age period of 0 disables automatic aging.
> +	 */
> +	lan_rmw(ANA_AUTOAGE_AGE_PERIOD_SET(age_secs),
> +		ANA_AUTOAGE_AGE_PERIOD,
> +		lan9645x, ANA_AUTOAGE);
> +	return 0;
> +}
> +
> +static int lan9645x_port_pre_bridge_flags(struct dsa_switch *ds, int port,
> +					  struct switchdev_brport_flags flags,
> +					  struct netlink_ext_ack *extack)
> +{
> +	if (flags.mask &
> +	    ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static void lan9645x_port_pgid_set(struct lan9645x *lan9645x, u16 pgid,
> +				   int chip_port, bool enabled)
> +{
> +	u32 reg_msk, port_msk;
> +
> +	WARN_ON(chip_port > CPU_PORT);
> +
> +	port_msk = ANA_PGID_PGID_SET(enabled ? BIT(chip_port) : 0);
> +	reg_msk = ANA_PGID_PGID_SET(BIT(chip_port));
> +
> +	lan_rmw(port_msk, reg_msk, lan9645x, ANA_PGID(pgid));
> +}
> +
> +static void lan9645x_port_set_learning(struct lan9645x *lan9645x, int port,
> +				       bool enabled)
> +{
> +	struct lan9645x_port *p;
> +
> +	lan_rmw(ANA_PORT_CFG_LEARN_ENA_SET(enabled), ANA_PORT_CFG_LEARN_ENA,
> +		lan9645x, ANA_PORT_CFG(port));

Actually, the port may be in an STP state where learning shouldn't be
enabled, when this function is called. Enabling the "learning" bridge
port flag shouldn't change that.

> +
> +	p = lan9645x_to_port(lan9645x, port);
> +	p->learn_ena = enabled;
> +}
> +
> +static int lan9645x_port_bridge_flags(struct dsa_switch *ds, int port,
> +				      struct switchdev_brport_flags f,
> +				      struct netlink_ext_ack *extack)
> +{
> +	struct lan9645x *l = ds->priv;

Could we have some consistency in variable naming throughout the driver,
at least for the main private structure? I don't have an issue with it
being called l, it's just that I would prefer it being called the same
everywhere.

> +
> +	if (WARN_ON(port == l->npi))
> +		return -EINVAL;
> +
> +	if (f.mask & BR_LEARNING)
> +		lan9645x_port_set_learning(l, port, !!(f.val & BR_LEARNING));
> +
> +	if (f.mask & BR_FLOOD)
> +		lan9645x_port_pgid_set(l, PGID_UC, port, !!(f.val & BR_FLOOD));
> +
> +	if (f.mask & BR_MCAST_FLOOD) {
> +		bool ena = !!(f.val & BR_MCAST_FLOOD);
> +
> +		lan9645x_port_pgid_set(l, PGID_MC, port, ena);
> +		lan9645x_port_pgid_set(l, PGID_MCIPV4, port, ena);
> +		lan9645x_port_pgid_set(l, PGID_MCIPV6, port, ena);
> +	}
> +
> +	if (f.mask & BR_BCAST_FLOOD)
> +		lan9645x_port_pgid_set(l, PGID_BC, port,
> +				       !!(f.val & BR_BCAST_FLOOD));
> +
> +	return 0;
> +}
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> index 038868ae0a32..b60c64458957 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> @@ -15,6 +15,8 @@ int lan9645x_port_init(struct lan9645x *lan9645x, int port)
>  		ANA_PORT_CFG_LEARN_ENA,
>  		lan9645x, ANA_PORT_CFG(p->chip_port));
>  
> +	p->learn_ena = false;
> +

This is already zero-initialized memory.

>  	lan9645x_port_set_maxlen(lan9645x, port, ETH_DATA_LEN);
>  
>  	lan9645x_phylink_port_down(lan9645x, port);
> 
> -- 
> 2.52.0
>