RE: [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time

[Michael Dege]

Hello Geert,

Thank you very much for your comments.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Monday, July 7, 2025 2:50 PM
> To: Michael Dege <michael.dege@xxxxxxxxxxx>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>; Niklas Söderlund
> <niklas.soderlund@xxxxxxxxxxxx>; Paul Barker <paul@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; Nikita Yushchenko <nikita.yoush@xxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH 3/3] net: renesas: rswitch: add modifiable ageing time
>
> Hi Michael,
>
> On Fri, 4 Jul 2025 at 07:52, Michael Dege <michael.dege@xxxxxxxxxxx> wrote:
> > This commit allows the setting of the MAC table aging in the R-Car S4
> > Rswitch using the SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME attribute.
> >
> > Signed-off-by: Michael Dege <michael.dege@xxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/net/ethernet/renesas/rswitch.h
> > +++ b/drivers/net/ethernet/renesas/rswitch.h
> > @@ -849,6 +849,7 @@ enum rswitch_gwca_mode {
> >  #define FWMACAGC_MACDESOG      BIT(29)
> >
> >  #define RSW_AGEING_TIME                300
> > +#define RSW_MAX_AGEING_TIME    65535
>
> This is not needed (see below).
>
> >
> >  /* TOP */
> >  #define TPEMIMC7(queue)                (TPEMIMC70 + (queue) * 4)
> > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > b/drivers/net/ethernet/renesas/rswitch_l2.c
> > index
> > 242beb1f15c089585f5fe5019f626df8824b971a..c8a8a60a20e70f7ce421280ed35c
> > 0c4afe1ed039 100644
> > --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> > +++ b/drivers/net/ethernet/renesas/rswitch_l2.c
> > @@ -196,6 +196,30 @@ static int rswitch_netdevice_event(struct notifier_block *nb,
> >         return NOTIFY_OK;
> >  }
> >
> > +static int rswitch_update_ageing_time(struct net_device *ndev,
> > +clock_t time) {
> > +       struct rswitch_device *rdev = netdev_priv(ndev);
> > +       u32 reg_val, time_val;
> > +
> > +       if (!is_rdev(ndev))
> > +               return -ENODEV;
> > +
> > +       /* Although brctl accepts the ageing time parameter in seconds, the value
> > +        * passed to the driver is multiplied by 100. We need it in seconds.
> > +        */
> > +       time_val = (u32)time / 100;
>
> switchdev_attr.u.ageing_time is clock_t, which is long, so no cast is needed before doing the
> division.  Actually the cast may truncate very large values on 64-bit.
> However, dropping the cast means time_val should be changed to clock_t.

This division has been dropped completely. As it is not needed. Andrew pointed out, that brctl
Is deprecated and I learned that the ageing time parameter in iproute2 is already in seconds.
I need the value in seconds for our hardware, therefore no conversion needed. Update will come
With V2 of this patch set.

>
> > +
> > +       if (time_val > RSW_MAX_AGEING_TIME)
>
> if (!FIELD_FIT(FWMACAGC_MACAGT, time_val))
>
Thank you this is helpful, I will change it in the code.

Best regards,

Michael

> > +               return -EINVAL;
> > +
> > +       rdev = netdev_priv(ndev);
> > +       reg_val = FIELD_PREP(FWMACAGC_MACAGT, time_val);
> > +       reg_val |= FWMACAGC_MACAGE | FWMACAGC_MACAGSL;
> > +       iowrite32(reg_val, rdev->priv->addr + FWMACAGC);
> > +
> > +       return 0;
> > +}
> > +
> >  static int rswitch_port_attr_set(struct net_device *ndev, const void *ctx,
> >                                  const struct switchdev_attr *attr,
> >                                  struct netlink_ext_ack *extack)
>
> 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
________________________________

Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793

Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.