RE: [PATCH net-next 06/13] net: renesas: rswitch: add forwarding rules for gwca

[Michael Dege]

Hello Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Tuesday, March 17, 2026 5:15 PM
> To: Michael Dege <michael.dege@xxxxxxxxxxx>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>; David
> S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>;
> Paolo Abeni <pabeni@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-renesas-soc@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next 06/13] net: renesas: rswitch: add forwarding rules for gwca
> 
> Hi Michael,
> 
> On Tue, 17 Mar 2026 at 10:45, Michael Dege <michael.dege@xxxxxxxxxxx> wrote:
> > Add rules to forward packets from the Ethernet ports to the CPU port
> > (GWCA) using L2 forwarding instead of port forwarding.
> >
> > Signed-off-by: Michael Dege <michael.dege@xxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> > +++ b/drivers/net/ethernet/renesas/rswitch_l2.c
> > @@ -93,10 +93,36 @@ static void rswitch_update_l2_hw_forwarding(struct rswitch_private *priv)
> >         }
> >  }
> >
> > +static void rswitch_update_l2_hw_forwarding_gwca(struct
> > +rswitch_private *priv) {
> > +       struct rswitch_device *rdev;
> > +
> > +       if (priv->offload_brdev) {
> > +               rswitch_modify(priv->addr, FWPC0(AGENT_INDEX_GWCA),
> > +                              0, FWPC0_MACSSA | FWPC0_MACDSA |
> > + FWPC0_MACRUDA);
> > +
> > +               rswitch_for_all_ports(priv, rdev) {
> > +                       rswitch_modify(priv->addr, FWPC2(rdev->etha->index),
> > +                                      FIELD_PREP(FWPC2_LTWFW, BIT(AGENT_INDEX_GWCA)),
> > +                                      0);
> > +               }
> > +       } else {
> > +               rswitch_modify(priv->addr, FWPC0(AGENT_INDEX_GWCA),
> > +                              FWPC0_MACSSA | FWPC0_MACDSA | FWPC0_MACRUDA,
> > +                              0);
> > +               rswitch_for_all_ports(priv, rdev) {
> > +                       rswitch_modify(priv->addr, FWPC2(rdev->etha->index),
> > +                                      0,
> > +                                      FIELD_PREP(FWPC2_LTWFW, BIT(AGENT_INDEX_GWCA)));
> > +               }
> > +       }
> > +}
> 
> This looks a bit hard to read: are the bitfield values in the two branches the same or not?
> 
> I came up with the following, which avoids duplication of the field values, and of the control logic,
> but I am still not 100% happy with it:
> 
>     u32 fwpc0_clr, fwpc0_set, fwpc2_clr, fwpc2_set;
> 
>     fwpc0_clr = fwpc0_set = FWPC0_MACSSA | FWPC0_MACDSA | FWPC0_MACRUDA;
>     fwpc2_clr = fwpc2_set = FIELD_PREP(FWPC2_LTWFW, BIT(AGENT_INDEX_GWCA);
>     if (priv->offload_brdev) {
>             fwpc0_clr = fwpc2_set = 0;
>     } else {
>             fwpc0_set = fwpc2_clr = 0;
>     }
> 
>     rswitch_modify(priv->addr, FWPC0(AGENT_INDEX_GWCA), fwpc0_clr, fwpc0_set);
> 
>     rswitch_for_all_ports(priv, rdev) {
>             rswitch_modify(priv->addr, FWPC2(rdev->etha->index), fwpc2_clr,
>                            fwpc2_set);
>     }
> 
> Can this be improved?

Thank you very much for your comment. I will rework the function and re-send it.

Best regards,

Michael

> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> In personal conversations with technical people, I call myself a hacker. But when I'm talking to
> journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds