diff mbox series

[net-next,2/5] net: renesas: rswitch: Rename GWCA related definitions

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

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 1 maintainers not CCed: alexanderduyck@fb.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 76 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yoshihiro Shimoda May 29, 2023, 8:08 a.m. UTC
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(-)

Comments

Geert Uytterhoeven May 30, 2023, 7:17 a.m. UTC | #1
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
Yoshihiro Shimoda May 30, 2023, 11:37 a.m. UTC | #2
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 mbox series

Patch

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;
 };