| Message ID | 20230529080840.1156458-3-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: renesas: rswitch: Improve performance of TX | expand |
Hi Shimoda-san, On Mon, May 29, 2023 at 10:08 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Rename GWCA related definitions to improve readability. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/rswitch.h > +++ b/drivers/net/ethernet/renesas/rswitch.h > @@ -959,9 +959,9 @@ struct rswitch_gwca { > int num_queues; > struct rswitch_gwca_queue ts_queue; > struct list_head ts_info_list; > - DECLARE_BITMAP(used, RSWITCH_MAX_NUM_QUEUES); > - u32 tx_irq_bits[RSWITCH_NUM_IRQ_REGS]; > - u32 rx_irq_bits[RSWITCH_NUM_IRQ_REGS]; > + DECLARE_BITMAP(used, GWCA_AXI_CHAIN_N); > + u32 tx_irq_bits[GWCA_NUM_IRQ_REGS]; > + u32 rx_irq_bits[GWCA_NUM_IRQ_REGS]; Not directly related to this patch, but is there a specific reason why tx_irq_bits and rx_irq_bits are arrays instead of bitmaps declared using DECLARE_BITMAP()? I think you can simplify the code that accesses them by using the bitmap APIs. > int speed; > }; Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Tuesday, May 30, 2023 4:18 PM > > Hi Shimoda-san, > > On Mon, May 29, 2023 at 10:08 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Rename GWCA related definitions to improve readability. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- a/drivers/net/ethernet/renesas/rswitch.h > > +++ b/drivers/net/ethernet/renesas/rswitch.h > > > @@ -959,9 +959,9 @@ struct rswitch_gwca { > > int num_queues; > > struct rswitch_gwca_queue ts_queue; > > struct list_head ts_info_list; > > - DECLARE_BITMAP(used, RSWITCH_MAX_NUM_QUEUES); > > - u32 tx_irq_bits[RSWITCH_NUM_IRQ_REGS]; > > - u32 rx_irq_bits[RSWITCH_NUM_IRQ_REGS]; > > + DECLARE_BITMAP(used, GWCA_AXI_CHAIN_N); > > + u32 tx_irq_bits[GWCA_NUM_IRQ_REGS]; > > + u32 rx_irq_bits[GWCA_NUM_IRQ_REGS]; > > Not directly related to this patch, but is there a specific reason why > tx_irq_bits and rx_irq_bits are arrays instead of bitmaps declared > using DECLARE_BITMAP()? I think you can simplify the code that accesses > them by using the bitmap APIs. Using arrays is easy to understand to me about GWDI[ES]i registers' handling in the following functions: - rswitch_is_any_data_irq() - rswitch_get_data_irq_status() - rswitch_data_irq() However, using bitmaps can avoid calculation of index and bit by division and modulo. So, it seems better. And, this is also not related to this patch though, I realized that separating tx_irq_bits and gwca.rx_irq_bits is not needed. Best regards, Yoshihiro Shimoda > > int speed; > > }; > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > 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
diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c index 29afaddb598d..51df96de6fd5 100644 --- a/drivers/net/ethernet/renesas/rswitch.c +++ b/drivers/net/ethernet/renesas/rswitch.c @@ -95,7 +95,7 @@ static void rswitch_top_init(struct rswitch_private *priv) { int i; - for (i = 0; i < RSWITCH_MAX_NUM_QUEUES; i++) + for (i = 0; i < GWCA_AXI_CHAIN_N; i++) iowrite32((i / 16) << (GWCA_INDEX * 8), priv->addr + TPEMIMC7(i)); } @@ -179,7 +179,7 @@ static bool rswitch_is_any_data_irq(struct rswitch_private *priv, u32 *dis, bool u32 *mask = tx ? priv->gwca.tx_irq_bits : priv->gwca.rx_irq_bits; int i; - for (i = 0; i < RSWITCH_NUM_IRQ_REGS; i++) { + for (i = 0; i < GWCA_NUM_IRQ_REGS; i++) { if (dis[i] & mask[i]) return true; } @@ -191,7 +191,7 @@ static void rswitch_get_data_irq_status(struct rswitch_private *priv, u32 *dis) { int i; - for (i = 0; i < RSWITCH_NUM_IRQ_REGS; i++) { + for (i = 0; i < GWCA_NUM_IRQ_REGS; i++) { dis[i] = ioread32(priv->addr + GWDIS(i)); dis[i] &= ioread32(priv->addr + GWDIE(i)); } @@ -863,7 +863,7 @@ static irqreturn_t rswitch_data_irq(struct rswitch_private *priv, u32 *dis) static irqreturn_t rswitch_gwca_irq(int irq, void *dev_id) { struct rswitch_private *priv = dev_id; - u32 dis[RSWITCH_NUM_IRQ_REGS]; + u32 dis[GWCA_NUM_IRQ_REGS]; irqreturn_t ret = IRQ_NONE; rswitch_get_data_irq_status(priv, dis); @@ -1891,7 +1891,7 @@ static int renesas_eth_sw_probe(struct platform_device *pdev) priv->gwca.index = AGENT_INDEX_GWCA; priv->gwca.num_queues = min(RSWITCH_NUM_PORTS * NUM_QUEUES_PER_NDEV, - RSWITCH_MAX_NUM_QUEUES); + GWCA_AXI_CHAIN_N); priv->gwca.queues = devm_kcalloc(&pdev->dev, priv->gwca.num_queues, sizeof(*priv->gwca.queues), GFP_KERNEL); if (!priv->gwca.queues) diff --git a/drivers/net/ethernet/renesas/rswitch.h b/drivers/net/ethernet/renesas/rswitch.h index b3e0411b408e..550a6bff9078 100644 --- a/drivers/net/ethernet/renesas/rswitch.h +++ b/drivers/net/ethernet/renesas/rswitch.h @@ -10,8 +10,6 @@ #include <linux/platform_device.h> #include "rcar_gen4_ptp.h" -#define RSWITCH_MAX_NUM_QUEUES 128 - #define RSWITCH_NUM_PORTS 3 #define rswitch_for_each_enabled_port(priv, i) \ for (i = 0; i < RSWITCH_NUM_PORTS; i++) \ @@ -50,6 +48,9 @@ #define AGENT_INDEX_GWCA 3 #define GWRO RSWITCH_GWCA0_OFFSET +#define GWCA_AXI_CHAIN_N 128 +#define GWCA_NUM_IRQ_REGS (GWCA_AXI_CHAIN_N / BITS_PER_TYPE(u32)) + #define GWCA_TS_IRQ_RESOURCE_NAME "gwca0_rxts0" #define GWCA_TS_IRQ_NAME "rswitch: gwca0_rxts0" #define GWCA_TS_IRQ_BIT BIT(0) @@ -949,7 +950,6 @@ struct rswitch_gwca_ts_info { u8 tag; }; -#define RSWITCH_NUM_IRQ_REGS (RSWITCH_MAX_NUM_QUEUES / BITS_PER_TYPE(u32)) struct rswitch_gwca { int index; struct rswitch_desc *linkfix_table; @@ -959,9 +959,9 @@ struct rswitch_gwca { int num_queues; struct rswitch_gwca_queue ts_queue; struct list_head ts_info_list; - DECLARE_BITMAP(used, RSWITCH_MAX_NUM_QUEUES); - u32 tx_irq_bits[RSWITCH_NUM_IRQ_REGS]; - u32 rx_irq_bits[RSWITCH_NUM_IRQ_REGS]; + DECLARE_BITMAP(used, GWCA_AXI_CHAIN_N); + u32 tx_irq_bits[GWCA_NUM_IRQ_REGS]; + u32 rx_irq_bits[GWCA_NUM_IRQ_REGS]; int speed; };
Rename GWCA related definitions to improve readability. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/net/ethernet/renesas/rswitch.c | 10 +++++----- drivers/net/ethernet/renesas/rswitch.h | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-)