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

[Geert Uytterhoeven]

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?
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