staging: vt6655: Convert four macros to one function

PATCH In review Philipp Hortmann 3 participants 13 messages Jul 17, 2022 – Jul 19, 2022 Staging

lore

Share & tools

lore

Philipp Hortmann

author

2022-07-17 22:20 UTC #0 lore raw

cover staging: vt6655: Convert four macros to one function

Convert four multiline macros to one function with parameter. checkpatch.pl does not accept multiline macros.

Tested with vt6655 on mini PCI Module Transferred this patch over wlan connection of vt6655

Philipp Hortmann (7):
staging: vt6655: Rename dwData to reg_value in four macros
staging: vt6655: Rename MACvReceive0
staging: vt6655: Convert macro vt6655_mac_dma_ctl to function
staging: vt6655: Add parameter to function vt6655_mac_dma_ctl
staging: vt6655: Replace MACvReceive1 with function vt6655_mac_dma_ctl
staging: vt6655: Replace MACvTransmit0 with function

    vt6655_mac_dma_ctl
  staging: vt6655: Replace MACvTransmitAC0 with function
    vt6655_mac_dma_ctl

 drivers/staging/vt6655/device_main.c | 25 ++++++++++++-----
 drivers/staging/vt6655/mac.h         | 40 ----------------------------
 2 files changed, 19 insertions(+), 46 deletions(-)

Signature · 1 line

2.37.1

Philipp Hortmann

author

2022-07-17 22:20 UTC #1 lore raw

PATCH 1/7 staging: vt6655: Rename dwData to reg_value in four macros

Fix name of a variable in four macros that use CamelCase which is not accepted by checkpatch.pl

Signed-off-by Philipp Hortmann <[email protected]>

---
 drivers/staging/vt6655/mac.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

drivers/staging/vt6655/mac.h +12 ~~-12~~

index b307161818a0..bfcbeb96f839 100644
@@ -539,9 +539,9 @@
 
 #define MACvReceive0(iobase)						\
 do {									\
-	unsigned long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_RXDMACTL0);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_RXDMACTL0);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL0);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL0);	\

@@ -549,9 +549,9 @@ do {									\
 
 #define MACvReceive1(iobase)						\
 do {									\
-	unsigned long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_RXDMACTL1);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_RXDMACTL1);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL1);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL1);	\

@@ -559,9 +559,9 @@ do {									\
 
 #define MACvTransmit0(iobase)						\
 do {									\
-	unsigned long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_TXDMACTL0);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_TXDMACTL0);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_TXDMACTL0);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_TXDMACTL0);	\

@@ -569,9 +569,9 @@ do {									\
 
 #define MACvTransmitAC0(iobase)					\
 do {									\
-	unsigned long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_AC0DMACTL);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_AC0DMACTL);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_AC0DMACTL);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_AC0DMACTL);	\

Signature · 1 line

2.37.1

Joe Perches

2022-07-18 06:07 UTC #2 lore raw

Re: [PATCH 1/7] staging: vt6655: Rename dwData to reg_value in four macros

Quoted 2 lines · Philipp Hortmann

Fix name of a variable in four macros that use CamelCase which is not accepted by checkpatch.pl

[]

drivers/staging/vt6655/mac.h

Quoted 1 line

drivers/staging/vt6655/mac.h +0 -0

[]

Quoted 42 lines

drivers/staging/vt6655/mac.h +9 -9

@@ -539,9 +539,9 @@
 
 #define MACvReceive0(iobase)						\
 do {									\
-	unsigned long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_RXDMACTL0);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_RXDMACTL0);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL0);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL0);	\

@@ -549,9 +549,9 @@ do {									\
 
 #define MACvReceive1(iobase)						\
 do {									\
-	unsigned long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_RXDMACTL1);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_RXDMACTL1);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL1);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL1);	\

@@ -559,9 +559,9 @@ do {									\
 
 #define MACvTransmit0(iobase)						\
 do {									\
-	unsignedc long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_TXDMACTL0);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_TXDMACTL0);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_TXDMACTL0);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_TXDMACTL0);	\

@@ -569,9 +569,9 @@ do {									\
 
 #define MACvTransmitAC0(iobase)					\

drivers/staging/vt6655/mac.h do { \ · line 577

Quoted 10 lines

drivers/staging/vt6655/mac.h +3 -3

 do {									\
-	unsigned long dwData;						\
-	dwData = ioread32(iobase + MAC_REG_AC0DMACTL);			\
-	if (dwData & DMACTL_RUN)					\
+	unsigned long reg_value;					\
+	reg_value = ioread32(iobase + MAC_REG_AC0DMACTL);		\
+	if (reg_value & DMACTL_RUN)					\
 		iowrite32(DMACTL_WAKE, iobase + MAC_REG_AC0DMACTL);	\
 	else								\
 		iowrite32(DMACTL_RUN, iobase + MAC_REG_AC0DMACTL);	\

Please remember that checkpatch is a stupid little scripted tool and the actual goal is to have readable code.

Look a bit beyond the code and see if and how you could make the code better.

All of these macros have the same form and logic.

Perhaps it'd be better to use another indirect macro and define all of these with that new macro.

Something like:

#define mac_v(iobase, reg) \

do {									\
	void __iomem *addr = (iobase) + (reg);				\
	iowrite32(ioread32(addr) & DMACTL_RUN ? DMACTL_WAKE : DMACTL_RUN,\
		  addr);						\
} while (0)

#define MACvReceive0(iobase) mac_v(iobase, MAC_REG_RXDMACTL0)
#define MACvReceive1(iobase) mac_v(iobase, MAC_REG_RXDMACTL1)
#define MACvTransmit0(iobase) mac_v(iobase, MAC_REG_TXDMACTL0)
#define MACvTransmitAC0(iobase) mac_v(iobase, MAC_REG_AC0DMACTL)

Philipp Hortmann

author

2022-07-18 19:33 UTC #3 lore raw

Re: [PATCH 1/7] staging: vt6655: Rename dwData to reg_value in four macros

Quoted 2 lines · Joe Perches

Please remember that checkpatch is a stupid little scripted tool and the actual goal is to have readable code.

Understood.

Quoted 6 lines

Look a bit beyond the code and see if and how you could make the code better.
All of these macros have the same form and logic.

That is the reason why I put them all together in one static function:
static void vt6655_mac_dma_ctl(void __iomem *iobase, u8 reg_index)

{
	unsigned long reg_value;

	reg_value = ioread32(iobase + reg_index);
	if (reg_value & DMACTL_RUN)
		iowrite32(DMACTL_WAKE, iobase + reg_index);
	else
		iowrite32(DMACTL_RUN, iobase + reg_index);
}

Quoted 12 lines

Perhaps it'd be better to use another indirect macro and define all of these with that new macro.
Something like:
#define mac_v(iobase, reg) \
do {									\
	void __iomem *addr = (iobase) + (reg);				\
	iowrite32(ioread32(addr) & DMACTL_RUN ? DMACTL_WAKE : DMACTL_RUN,\
		  addr);						\
} while (0)

Quoted 4 lines

#define MACvReceive0(iobase) mac_v(iobase, MAC_REG_RXDMACTL0)
#define MACvReceive1(iobase) mac_v(iobase, MAC_REG_RXDMACTL1)
#define MACvTransmit0(iobase) mac_v(iobase, MAC_REG_TXDMACTL0)
#define MACvTransmitAC0(iobase) mac_v(iobase, MAC_REG_AC0DMACTL)

That is an interesting solution. But for me this code is not as good readable as my proposal. Reason is that I struggle with the function in function with condition broken into two lines.

Philipp Hortmann

author

2022-07-17 22:20 UTC #4 lore raw

PATCH 2/7 staging: vt6655: Rename MACvReceive0

In reply to Philipp Hortmann

Fix name of a macro that uses CamelCase which is not accepted by checkpatch.pl

Signed-off-by Philipp Hortmann <[email protected]>

---
Please read all patches of this series then the
new name makes sense.

---
 drivers/staging/vt6655/device_main.c | 4 ++--
 drivers/staging/vt6655/mac.h         | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

drivers/staging/vt6655/device_main.c +2 -2

index 92583ee8bffd..6525b7baf644 100644
@@ -420,7 +420,7 @@ static void device_init_registers(struct vnt_private *priv)
 		vt6655_mac_reg_bits_on(priv->port_offset, MAC_REG_RCR, RCR_WPAERR);
 
 	/* Turn On Rx DMA */
-	MACvReceive0(priv->port_offset);
+	vt6655_mac_dma_ctl(priv->port_offset);
 	MACvReceive1(priv->port_offset);
 
 	/* start the adapter */

@@ -1135,7 +1135,7 @@ static void vnt_interrupt_process(struct vnt_private *priv)
 
 		isr = ioread32(priv->port_offset + MAC_REG_ISR);
 
-		MACvReceive0(priv->port_offset);
+		vt6655_mac_dma_ctl(priv->port_offset);
 		MACvReceive1(priv->port_offset);
 
 		if (max_count > priv->opts.int_works)

drivers/staging/vt6655/mac.h +1 -1

index bfcbeb96f839..60e0f980bcc5 100644
@@ -537,7 +537,7 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define MACvReceive0(iobase)						\
+#define vt6655_mac_dma_ctl(iobase)					\
 do {									\
 	unsigned long reg_value;					\
 	reg_value = ioread32(iobase + MAC_REG_RXDMACTL0);		\

Signature · 1 line

2.37.1

Philipp Hortmann

author

2022-07-17 22:20 UTC #5 lore raw

PATCH 3/7 staging: vt6655: Convert macro vt6655_mac_dma_ctl to function

In reply to Philipp Hortmann

Convert macro vt6655_mac_dma_ctl to function.
checkpatch.pl does not accept multiline macros.

Signed-off-by Philipp Hortmann <[email protected]>

---
 drivers/staging/vt6655/device_main.c | 13 +++++++++++++
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 13 insertions(+), 10 deletions(-)

drivers/staging/vt6655/device_main.c +13 -0

index 6525b7baf644..cf25644f8671 100644
@@ -125,6 +125,8 @@ static void device_print_info(struct vnt_private *priv);
 static void vt6655_mac_write_bssid_addr(void __iomem *iobase, const u8 *mac_addr);
 static void vt6655_mac_read_ether_addr(void __iomem *iobase, u8 *mac_addr);
 
+static void vt6655_mac_dma_ctl(void __iomem *iobase);
+
 static int device_init_rd0_ring(struct vnt_private *priv);
 static int device_init_rd1_ring(struct vnt_private *priv);
 static int device_init_td0_ring(struct vnt_private *priv);

@@ -205,6 +207,17 @@ static void vt6655_mac_read_ether_addr(void __iomem *iobase, u8 *mac_addr)
 	iowrite8(0, iobase + MAC_REG_PAGE1SEL);
 }
 
+static void vt6655_mac_dma_ctl(void __iomem *iobase)
+{
+	unsigned long reg_value;
+
+	reg_value = ioread32(iobase + MAC_REG_RXDMACTL0);
+	if (reg_value & DMACTL_RUN)
+		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL0);
+	else
+		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL0);
+}
+
 /*
  * Initialisation of MAC & BBP registers
  */

drivers/staging/vt6655/mac.h +0 ~~-10~~

index 60e0f980bcc5..5747de436911 100644
@@ -537,16 +537,6 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define vt6655_mac_dma_ctl(iobase)					\
-do {									\
-	unsigned long reg_value;					\
-	reg_value = ioread32(iobase + MAC_REG_RXDMACTL0);		\
-	if (reg_value & DMACTL_RUN)					\
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL0);	\
-	else								\
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL0);	\
-} while (0)
-
 #define MACvReceive1(iobase)						\
 do {									\
 	unsigned long reg_value;					\

Signature · 1 line

2.37.1

Philipp Hortmann

author

2022-07-17 22:20 UTC #6 lore raw

PATCH 4/7 staging: vt6655: Add parameter to function vt6655_mac_dma_ctl

In reply to Philipp Hortmann

Add one parameter to avoid multiple repetitions of the same macro/function.

Signed-off-by Philipp Hortmann <[email protected]>

---
 drivers/staging/vt6655/device_main.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/staging/vt6655/device_main.c +7 -7

index cf25644f8671..31e0e55b9220 100644
@@ -125,7 +125,7 @@ static void device_print_info(struct vnt_private *priv);
 static void vt6655_mac_write_bssid_addr(void __iomem *iobase, const u8 *mac_addr);
 static void vt6655_mac_read_ether_addr(void __iomem *iobase, u8 *mac_addr);
 
-static void vt6655_mac_dma_ctl(void __iomem *iobase);
+static void vt6655_mac_dma_ctl(void __iomem *iobase, u8 reg_index);
 
 static int device_init_rd0_ring(struct vnt_private *priv);
 static int device_init_rd1_ring(struct vnt_private *priv);

@@ -207,15 +207,15 @@ static void vt6655_mac_read_ether_addr(void __iomem *iobase, u8 *mac_addr)
 	iowrite8(0, iobase + MAC_REG_PAGE1SEL);
 }
 
-static void vt6655_mac_dma_ctl(void __iomem *iobase)
+static void vt6655_mac_dma_ctl(void __iomem *iobase, u8 reg_index)
 {
 	unsigned long reg_value;
 
-	reg_value = ioread32(iobase + MAC_REG_RXDMACTL0);
+	reg_value = ioread32(iobase + reg_index);
 	if (reg_value & DMACTL_RUN)
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL0);
+		iowrite32(DMACTL_WAKE, iobase + reg_index);
 	else
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL0);
+		iowrite32(DMACTL_RUN, iobase + reg_index);
 }
 
 /*

@@ -433,7 +433,7 @@ static void device_init_registers(struct vnt_private *priv)
 		vt6655_mac_reg_bits_on(priv->port_offset, MAC_REG_RCR, RCR_WPAERR);
 
 	/* Turn On Rx DMA */
-	vt6655_mac_dma_ctl(priv->port_offset);
+	vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_RXDMACTL0);
 	MACvReceive1(priv->port_offset);
 
 	/* start the adapter */

@@ -1148,7 +1148,7 @@ static void vnt_interrupt_process(struct vnt_private *priv)
 
 		isr = ioread32(priv->port_offset + MAC_REG_ISR);
 
-		vt6655_mac_dma_ctl(priv->port_offset);
+		vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_RXDMACTL0);
 		MACvReceive1(priv->port_offset);
 
 		if (max_count > priv->opts.int_works)

Signature · 1 line

2.37.1

Philipp Hortmann

author

2022-07-17 22:20 UTC #7 lore raw

PATCH 5/7 staging: vt6655: Replace MACvReceive1 with function vt6655_mac_dma_ctl

In reply to Philipp Hortmann

checkpatch.pl does not accept multiline macros.

Signed-off-by Philipp Hortmann <[email protected]>

---
 drivers/staging/vt6655/device_main.c |  4 ++--
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 2 insertions(+), 12 deletions(-)

drivers/staging/vt6655/device_main.c +2 -2

index 31e0e55b9220..01ce1c90ab09 100644
@@ -434,7 +434,7 @@ static void device_init_registers(struct vnt_private *priv)
 
 	/* Turn On Rx DMA */
 	vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_RXDMACTL0);
-	MACvReceive1(priv->port_offset);
+	vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_RXDMACTL1);
 
 	/* start the adapter */
 	iowrite8(HOSTCR_MACEN | HOSTCR_RXON | HOSTCR_TXON, priv->port_offset + MAC_REG_HOSTCR);

@@ -1149,7 +1149,7 @@ static void vnt_interrupt_process(struct vnt_private *priv)
 		isr = ioread32(priv->port_offset + MAC_REG_ISR);
 
 		vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_RXDMACTL0);
-		MACvReceive1(priv->port_offset);
+		vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_RXDMACTL1);
 
 		if (max_count > priv->opts.int_works)
 			break;

drivers/staging/vt6655/mac.h +0 ~~-10~~

index 5747de436911..129a6602f6f0 100644
@@ -537,16 +537,6 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define MACvReceive1(iobase)						\
-do {									\
-	unsigned long reg_value;					\
-	reg_value = ioread32(iobase + MAC_REG_RXDMACTL1);		\
-	if (reg_value & DMACTL_RUN)					\
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL1);	\
-	else								\
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL1);	\
-} while (0)
-
 #define MACvTransmit0(iobase)						\
 do {									\
 	unsigned long reg_value;					\

Signature · 1 line

2.37.1

Dan Carpenter

2022-07-18 12:41 UTC #8 lore raw

Re: [PATCH 5/7] staging: vt6655: Replace MACvReceive1 with function vt6655_mac_dma_ctl

Quoted 2 lines · Philipp Hortmann

checkpatch.pl does not accept multiline macros.

What? Really?

I tested this to see if was true and it just complained about potential side effects on iobase.

regads,
dan carpenter

Quoted 21 lines

drivers/staging/vt6655/mac.h +0 ~~-10~~

index 5747de436911..129a6602f6f0 100644
@@ -537,16 +537,6 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define MACvReceive1(iobase)						\
-do {									\
-	unsigned long reg_value;					\
-	reg_value = ioread32(iobase + MAC_REG_RXDMACTL1);		\
-	if (reg_value & DMACTL_RUN)					\
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL1);	\
-	else								\
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL1);	\
-} while (0)
-
 #define MACvTransmit0(iobase)						\
 do {									\
 	unsigned long reg_value;					\

Philipp Hortmann

author

2022-07-18 20:01 UTC #9 lore raw

Re: [PATCH 5/7] staging: vt6655: Replace MACvReceive1 with function vt6655_mac_dma_ctl

Quoted 5 lines · Dan Carpenter

Quoted 2 lines · Philipp Hortmann
checkpatch.pl does not accept multiline macros.
What? Really?

You are right. It does not really complain about multiline macros but you cannot have a clean checkpatch check when using more than one macro containing the same variable. Find more info below.

Quoted 15 lines

I tested this to see if was true and it just complained about potential side effects on iobase.
regads,
dan carpenter
Quoted 8 lines
drivers/staging/vt6655/mac.h +0 -0
index 5747de436911..129a6602f6f0 100644
@@ -537,16 +537,6 @@
 
  /*---------------------  Export Macros ------------------------------*/
 

Quoted 14 lines

drivers/staging/vt6655/mac.h line 542

Quoted 13 lines

drivers/staging/vt6655/mac.h +0 ~~-10~~

-#define MACvReceive1(iobase)						\
-do {									\
-	unsigned long reg_value;					\
-	reg_value = ioread32(iobase + MAC_REG_RXDMACTL1);		\
-	if (reg_value & DMACTL_RUN)					\
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL1);	\
-	else								\
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL1);	\
-} while (0)
-
  #define MACvTransmit0(iobase)						\
  do {									\
  	unsigned long reg_value;					\

I was asking in kernelnewbies what to do with multi line macros as checkpatch.pl warnings cannot be totally avoided.

Greg replied to make functions out of them.

Please find the full email under:

https://www.mail-archive.com/[email protected]/msg22042.html

In this case I really like the static function solution much more than the macros.

Thanks for your support.

Dan Carpenter

2022-07-19 10:04 UTC #10 lore raw

Re: [PATCH 5/7] staging: vt6655: Replace MACvReceive1 with function vt6655_mac_dma_ctl

Quoted 24 lines · Philipp Hortmann

Quoted 11 lines
Quoted 11 lines
drivers/staging/vt6655/mac.h +0 -5
index 5747de436911..129a6602f6f0 100644
@@ -537,16 +537,6 @@
  /*---------------------  Export Macros ------------------------------*/
-#define MACvReceive1(iobase)						\
-do {									\
-	unsigned long reg_value;					\
-	reg_value = ioread32(iobase + MAC_REG_RXDMACTL1);		\
-	if (reg_value & DMACTL_RUN)					\
Quoted 9 lines
drivers/staging/vt6655/mac.h line 542
Quoted 8 lines
drivers/staging/vt6655/mac.h +0 -5
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL1);	\
-	else								\
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL1);	\
-} while (0)
-
  #define MACvTransmit0(iobase)						\
  do {									\
  	unsigned long reg_value;					\
I was asking in kernelnewbies what to do with multi line macros as checkpatch.pl warnings cannot be totally avoided.

Quoted 8 lines

Greg replied to make functions out of them.
Please find the full email under:
https://www.mail-archive.com/[email protected]/msg22042.html
In this case I really like the static function solution much more than the macros.

Yes. We all like the static function. It's the commit message, I'm not so keen on.

You could have avoided the checkpatch warning with an assignment at the start of the macro:

	typeof(iobase) base = (iobase);

#define MACvReceive1(iobase) \

do {									\
	typeof(iobase) base = (iobase);					\
	unsigned long reg_value = ioread32(base + MAC_REG_RXDMACTL1);	\
	if (reg_value & DMACTL_RUN)					\
		iowrite32(DMACTL_WAKE, base + MAC_REG_RXDMACTL1);	\
	else								\
		iowrite32(DMACTL_RUN, base + MAC_REG_RXDMACTL1);	\
} while (0)

It's not a *good* solution, but it works.

The means the "iobase" argument would only be executed one time. Imagine if someone passed "iobase++" to the original function. It would have incremented twice instead of once as expected. That's what the checkpatch warning is saying. Nothing to do with multiple lines.

regards,
dan carpenter

Philipp Hortmann

author

2022-07-17 22:20 UTC #11 lore raw

PATCH 6/7 staging: vt6655: Replace MACvTransmit0 with function vt6655_mac_dma_ctl

In reply to Philipp Hortmann

checkpatch.pl does not accept multiline macros.

Signed-off-by Philipp Hortmann <[email protected]>

---
 drivers/staging/vt6655/device_main.c |  2 +-
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

drivers/staging/vt6655/device_main.c +1 -1

index 01ce1c90ab09..898e06958203 100644
@@ -1233,7 +1233,7 @@ static int vnt_tx_packet(struct vnt_private *priv, struct sk_buff *skb)
 	if (head_td->td_info->flags & TD_FLAGS_NETIF_SKB)
 		MACvTransmitAC0(priv->port_offset);
 	else
-		MACvTransmit0(priv->port_offset);
+		vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_TXDMACTL0);
 
 	priv->iTDUsed[dma_idx]++;

drivers/staging/vt6655/mac.h +0 ~~-10~~

index 129a6602f6f0..1e57663ff066 100644
@@ -537,16 +537,6 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define MACvTransmit0(iobase)						\
-do {									\
-	unsigned long reg_value;					\
-	reg_value = ioread32(iobase + MAC_REG_TXDMACTL0);		\
-	if (reg_value & DMACTL_RUN)					\
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_TXDMACTL0);	\
-	else								\
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_TXDMACTL0);	\
-} while (0)
-
 #define MACvTransmitAC0(iobase)					\
 do {									\
 	unsigned long reg_value;					\

Signature · 1 line

2.37.1

Philipp Hortmann

author

2022-07-17 22:20 UTC #12 lore raw

PATCH 7/7 staging: vt6655: Replace MACvTransmitAC0 with function vt6655_mac_dma_ctl

In reply to Philipp Hortmann

checkpatch.pl does not accept multiline macros.

Signed-off-by Philipp Hortmann <[email protected]>

---
 drivers/staging/vt6655/device_main.c |  2 +-
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

drivers/staging/vt6655/device_main.c +1 -1

index 898e06958203..b2f3174f80df 100644
@@ -1231,7 +1231,7 @@ static int vnt_tx_packet(struct vnt_private *priv, struct sk_buff *skb)
 	wmb(); /* second memory barrier */
 
 	if (head_td->td_info->flags & TD_FLAGS_NETIF_SKB)
-		MACvTransmitAC0(priv->port_offset);
+		vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_AC0DMACTL);
 	else
 		vt6655_mac_dma_ctl(priv->port_offset, MAC_REG_TXDMACTL0);

drivers/staging/vt6655/mac.h +0 ~~-10~~

index 1e57663ff066..e2ef8c6ef7b7 100644
@@ -537,16 +537,6 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define MACvTransmitAC0(iobase)					\
-do {									\
-	unsigned long reg_value;					\
-	reg_value = ioread32(iobase + MAC_REG_AC0DMACTL);		\
-	if (reg_value & DMACTL_RUN)					\
-		iowrite32(DMACTL_WAKE, iobase + MAC_REG_AC0DMACTL);	\
-	else								\
-		iowrite32(DMACTL_RUN, iobase + MAC_REG_AC0DMACTL);	\
-} while (0)
-
 #define MACvClearStckDS(iobase)					\
 do {									\
 	unsigned char byOrgValue;					\

Signature · 1 line

2.37.1