Re: [net-next PATCH 09/10] net: dsa: realtek: rtl8365mb: add bridge port flags

[Luiz Angelo Daros de Luca]

> > +static int rtl8365mb_port_set_ucast_flood(struct realtek_priv *priv, int port,
> > +                                         bool enable)
> > +static int rtl8365mb_port_set_mcast_flood(struct realtek_priv *priv, int port,
> > +                                         bool enable)
> > +static int rtl8365mb_port_set_bcast_flood(struct realtek_priv *priv, int port,
> > +                                         bool enable)
>
> Those are small and just writing registers so I guess they can be kept local,
> but I think they should be accessed indirectly through
> struct realtek_ops .rtl8366_port_set_ucast_flood() etc.

Sure.

> > +static int rtl8365mb_port_bridge_flags(struct dsa_switch *ds, int port,
> > +                                      struct switchdev_brport_flags flags,
> > +                                      struct netlink_ext_ack *exack)
>
> Move this one to rtl8366-core.c under the name rtl8366_port_[pre_]bridge_flags()
> and use the abstract accessors to do the register writes.

OK

> > @@ -1691,6 +1797,8 @@ static int rtl8365mb_port_bridge_join(struct dsa_switch *ds, int port,
> >         int ret;
> >         int i;
> >
> > +       dev_dbg(priv->dev, "bridge %d join port %d\n", port, bridge.num);
> > +
> >         /* Add this port to the isolation group of every other port
> >          * offloading this bridge.
> >          */
> > @@ -1730,6 +1838,8 @@ static void rtl8365mb_port_bridge_leave(struct dsa_switch *ds, int port,
> >         u32 mask = 0;
> >         int i;
> >
> > +       dev_dbg(priv->dev, "bridge %d leave port %d\n", port, bridge.num);
> > +
> >         /* Remove this port from the isolation group of every other
> >          * port offloading this bridge.
> >          */
>
> These debug prints seem unrelated to this patch?

Yes, sorry. I'll move them.

>
> > @@ -2547,6 +2657,19 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
> >                 if (ret)
> >                         goto out_teardown_irq;
> >
> > +               /* Enable all types of flooding */
> > +               ret = rtl8365mb_port_set_ucast_flood(priv, i, true);
> > +               if (ret)
> > +                       goto out_teardown_irq;
> > +
> > +               ret = rtl8365mb_port_set_mcast_flood(priv, i, true);
> > +               if (ret)
> > +                       goto out_teardown_irq;
> > +
> > +               ret = rtl8365mb_port_set_bcast_flood(priv, i, true);
> > +               if (ret)
> > +                       goto out_teardown_irq;
>
> Create rtl8366_setup_flood_control() in realtek-core.c and add these
> three calls as a helper using the rtl8366_* generic calls there. It will be
> done the same on all chips.

OK. I think

rtl83xx_setup_port_flood_control(struct realtek_priv *priv, int port);

might be better as we can avoid looping again over the ports.

Regards,

Luiz