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

[Jens Emil Schulz Ostergaard <jensemil.schulzostergaard@microchip.com>]

On Tue, 2026-03-03 at 16:51 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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/
> 

I will change this.

> > +     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"

I will change this.

> 
> > +      * 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.
> 

Thank you, I will check stp_state before writing to HW, similar to 
set_stp_state.

> > +
> > +     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.
> 

I will use lan9645x 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.

Removed.

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



Thanks,
Emil