Hi Shimoda-san, On Mon, May 29, 2023 at 10:08 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > Use per-queue rate limiter instead of global rate limiter. Otherwise > TX performance will be low when we use multiple ports at the same time. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/rswitch.c > +++ b/drivers/net/ethernet/renesas/rswitch.c > @@ -156,22 +156,31 @@ static int rswitch_gwca_axi_ram_reset(struct rswitch_private *priv) > return rswitch_reg_wait(priv->addr, GWARIRM, GWARIRM_ARR, GWARIRM_ARR); > } > > -static void rswitch_gwca_set_rate_limit(struct rswitch_private *priv, int rate) > +static void rswitch_gwca_set_rate_limit(struct rswitch_private *priv, > + struct rswitch_gwca_queue *txq) > { > - u32 gwgrlulc, gwgrlc; > + u64 period_ps; > + unsigned long rate; > + u32 gwrlc; > > - switch (rate) { > - case 1000: > - gwgrlulc = 0x0000005f; > - gwgrlc = 0x00010260; > - break; > - default: > - dev_err(&priv->pdev->dev, "%s: This rate is not supported (%d)\n", __func__, rate); > - return; > - } > + rate = clk_get_rate(priv->aclk); > + if (!rate) > + rate = RSWITCH_ACLK_DEFAULT; > + > + period_ps = div64_u64(1000000000000ULL, rate); div64_ul, as rate is unsigned long. > + > + /* GWRLC value = 256 * ACLK_period[ns] * maxBandwidth[Gbps] */ > + gwrlc = 256 * period_ps * txq->speed / 1000000; This contains an open-coded 64-by-32 division, causing link failures on 32-bit platforms, so you should use div_u64() instead. However, because of the premultiplication by speed, which is 32-bit, you can use the mul_u64_u32_div() helper. Combining this with the calculation of period_ps above, you can simplify this to: gwrlc = div64_ul(256000000ULL * txq->speed, rate); > + > + /* To avoid overflow internally, the value should be 97% */ > + gwrlc = gwrlc * 97 / 100; > > - iowrite32(gwgrlulc, priv->addr + GWGRLULC); > - iowrite32(gwgrlc, priv->addr + GWGRLC); > + dev_dbg(&priv->pdev->dev, > + "%s: index = %d, speed = %d, rate = %ld, gwrlc = %08x\n", > + __func__, txq->index_trim, txq->speed, rate, gwrlc); > + > + iowrite32(GWRLULC_NOT_REQUIRED, priv->addr + GWRLULC(txq->index_trim)); > + iowrite32(gwrlc | GWRLC_RLE, priv->addr + GWRLC(txq->index_trim)); > } > 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