[PATCH 0/7] staging: vt6655: Convert four macros to one function

[PATCH 0/7] staging: vt6655: Convert four macros to one function

[Philipp Hortmann]

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(-)

-- 
2.37.1

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

[Philipp Hortmann]

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

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
 drivers/staging/vt6655/mac.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index b307161818a0..bfcbeb96f839 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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);	\
-- 
2.37.1

[PATCH 2/7] staging: vt6655: Rename MACvReceive0

[Philipp Hortmann]

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

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
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(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 92583ee8bffd..6525b7baf644 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -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)
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index bfcbeb96f839..60e0f980bcc5 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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);		\
-- 
2.37.1

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

[Philipp Hortmann]

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

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
 drivers/staging/vt6655/device_main.c | 13 +++++++++++++
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 6525b7baf644..cf25644f8671 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -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
  */
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 60e0f980bcc5..5747de436911 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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;					\
-- 
2.37.1

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

[Philipp Hortmann]

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

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
 drivers/staging/vt6655/device_main.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index cf25644f8671..31e0e55b9220 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -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)
-- 
2.37.1

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

[Philipp Hortmann]

checkpatch.pl does not accept multiline macros.

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
 drivers/staging/vt6655/device_main.c |  4 ++--
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 31e0e55b9220..01ce1c90ab09 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -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;
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 5747de436911..129a6602f6f0 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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;					\
-- 
2.37.1

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

[Philipp Hortmann]

checkpatch.pl does not accept multiline macros.

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
 drivers/staging/vt6655/device_main.c |  2 +-
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 01ce1c90ab09..898e06958203 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -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]++;
 
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 129a6602f6f0..1e57663ff066 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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;					\
-- 
2.37.1

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

[Philipp Hortmann]

checkpatch.pl does not accept multiline macros.

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
 drivers/staging/vt6655/device_main.c |  2 +-
 drivers/staging/vt6655/mac.h         | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 898e06958203..b2f3174f80df 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -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);
 
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 1e57663ff066..e2ef8c6ef7b7 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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;					\
-- 
2.37.1

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

[Joe Perches]

On Mon, 2022-07-18 at 00:20 +0200, Philipp Hortmann wrote:
> Fix name of a variable in four macros that use CamelCase which is not
> accepted by checkpatch.pl
[] 
> diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
[]
> @@ -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)					\
>  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)

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

[Dan Carpenter]

On Mon, Jul 18, 2022 at 12:20:25AM +0200, Philipp Hortmann wrote:
> 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

> diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
> index 5747de436911..129a6602f6f0 100644
> --- a/drivers/staging/vt6655/mac.h
> +++ b/drivers/staging/vt6655/mac.h
> @@ -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;					\

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

[Philipp Hortmann]

On 7/18/22 08:07, Joe Perches wrote:
> Please remember that checkpatch is a stupid little scripted tool
> and the actual goal is to have readable code.
Understood.
> 
> 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);
}

> 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)

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.

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

[Philipp Hortmann]

On 7/18/22 14:41, Dan Carpenter wrote:
> On Mon, Jul 18, 2022 at 12:20:25AM +0200, Philipp Hortmann wrote:
>> 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.
> 
> I tested this to see if was true and it just complained about potential
> side effects on iobase.
> 
> regads,
> dan carpenter
> 
>> diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
>> index 5747de436911..129a6602f6f0 100644
>> --- a/drivers/staging/vt6655/mac.h
>> +++ b/drivers/staging/vt6655/mac.h
>> @@ -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;					\
> 

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/kernelnewbies@kernelnewbies.org/msg22042.html

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

Thanks for your support.

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

[Dan Carpenter]

On Mon, Jul 18, 2022 at 10:00:59PM +0200, Philipp Hortmann wrote:
> > > diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
> > > index 5747de436911..129a6602f6f0 100644
> > > --- a/drivers/staging/vt6655/mac.h
> > > +++ b/drivers/staging/vt6655/mac.h
> > > @@ -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;					\
> > 
> 
> 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/kernelnewbies@kernelnewbies.org/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