[PATCH 0/3] staging: vt6655: Convert multiline macro to static function

[PATCH 0/3] staging: vt6655: Convert multiline macro to static function

[Philipp Hortmann]

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

-- 
2.37.1

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

[Philipp Hortmann]

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

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

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

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

[Greg Kroah-Hartman]

On Wed, Jul 20, 2022 at 06:24:32PM +0200, Philipp Hortmann wrote:
> 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

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

[Philipp Hortmann]

On 7/27/22 08:47, Greg Kroah-Hartman wrote:
> On Wed, Jul 20, 2022 at 06:24:32PM +0200, Philipp Hortmann wrote:
>> 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

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

[Greg Kroah-Hartman]

On Wed, Jul 27, 2022 at 05:54:04PM +0200, Philipp Hortmann wrote:
> On 7/27/22 08:47, Greg Kroah-Hartman wrote:
> > On Wed, Jul 20, 2022 at 06:24:32PM +0200, Philipp Hortmann wrote:
> > > 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?

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

thanks,

greg k-h

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

[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>
---
 drivers/staging/vt6655/mac.c | 2 +-
 drivers/staging/vt6655/mac.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index a7f645f9ccf8..d7ee42df7062 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -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 */
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 837ae9dd6929..c2c9cb07f32d 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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);			\
-- 
2.37.1

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

[Greg Kroah-Hartman]

On Wed, Jul 20, 2022 at 06:24:36PM +0200, Philipp Hortmann wrote:
> 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

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

[Philipp Hortmann]

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

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

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index d7ee42df7062..3e9494c653dc 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -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
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index c2c9cb07f32d..467c599a3289 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -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)
 
-- 
2.37.1

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

[Greg Kroah-Hartman]

On Wed, Jul 20, 2022 at 06:24:43PM +0200, Philipp Hortmann wrote:
> Convert once used macro to static function.
> Multiline macros are not liked by kernel community.
> 
> Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
> ---
>  drivers/staging/vt6655/mac.c | 11 +++++++++++
>  drivers/staging/vt6655/mac.h |  8 --------
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
> index d7ee42df7062..3e9494c653dc 100644
> --- a/drivers/staging/vt6655/mac.c
> +++ b/drivers/staging/vt6655/mac.c
> @@ -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);
> +}
> +

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