Le Wed, 4 May 2022 19:14:14 +0300, Vladimir Oltean <olteanv@xxxxxxxxx> a écrit : > On Wed, May 04, 2022 at 11:29:54AM +0200, Clément Léger wrote: > > Add Renesas RZ/N1 advanced 5 port switch driver. This switch handles 5 > > ports including 1 CPU management port. A MDIO bus is also exposed by > > this switch and allows to communicate with PHYs connected to the ports. > > Each switch port (except for the CPU management ports) is connected to > > the MII converter. > > > > This driver includes basic bridging support, more support will be added > > later (vlan, etc). > > > > Suggested-by: Jean-Pierre Geslin <jean-pierre.geslin@xxxxxxxxxx> > > Suggested-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx> > > Signed-off-by: Clément Léger <clement.leger@xxxxxxxxxxx> > > --- > > +static void a5psw_port_disable(struct dsa_switch *ds, int port) > > +{ > > + struct a5psw *a5psw = ds->priv; > > + > > + a5psw_port_authorize_set(a5psw, port, false); > > + a5psw_port_enable_set(a5psw, port, false); > > + a5psw_port_fdb_flush(a5psw, port); > > The bridge core takes care of this by setting the port state to > DISABLED, which makes DSA call dsa_port_fast_age(), no? Yes you are right. > > Standalone ports shouldn't need fast ageing because they shouldn't have > address learning enabled in the first place. Ok, makes sense. > > > +} > > > +static int a5psw_port_bridge_join(struct dsa_switch *ds, int port, > > + struct dsa_bridge bridge, > > + bool *tx_fwd_offload, > > + struct netlink_ext_ack *extack) > > +{ > > + struct a5psw *a5psw = ds->priv; > > + > > + /* We only support 1 bridge device */ > > + if (a5psw->br_dev && bridge.dev != a5psw->br_dev) > > + return -EINVAL; > > return -EOPNOTSUPP, to allow software bridging. Ok. > You might also want to set an extack message here and avoid overwriting > it in dsa_slave_changeupper() with "Offloading not supported", but say > something more specific like "Forwarding offload supported for a single > bridge". > > > + a5psw->br_dev = NULL; > > +} > > > +static int a5psw_pcs_get(struct a5psw *a5psw) > > +{ > > + struct device_node *ports, *port, *pcs_node; > > + struct phylink_pcs *pcs; > > + int ret; > > + u32 reg; > > + > > + ports = of_get_child_by_name(a5psw->dev->of_node, "ports"); > > Can you please do: > > ports = of_get_child_by_name(a5psw->dev->of_node, "ethernet-ports"); > if (!ports) > ports = of_get_child_by_name(a5psw->dev->of_node, "ports"); Acked. -- Clément Léger, Embedded Linux and Kernel engineer at Bootlin https://bootlin.com