Skip to contentSkip to conversation

staging: vt6655: Convert multiline macro to static function

PATCH Superseded Philipp Hortmann 2 participants 9 messages Jul 20, 2022 – Jul 27, 2022 Staging

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Philipp Hortmann

author
#0 lore raw

cover staging: vt6655: Convert multiline macro to static function

Convert multiline macro to static function. Multiline macros are not liked by kernel community.

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

Philipp Hortmann (3):
staging: vt6655: Rename byOrgValue to reg_value in a macro
staging: vt6655: Rename MACvClearStckDS
staging: vt6655: Convert macro vt6655_mac_clear_stck_ds to function

 drivers/staging/vt6655/mac.c | 13 ++++++++++++-
 drivers/staging/vt6655/mac.h |  8 --------
 2 files changed, 12 insertions(+), 9 deletions(-)
Signature · 1 line
2.37.1
author
#1 lore raw

PATCH 1/3 staging: vt6655: Rename byOrgValue to reg_value in a macro

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

Signed-off-by Philipp Hortmann <[email protected]>
---
 drivers/staging/vt6655/mac.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/vt6655/mac.h +4 -4
index e2ef8c6ef7b7..837ae9dd6929 100644
@@ -539,10 +539,10 @@
 
 #define MACvClearStckDS(iobase)					\
 do {									\
-	unsigned char byOrgValue;					\
-	byOrgValue = ioread8(iobase + MAC_REG_STICKHW);			\
-	byOrgValue = byOrgValue & 0xFC;					\
-	iowrite8(byOrgValue, iobase + MAC_REG_STICKHW);			\
+	unsigned char reg_value;					\
+	reg_value = ioread8(iobase + MAC_REG_STICKHW);			\
+	reg_value = reg_value & 0xFC;					\
+	iowrite8(reg_value, iobase + MAC_REG_STICKHW);			\
 } while (0)
 
 #define MACvSelectPage0(iobase)				\
Signature · 1 line
2.37.1
author
#5 lore raw

PATCH 2/3 staging: vt6655: Rename MACvClearStckDS

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]>
---
 drivers/staging/vt6655/mac.c | 2 +-
 drivers/staging/vt6655/mac.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/staging/vt6655/mac.c +1 -1
index a7f645f9ccf8..d7ee42df7062 100644
@@ -518,7 +518,7 @@ void MACvInitialize(struct vnt_private *priv)
 {
 	void __iomem *io_base = priv->port_offset;
 	/* clear sticky bits */
-	MACvClearStckDS(io_base);
+	vt6655_mac_clear_stck_ds(io_base);
 	/* disable force PME-enable */
 	iowrite8(PME_OVR, io_base + MAC_REG_PMC1);
 	/* only 3253 A */
drivers/staging/vt6655/mac.h +1 -1
index 837ae9dd6929..c2c9cb07f32d 100644
@@ -537,7 +537,7 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define MACvClearStckDS(iobase)					\
+#define vt6655_mac_clear_stck_ds(iobase)				\
 do {									\
 	unsigned char reg_value;					\
 	reg_value = ioread8(iobase + MAC_REG_STICKHW);			\
Signature · 1 line
2.37.1
author
#7 lore raw

PATCH 3/3 staging: vt6655: Convert macro vt6655_mac_clear_stck_ds to function

In reply to Philipp Hortmann

Convert once used macro to static function. Multiline macros are not liked by kernel community.

Signed-off-by Philipp Hortmann <[email protected]>
---
 drivers/staging/vt6655/mac.c | 11 +++++++++++
 drivers/staging/vt6655/mac.h |  8 --------
 2 files changed, 11 insertions(+), 8 deletions(-)
drivers/staging/vt6655/mac.c +11 -0
index d7ee42df7062..3e9494c653dc 100644
@@ -38,6 +38,8 @@
 
 #include "mac.h"
 
+static void vt6655_mac_clear_stck_ds(void __iomem *iobase);
+
 void vt6655_mac_reg_bits_on(void __iomem *iobase, const u8 reg_offset, const u8 bit_mask)
 {
 	unsigned char reg_value;

@@ -70,6 +72,15 @@ void vt6655_mac_word_reg_bits_off(void __iomem *iobase, const u8 reg_offset, con
 	iowrite16(reg_value & ~(bit_mask), iobase + reg_offset);
 }
 
+static void vt6655_mac_clear_stck_ds(void __iomem *iobase)
+{
+	unsigned char reg_value;
+
+	reg_value = ioread8(iobase + MAC_REG_STICKHW);
+	reg_value = reg_value & 0xFC;
+	iowrite8(reg_value, iobase + MAC_REG_STICKHW);
+}
+
 /*
  * Description:
  *      Test if all test bits off
drivers/staging/vt6655/mac.h +0 -8
index c2c9cb07f32d..467c599a3289 100644
@@ -537,14 +537,6 @@
 
 /*---------------------  Export Macros ------------------------------*/
 
-#define vt6655_mac_clear_stck_ds(iobase)				\
-do {									\
-	unsigned char reg_value;					\
-	reg_value = ioread8(iobase + MAC_REG_STICKHW);			\
-	reg_value = reg_value & 0xFC;					\
-	iowrite8(reg_value, iobase + MAC_REG_STICKHW);			\
-} while (0)
-
 #define MACvSelectPage0(iobase)				\
 	iowrite8(0, iobase + MAC_REG_PAGE1SEL)
Signature · 1 line
2.37.1
maintainer
#8 lore raw

Re: [PATCH 3/3] staging: vt6655: Convert macro vt6655_mac_clear_stck_ds to function

Quoted 25 lines · Philipp Hortmann

Convert once used macro to static function. Multiline macros are not liked by kernel community.

Signed-off-by Philipp Hortmann <[email protected]>
---
 drivers/staging/vt6655/mac.c | 11 +++++++++++
 drivers/staging/vt6655/mac.h |  8 --------
 2 files changed, 11 insertions(+), 8 deletions(-)
drivers/staging/vt6655/mac.c +2 -0
index d7ee42df7062..3e9494c653dc 100644
@@ -38,6 +38,8 @@
 
 #include "mac.h"
 
+static void vt6655_mac_clear_stck_ds(void __iomem *iobase);
+
 void vt6655_mac_reg_bits_on(void __iomem *iobase, const u8 reg_offset, const u8 bit_mask)
 {
 	unsigned char reg_value;

@@ -70,6 +72,15 @@ void vt6655_mac_word_reg_bits_off(void __iomem *iobase, const u8 reg_offset, con
 	iowrite16(reg_value & ~(bit_mask), iobase + reg_offset);
 }

drivers/staging/vt6655/mac.c void vt6655_mac_word_reg_bits_off(void __iomem *iobase, const u8 reg_offset, con · line 83

Quoted 10 lines
drivers/staging/vt6655/mac.c +9 -0
 
+static void vt6655_mac_clear_stck_ds(void __iomem *iobase)
+{
+	unsigned char reg_value;
+
+	reg_value = ioread8(iobase + MAC_REG_STICKHW);
+	reg_value = reg_value & 0xFC;
+	iowrite8(reg_value, iobase + MAC_REG_STICKHW);
+}
+

Why not move this function up above, to before you call it, and then you do not need the function declaration in this file?

thanks,

greg k-h

maintainer
#2 lore raw

Re: [PATCH 1/3] staging: vt6655: Rename byOrgValue to reg_value in a macro

In reply to Philipp Hortmann
Quoted 2 lines · Philipp Hortmann

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

Why rename something you are about to delete in a later patch?

thanks,

greg k-h

maintainer
#6 lore raw

Re: [PATCH 2/3] staging: vt6655: Rename MACvClearStckDS

In reply to Philipp Hortmann
Quoted 2 lines · Philipp Hortmann

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

You do have 72 columns to use here, feel free to do so :)

Also a bit better description would be nice.

thanks,

greg k-h

author
#3 lore raw

Re: [PATCH 1/3] staging: vt6655: Rename byOrgValue to reg_value in a macro

In reply to Greg Kroah-Hartman
Quoted 9 lines · Greg Kroah-Hartman
Quoted 2 lines · Philipp Hortmann

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

Why rename something you are about to delete in a later patch?

thanks,

greg k-h

I need to rename this variable. Does it matter if I do it in first or last patch? When doing it first I do not have to review so much checkpatch warnings.

Or is it OK to convert the macro to a function and rename the variable in one patch?

thanks,

Philipp

maintainer
#4 lore raw

Re: [PATCH 1/3] staging: vt6655: Rename byOrgValue to reg_value in a macro

Quoted 9 lines · Philipp Hortmann
Quoted 8 lines · Greg Kroah-Hartman
Quoted 2 lines · Philipp Hortmann

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

Why rename something you are about to delete in a later patch?

thanks,

Quoted 8 lines
Quoted 1 line

greg k-h

I need to rename this variable. Does it matter if I do it in first or last patch? When doing it first I do not have to review so much checkpatch warnings.

Or is it OK to convert the macro to a function and rename the variable in one patch?

Convert and rename at the same time, that makes it easier and simpler overall.

thanks,

greg k-h