[PATCH] staging: sm750fb: fix instances of camel case

linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] staging: sm750fb: fix instances of camel case
       [not found] <20250417153101.353645-1-rubenru09.ref@aol.com>
@ 2025-04-17 15:27 ` Ruben Wauters
  2025-04-17 16:58   ` Greg Korah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Ruben Wauters @ 2025-04-17 15:27 UTC (permalink / raw)
  To: Greg Korah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
  Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel

As per the kernel style guidelines, and as reported by checkpatch.pl,
replaced instances of camel case with snake_case where appropriate and
aligned names in the header with those in the c file.

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 drivers/staging/sm750fb/ddk750_sii164.c | 113 ++++++++++++------------
 drivers/staging/sm750fb/ddk750_sii164.h |  26 +++---
 2 files changed, 69 insertions(+), 70 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 89700fc5dd2e..20c2f386220c 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -12,11 +12,11 @@
 #define USE_HW_I2C
 
 #ifdef USE_HW_I2C
-    #define i2cWriteReg sm750_hw_i2c_write_reg
-    #define i2cReadReg  sm750_hw_i2c_read_reg
+    #define I2C_WRITE_REG sm750_hw_i2c_write_reg
+    #define I2C_READ_REG  sm750_hw_i2c_read_reg
 #else
-    #define i2cWriteReg sm750_sw_i2c_write_reg
-    #define i2cReadReg  sm750_sw_i2c_read_reg
+    #define I2C_WRITE_REG sm750_sw_i2c_write_reg
+    #define I2C_READ_REG  sm750_sw_i2c_read_reg
 #endif
 
 /* SII164 Vendor and Device ID */
@@ -25,7 +25,7 @@
 
 #ifdef SII164_FULL_FUNCTIONS
 /* Name of the DVI Controller chip */
-static char *gDviCtrlChipName = "Silicon Image SiI 164";
+static char *dvi_controller_chip_name = "Silicon Image SiI 164";
 #endif
 
 /*
@@ -37,14 +37,14 @@ static char *gDviCtrlChipName = "Silicon Image SiI 164";
  */
 unsigned short sii164_get_vendor_id(void)
 {
-	unsigned short vendorID;
+	unsigned short vendor;
 
-	vendorID = ((unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
+	vendor = ((unsigned short)I2C_READ_REG(SII164_I2C_ADDRESS,
 					       SII164_VENDOR_ID_HIGH) << 8) |
-		   (unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
-					      SII164_VENDOR_ID_LOW);
+		  (unsigned short)I2C_READ_REG(SII164_I2C_ADDRESS,
+					       SII164_VENDOR_ID_LOW);
 
-	return vendorID;
+	return vendor;
 }
 
 /*
@@ -56,14 +56,14 @@ unsigned short sii164_get_vendor_id(void)
  */
 unsigned short sii164_get_device_id(void)
 {
-	unsigned short device_id;
+	unsigned short device;
 
-	device_id = ((unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
+	device = ((unsigned short)I2C_READ_REG(SII164_I2C_ADDRESS,
 					       SII164_DEVICE_ID_HIGH) << 8) |
-		   (unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
-					      SII164_DEVICE_ID_LOW);
+		  (unsigned short)I2C_READ_REG(SII164_I2C_ADDRESS,
+					       SII164_DEVICE_ID_LOW);
 
-	return device_id;
+	return device;
 }
 
 /*
@@ -176,7 +176,7 @@ long sii164_init_chip(unsigned char edge_select,
 		else
 			config |= SII164_CONFIGURATION_VSYNC_AS_IS;
 
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+		I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
 
 		/*
 		 * De-skew enabled with default 111b value.
@@ -214,7 +214,7 @@ long sii164_init_chip(unsigned char edge_select,
 			config |= SII164_DESKEW_8_STEP;
 			break;
 		}
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_DESKEW, config);
+		I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_DESKEW, config);
 
 		/* Enable/Disable Continuous Sync. */
 		if (continuous_sync_enable == 0)
@@ -231,12 +231,12 @@ long sii164_init_chip(unsigned char edge_select,
 		/* Set the PLL Filter value */
 		config |= ((pll_filter_value & 0x07) << 1);
 
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_PLL, config);
+		I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_PLL, config);
 
 		/* Recover from Power Down and enable output. */
-		config = i2cReadReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
+		config = I2C_READ_REG(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
 		config |= SII164_CONFIGURATION_POWER_NORMAL;
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+		I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
 
 		return 0;
 	}
@@ -269,7 +269,7 @@ void sii164_reset_chip(void)
  */
 char *sii164_get_chip_string(void)
 {
-	return gDviCtrlChipName;
+	return dvi_controller_chip_name;
 }
 
 /*
@@ -277,55 +277,55 @@ char *sii164_get_chip_string(void)
  *      This function sets the power configuration of the DVI Controller Chip.
  *
  *  Input:
- *      powerUp - Flag to set the power down or up
+ *      power - Flag to set the power down or up
  */
-void sii164_set_power(unsigned char powerUp)
+void sii164_set_power(unsigned char power)
 {
 	unsigned char config;
 
-	config = i2cReadReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
-	if (powerUp == 1) {
+	config = I2C_READ_REG(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
+	if (power == 1) {
 		/* Power up the chip */
 		config &= ~SII164_CONFIGURATION_POWER_MASK;
 		config |= SII164_CONFIGURATION_POWER_NORMAL;
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+		I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
 	} else {
 		/* Power down the chip */
 		config &= ~SII164_CONFIGURATION_POWER_MASK;
 		config |= SII164_CONFIGURATION_POWER_DOWN;
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+		I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
 	}
 }
 
 /*
- *  sii164SelectHotPlugDetectionMode
+ *  sii164_select_hot_plug_detection_mode
  *      This function selects the mode of the hot plug detection.
  */
 static
-void sii164SelectHotPlugDetectionMode(enum sii164_hot_plug_mode hotPlugMode)
+void sii164_select_hot_plug_detection_mode(enum sii164_hot_plug_mode hot_plug_mode)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
+	detect_reg = I2C_READ_REG(SII164_I2C_ADDRESS, SII164_DETECT) &
 		    ~SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG;
-	switch (hotPlugMode) {
+	switch (hot_plug_mode) {
 	case SII164_HOTPLUG_DISABLE:
-		detectReg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_HIGH;
+		detect_reg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_HIGH;
 		break;
 	case SII164_HOTPLUG_USE_MDI:
-		detectReg &= ~SII164_DETECT_INTERRUPT_MASK;
-		detectReg |= SII164_DETECT_INTERRUPT_BY_HTPLG_PIN;
-		detectReg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_MDI;
+		detect_reg &= ~SII164_DETECT_INTERRUPT_MASK;
+		detect_reg |= SII164_DETECT_INTERRUPT_BY_HTPLG_PIN;
+		detect_reg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_MDI;
 		break;
 	case SII164_HOTPLUG_USE_RSEN:
-		detectReg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_RSEN;
+		detect_reg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_RSEN;
 		break;
 	case SII164_HOTPLUG_USE_HTPLG:
-		detectReg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_HTPLG;
+		detect_reg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_HTPLG;
 		break;
 	}
 
-	i2cWriteReg(SII164_I2C_ADDRESS, SII164_DETECT, detectReg);
+	I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_DETECT, detect_reg);
 }
 
 /*
@@ -336,17 +336,18 @@ void sii164SelectHotPlugDetectionMode(enum sii164_hot_plug_mode hotPlugMode)
  */
 void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT);
+	detect_reg = I2C_READ_REG(SII164_I2C_ADDRESS, SII164_DETECT);
 
-	/* Depending on each DVI controller, need to enable the hot plug based
+	/*
+	 * Depending on each DVI controller, need to enable the hot plug based
 	 * on each individual chip design.
 	 */
 	if (enable_hot_plug != 0)
-		sii164SelectHotPlugDetectionMode(SII164_HOTPLUG_USE_MDI);
+		sii164_select_hot_plug_detection_mode(SII164_HOTPLUG_USE_MDI);
 	else
-		sii164SelectHotPlugDetectionMode(SII164_HOTPLUG_DISABLE);
+		sii164_select_hot_plug_detection_mode(SII164_HOTPLUG_DISABLE);
 }
 
 /*
@@ -359,11 +360,11 @@ void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug)
  */
 unsigned char sii164_is_connected(void)
 {
-	unsigned char hotPlugValue;
+	unsigned char hot_plug;
 
-	hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
-		       SII164_DETECT_HOT_PLUG_STATUS_MASK;
-	if (hotPlugValue == SII164_DETECT_HOT_PLUG_STATUS_ON)
+	hot_plug = I2C_READ_REG(SII164_I2C_ADDRESS, SII164_DETECT) &
+				SII164_DETECT_HOT_PLUG_STATUS_MASK;
+	if (hot_plug == SII164_DETECT_HOT_PLUG_STATUS_ON)
 		return 1;
 	else
 		return 0;
@@ -379,11 +380,11 @@ unsigned char sii164_is_connected(void)
  */
 unsigned char sii164_check_interrupt(void)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
-		    SII164_DETECT_MONITOR_STATE_MASK;
-	if (detectReg == SII164_DETECT_MONITOR_STATE_CHANGE)
+	detect_reg = I2C_READ_REG(SII164_I2C_ADDRESS, SII164_DETECT) &
+				  SII164_DETECT_MONITOR_STATE_MASK;
+	if (detect_reg == SII164_DETECT_MONITOR_STATE_CHANGE)
 		return 1;
 	else
 		return 0;
@@ -395,12 +396,12 @@ unsigned char sii164_check_interrupt(void)
  */
 void sii164_clear_interrupt(void)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
 	/* Clear the MDI interrupt */
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT);
-	i2cWriteReg(SII164_I2C_ADDRESS, SII164_DETECT,
-		    detectReg | SII164_DETECT_MONITOR_STATE_CLEAR);
+	detect_reg = I2C_READ_REG(SII164_I2C_ADDRESS, SII164_DETECT);
+	I2C_WRITE_REG(SII164_I2C_ADDRESS, SII164_DETECT,
+		      detect_reg | SII164_DETECT_MONITOR_STATE_CLEAR);
 }
 
 #endif
diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
index ebc173658f0e..028327816f7f 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.h
+++ b/drivers/staging/sm750fb/ddk750_sii164.h
@@ -16,16 +16,16 @@ enum sii164_hot_plug_mode {
 };
 
 /* Silicon Image SiI164 chip prototype */
-long sii164_init_chip(unsigned char edgeSelect,
-		      unsigned char busSelect,
-		      unsigned char dualEdgeClkSelect,
-		      unsigned char hsyncEnable,
-		      unsigned char vsyncEnable,
-		      unsigned char deskewEnable,
-		      unsigned char deskewSetting,
-		      unsigned char continuousSyncEnable,
-		      unsigned char pllFilterEnable,
-		      unsigned char pllFilterValue);
+long sii164_init_chip(unsigned char edge_select,
+		      unsigned char bus_select,
+		      unsigned char dual_edge_clk_select,
+		      unsigned char hsync_enable,
+		      unsigned char vsync_enable,
+		      unsigned char deskew_enable,
+		      unsigned char deskew_setting,
+		      unsigned char continuous_sync_enable,
+		      unsigned char pll_filter_enable,
+		      unsigned char pll_filter_value);
 
 unsigned short sii164_get_vendor_id(void);
 unsigned short sii164_get_device_id(void);
@@ -33,7 +33,7 @@ unsigned short sii164_get_device_id(void);
 #ifdef SII164_FULL_FUNCTIONS
 void sii164_reset_chip(void);
 char *sii164_get_chip_string(void);
-void sii164_set_power(unsigned char powerUp);
+void sii164_set_power(unsigned char power);
 void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug);
 unsigned char sii164_is_connected(void);
 unsigned char sii164_check_interrupt(void);
@@ -96,9 +96,7 @@ void sii164_clear_interrupt(void);
 #define SII164_CONFIGURATION_VSYNC_FORCE_LOW        0x00
 #define SII164_CONFIGURATION_VSYNC_AS_IS            0x20
 
-/*
- * Detection registers
- */
+/* Detection registers */
 #define SII164_DETECT                               0x09
 
 /* Monitor Detect Interrupt (MDI) */
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] staging: sm750fb: fix instances of camel case
  2025-04-17 15:27 ` [PATCH] staging: sm750fb: fix instances of camel case Ruben Wauters
@ 2025-04-17 16:58   ` Greg Korah-Hartman
  2025-04-17 17:28     ` Ruben Wauters
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Korah-Hartman @ 2025-04-17 16:58 UTC (permalink / raw)
  To: Ruben Wauters
  Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
	linux-staging, linux-kernel

On Thu, Apr 17, 2025 at 04:27:47PM +0100, Ruben Wauters wrote:
> As per the kernel style guidelines, and as reported by checkpatch.pl,
> replaced instances of camel case with snake_case where appropriate and
> aligned names in the header with those in the c file.
> 
> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 113 ++++++++++++------------
>  drivers/staging/sm750fb/ddk750_sii164.h |  26 +++---
>  2 files changed, 69 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> index 89700fc5dd2e..20c2f386220c 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -12,11 +12,11 @@
>  #define USE_HW_I2C
>  
>  #ifdef USE_HW_I2C
> -    #define i2cWriteReg sm750_hw_i2c_write_reg
> -    #define i2cReadReg  sm750_hw_i2c_read_reg
> +    #define I2C_WRITE_REG sm750_hw_i2c_write_reg
> +    #define I2C_READ_REG  sm750_hw_i2c_read_reg

Close, but these are really a function name, not a macro, right?

And what sets this #define?  If it's always enabled, then unwrap this
indirection instead of keeping it around

>  #else
> -    #define i2cWriteReg sm750_sw_i2c_write_reg
> -    #define i2cReadReg  sm750_sw_i2c_read_reg
> +    #define I2C_WRITE_REG sm750_sw_i2c_write_reg
> +    #define I2C_READ_REG  sm750_sw_i2c_read_reg
>  #endif
>  
>  /* SII164 Vendor and Device ID */
> @@ -25,7 +25,7 @@
>  
>  #ifdef SII164_FULL_FUNCTIONS
>  /* Name of the DVI Controller chip */
> -static char *gDviCtrlChipName = "Silicon Image SiI 164";
> +static char *dvi_controller_chip_name = "Silicon Image SiI 164";

This is a totally different thing.

>  #endif
>  
>  /*
> @@ -37,14 +37,14 @@ static char *gDviCtrlChipName = "Silicon Image SiI 164";
>   */
>  unsigned short sii164_get_vendor_id(void)
>  {
> -	unsigned short vendorID;
> +	unsigned short vendor;

Why change this?

This is a mix of lots of different changes, please break things up into
"one logical change per patch"

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] staging: sm750fb: fix instances of camel case
  2025-04-17 16:58   ` Greg Korah-Hartman
@ 2025-04-17 17:28     ` Ruben Wauters
  2025-04-17 17:46       ` Greg Korah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Ruben Wauters @ 2025-04-17 17:28 UTC (permalink / raw)
  To: Greg Korah-Hartman
  Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
	linux-staging, linux-kernel

On Thu, 2025-04-17 at 18:58 +0200, Greg Korah-Hartman wrote:
> On Thu, Apr 17, 2025 at 04:27:47PM +0100, Ruben Wauters wrote:
> > As per the kernel style guidelines, and as reported by
> > checkpatch.pl,
> > replaced instances of camel case with snake_case where appropriate
> > and
> > aligned names in the header with those in the c file.
> > 
> > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > ---
> >  drivers/staging/sm750fb/ddk750_sii164.c | 113 ++++++++++++--------
> > ----
> >  drivers/staging/sm750fb/ddk750_sii164.h |  26 +++---
> >  2 files changed, 69 insertions(+), 70 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/ddk750_sii164.c
> > b/drivers/staging/sm750fb/ddk750_sii164.c
> > index 89700fc5dd2e..20c2f386220c 100644
> > --- a/drivers/staging/sm750fb/ddk750_sii164.c
> > +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> > @@ -12,11 +12,11 @@
> >  #define USE_HW_I2C
> >  
> >  #ifdef USE_HW_I2C
> > -    #define i2cWriteReg sm750_hw_i2c_write_reg
> > -    #define i2cReadReg  sm750_hw_i2c_read_reg
> > +    #define I2C_WRITE_REG sm750_hw_i2c_write_reg
> > +    #define I2C_READ_REG  sm750_hw_i2c_read_reg
> 
> Close, but these are really a function name, not a macro, right?
> 
> And what sets this #define?  If it's always enabled, then unwrap this
> indirection instead of keeping it around

Will take a look into it, if it turns out that this is in fact actually
used/different, what would be the best way to name this? checkpatch.pl
doesn't like the camelCase that's currently there.

> >  #else
> > -    #define i2cWriteReg sm750_sw_i2c_write_reg
> > -    #define i2cReadReg  sm750_sw_i2c_read_reg
> > +    #define I2C_WRITE_REG sm750_sw_i2c_write_reg
> > +    #define I2C_READ_REG  sm750_sw_i2c_read_reg
> >  #endif
> >  
> >  /* SII164 Vendor and Device ID */
> > @@ -25,7 +25,7 @@
> >  
> >  #ifdef SII164_FULL_FUNCTIONS
> >  /* Name of the DVI Controller chip */
> > -static char *gDviCtrlChipName = "Silicon Image SiI 164";
> > +static char *dvi_controller_chip_name = "Silicon Image SiI 164";
> 
> This is a totally different thing.

It is, however I believe it is somewhat more descriptive, I suppose it
doesn't really matter though and if it should be the same, just made
snake_case, I can do that.
> 
> >  #endif
> >  
> >  /*
> > @@ -37,14 +37,14 @@ static char *gDviCtrlChipName = "Silicon Image
> > SiI 164";
> >   */
> >  unsigned short sii164_get_vendor_id(void)
> >  {
> > -	unsigned short vendorID;
> > +	unsigned short vendor;
> 
> Why change this?

Again removing camelCase, kernel style guide says that shorter names
are preferred (unless I misinterpreted that), I could make it vendor_id
if that is preferred, I believe the same would be with device_id below
it.

> This is a mix of lots of different changes, please break things up
> into
> "one logical change per patch"

Would it be best to split each rename (function or variable) into a
separate patch? I do agree it is quite a lot and I was a little unsure
when sending this one, but I also don't want to make a lot of different
patches and spam your email with 100 patches at once.

I suppose I could make one rename per patch and do them a few at a
time, that may take longer though.
> 
> thanks,
> 
> greg k-h


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] staging: sm750fb: fix instances of camel case
  2025-04-17 17:28     ` Ruben Wauters
@ 2025-04-17 17:46       ` Greg Korah-Hartman
  0 siblings, 0 replies; 4+ messages in thread
From: Greg Korah-Hartman @ 2025-04-17 17:46 UTC (permalink / raw)
  To: Ruben Wauters
  Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
	linux-staging, linux-kernel

On Thu, Apr 17, 2025 at 06:28:07PM +0100, Ruben Wauters wrote:
> On Thu, 2025-04-17 at 18:58 +0200, Greg Korah-Hartman wrote:
> > On Thu, Apr 17, 2025 at 04:27:47PM +0100, Ruben Wauters wrote:
> > > As per the kernel style guidelines, and as reported by
> > > checkpatch.pl,
> > > replaced instances of camel case with snake_case where appropriate
> > > and
> > > aligned names in the header with those in the c file.
> > > 
> > > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > > ---
> > >  drivers/staging/sm750fb/ddk750_sii164.c | 113 ++++++++++++--------
> > > ----
> > >  drivers/staging/sm750fb/ddk750_sii164.h |  26 +++---
> > >  2 files changed, 69 insertions(+), 70 deletions(-)
> > > 
> > > diff --git a/drivers/staging/sm750fb/ddk750_sii164.c
> > > b/drivers/staging/sm750fb/ddk750_sii164.c
> > > index 89700fc5dd2e..20c2f386220c 100644
> > > --- a/drivers/staging/sm750fb/ddk750_sii164.c
> > > +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> > > @@ -12,11 +12,11 @@
> > >  #define USE_HW_I2C
> > >  
> > >  #ifdef USE_HW_I2C
> > > -    #define i2cWriteReg sm750_hw_i2c_write_reg
> > > -    #define i2cReadReg  sm750_hw_i2c_read_reg
> > > +    #define I2C_WRITE_REG sm750_hw_i2c_write_reg
> > > +    #define I2C_READ_REG  sm750_hw_i2c_read_reg
> > 
> > Close, but these are really a function name, not a macro, right?
> > 
> > And what sets this #define?  If it's always enabled, then unwrap this
> > indirection instead of keeping it around
> 
> Will take a look into it, if it turns out that this is in fact actually
> used/different, what would be the best way to name this? checkpatch.pl
> doesn't like the camelCase that's currently there.
> 
> > >  #else
> > > -    #define i2cWriteReg sm750_sw_i2c_write_reg
> > > -    #define i2cReadReg  sm750_sw_i2c_read_reg
> > > +    #define I2C_WRITE_REG sm750_sw_i2c_write_reg
> > > +    #define I2C_READ_REG  sm750_sw_i2c_read_reg
> > >  #endif
> > >  
> > >  /* SII164 Vendor and Device ID */
> > > @@ -25,7 +25,7 @@
> > >  
> > >  #ifdef SII164_FULL_FUNCTIONS
> > >  /* Name of the DVI Controller chip */
> > > -static char *gDviCtrlChipName = "Silicon Image SiI 164";
> > > +static char *dvi_controller_chip_name = "Silicon Image SiI 164";
> > 
> > This is a totally different thing.
> 
> It is, however I believe it is somewhat more descriptive, I suppose it
> doesn't really matter though and if it should be the same, just made
> snake_case, I can do that.
> > 
> > >  #endif
> > >  
> > >  /*
> > > @@ -37,14 +37,14 @@ static char *gDviCtrlChipName = "Silicon Image
> > > SiI 164";
> > >   */
> > >  unsigned short sii164_get_vendor_id(void)
> > >  {
> > > -	unsigned short vendorID;
> > > +	unsigned short vendor;
> > 
> > Why change this?
> 
> Again removing camelCase, kernel style guide says that shorter names
> are preferred (unless I misinterpreted that), I could make it vendor_id
> if that is preferred, I believe the same would be with device_id below
> it.
> 
> > This is a mix of lots of different changes, please break things up
> > into
> > "one logical change per patch"
> 
> Would it be best to split each rename (function or variable) into a
> separate patch? I do agree it is quite a lot and I was a little unsure
> when sending this one, but I also don't want to make a lot of different
> patches and spam your email with 100 patches at once.

Do "one logical thing at a time".  Think about what you would want to
see if you were the reviewer.  Sometimes yes, 100 patches is fine, but
really, make it in smaller chunks as odds are something is going to go
wrong with that many at once.

See all of the other patches on the list for examples, you have
thousands to learn from :)

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-04-17 17:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250417153101.353645-1-rubenru09.ref@aol.com>
2025-04-17 15:27 ` [PATCH] staging: sm750fb: fix instances of camel case Ruben Wauters
2025-04-17 16:58   ` Greg Korah-Hartman
2025-04-17 17:28     ` Ruben Wauters
2025-04-17 17:46       ` Greg Korah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).