[PATCH 0/8] staging: sm750fb: cleanup ddk750_sii164

[PATCH 0/8] staging: sm750fb: cleanup ddk750_sii164

[Ruben Wauters]

This patch series fixes errors shown by checkpatch.pl in
ddk750_sii164.c and .h

The changes are mostly camelCase to snake_case changes,
however include a variable rename to a clearer name
and removal of the USE_HW_I2C check.

This patch series is created from spliting an earlier
patch into smaller, more logical changes.

The patches will need to be applied in order.

I have included details of the latter within the patch
description itself.

It is also worth noting that I do not have the hardware in
question, so I am unable to test whether any functional
changes would be better served one way or another.

Thank you

Ruben

Ruben Wauters (8):
  staging: sm250fb: remove USE_HW_I2C check
  staging: sm750fb: rename gDviCtrlChipName
  staging: sm750fb: rename vendorID to vendor_id
  staging: sm750fb: rename sii164_init_chip params
  staging: sm750fb: rename sii164_set_power's param
  staging: sm750fb: rename sii164SelectHotPlugDetectionMode
  staging: sm750fb: rename detectReg to detect_reg
  staging: sm750fb: rename hotPlugValue to hot_plug_value

 drivers/staging/sm750fb/ddk750_sii164.c | 119 +++++++++++-------------
 drivers/staging/sm750fb/ddk750_sii164.h |  22 ++---
 2 files changed, 63 insertions(+), 78 deletions(-)

-- 
2.45.2

[PATCH 1/8] staging: sm250fb: remove USE_HW_I2C check

[Ruben Wauters]

Removes the USE_HW_I2C check and function defines in
ddk750_sii164.c.

The software equivalents were never used due to
USE_HW_I2C being defined just before the ifdef, meaning
the hardware versions were always used.

The define names were also triggering checkpatch.pl's
camel case check.

Signed-off-by: Ruben Wauters <rubenru09@aol.com>

---

I am somewhat unsure whether this is the way to go or
the correct way would be to add an option/opportunity for
the software version to be used. Currently the hardware
version is always used, but I am unsure if there ever even
would be a case where you would want to use the software
version over the hardware version.

I do not have the hardware in question so I cannot test
what the difference between the two versions exactly is.

I also note that the removal is mentioned in the TODO,
however once again this is currently hardcoded.
---
 drivers/staging/sm750fb/ddk750_sii164.c | 63 ++++++++++---------------
 1 file changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 89700fc5dd2e..dd7811b18bf6 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -8,17 +8,6 @@
 /* I2C Address of each SII164 chip */
 #define SII164_I2C_ADDRESS                  0x70
 
-/* Define this definition to use hardware i2c. */
-#define USE_HW_I2C
-
-#ifdef USE_HW_I2C
-    #define i2cWriteReg sm750_hw_i2c_write_reg
-    #define i2cReadReg  sm750_hw_i2c_read_reg
-#else
-    #define i2cWriteReg sm750_sw_i2c_write_reg
-    #define i2cReadReg  sm750_sw_i2c_read_reg
-#endif
-
 /* SII164 Vendor and Device ID */
 #define SII164_VENDOR_ID                    0x0001
 #define SII164_DEVICE_ID                    0x0006
@@ -39,10 +28,10 @@ unsigned short sii164_get_vendor_id(void)
 {
 	unsigned short vendorID;
 
-	vendorID = ((unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
-					       SII164_VENDOR_ID_HIGH) << 8) |
-		   (unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
-					      SII164_VENDOR_ID_LOW);
+	vendorID = ((unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
+							  SII164_VENDOR_ID_HIGH) << 8) |
+		   (unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
+							 SII164_VENDOR_ID_LOW);
 
 	return vendorID;
 }
@@ -58,10 +47,10 @@ unsigned short sii164_get_device_id(void)
 {
 	unsigned short device_id;
 
-	device_id = ((unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
-					       SII164_DEVICE_ID_HIGH) << 8) |
-		   (unsigned short)i2cReadReg(SII164_I2C_ADDRESS,
-					      SII164_DEVICE_ID_LOW);
+	device_id = ((unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
+							   SII164_DEVICE_ID_HIGH) << 8) |
+		   (unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
+							 SII164_DEVICE_ID_LOW);
 
 	return device_id;
 }
@@ -132,12 +121,8 @@ long sii164_init_chip(unsigned char edge_select,
 	unsigned char config;
 
 	/* Initialize the i2c bus */
-#ifdef USE_HW_I2C
 	/* Use fast mode. */
 	sm750_hw_i2c_init(1);
-#else
-	sm750_sw_i2c_init(DEFAULT_I2C_SCL, DEFAULT_I2C_SDA);
-#endif
 
 	/* Check if SII164 Chip exists */
 	if ((sii164_get_vendor_id() == SII164_VENDOR_ID) &&
@@ -176,7 +161,7 @@ long sii164_init_chip(unsigned char edge_select,
 		else
 			config |= SII164_CONFIGURATION_VSYNC_AS_IS;
 
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+		sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
 
 		/*
 		 * De-skew enabled with default 111b value.
@@ -214,7 +199,7 @@ long sii164_init_chip(unsigned char edge_select,
 			config |= SII164_DESKEW_8_STEP;
 			break;
 		}
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_DESKEW, config);
+		sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_DESKEW, config);
 
 		/* Enable/Disable Continuous Sync. */
 		if (continuous_sync_enable == 0)
@@ -231,12 +216,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);
+		sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_PLL, config);
 
 		/* Recover from Power Down and enable output. */
-		config = i2cReadReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
+		config = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
 		config |= SII164_CONFIGURATION_POWER_NORMAL;
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+		sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
 
 		return 0;
 	}
@@ -283,17 +268,17 @@ void sii164_set_power(unsigned char powerUp)
 {
 	unsigned char config;
 
-	config = i2cReadReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
+	config = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
 	if (powerUp == 1) {
 		/* Power up the chip */
 		config &= ~SII164_CONFIGURATION_POWER_MASK;
 		config |= SII164_CONFIGURATION_POWER_NORMAL;
-		i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+		sm750_hw_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);
+		sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
 	}
 }
 
@@ -306,7 +291,7 @@ void sii164SelectHotPlugDetectionMode(enum sii164_hot_plug_mode hotPlugMode)
 {
 	unsigned char detectReg;
 
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
+	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
 		    ~SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG;
 	switch (hotPlugMode) {
 	case SII164_HOTPLUG_DISABLE:
@@ -325,7 +310,7 @@ void sii164SelectHotPlugDetectionMode(enum sii164_hot_plug_mode hotPlugMode)
 		break;
 	}
 
-	i2cWriteReg(SII164_I2C_ADDRESS, SII164_DETECT, detectReg);
+	sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_DETECT, detectReg);
 }
 
 /*
@@ -338,7 +323,7 @@ void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug)
 {
 	unsigned char detectReg;
 
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT);
+	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT);
 
 	/* Depending on each DVI controller, need to enable the hot plug based
 	 * on each individual chip design.
@@ -361,7 +346,7 @@ unsigned char sii164_is_connected(void)
 {
 	unsigned char hotPlugValue;
 
-	hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
+	hotPlugValue = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
 		       SII164_DETECT_HOT_PLUG_STATUS_MASK;
 	if (hotPlugValue == SII164_DETECT_HOT_PLUG_STATUS_ON)
 		return 1;
@@ -381,7 +366,7 @@ unsigned char sii164_check_interrupt(void)
 {
 	unsigned char detectReg;
 
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
+	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
 		    SII164_DETECT_MONITOR_STATE_MASK;
 	if (detectReg == SII164_DETECT_MONITOR_STATE_CHANGE)
 		return 1;
@@ -398,9 +383,9 @@ void sii164_clear_interrupt(void)
 	unsigned char detectReg;
 
 	/* Clear the MDI interrupt */
-	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT);
-	i2cWriteReg(SII164_I2C_ADDRESS, SII164_DETECT,
-		    detectReg | SII164_DETECT_MONITOR_STATE_CLEAR);
+	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT);
+	sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_DETECT,
+			       detectReg | SII164_DETECT_MONITOR_STATE_CLEAR);
 }
 
 #endif
-- 
2.45.2

Re: [PATCH 1/8] staging: sm250fb: remove USE_HW_I2C check

[Greg Kroah-Hartman]

On Thu, Apr 17, 2025 at 08:02:49PM +0100, Ruben Wauters wrote:
> Removes the USE_HW_I2C check and function defines in
> ddk750_sii164.c.
> 
> The software equivalents were never used due to
> USE_HW_I2C being defined just before the ifdef, meaning
> the hardware versions were always used.
> 
> The define names were also triggering checkpatch.pl's
> camel case check.
> 
> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> 
> ---
> 
> I am somewhat unsure whether this is the way to go or
> the correct way would be to add an option/opportunity for
> the software version to be used. Currently the hardware
> version is always used, but I am unsure if there ever even
> would be a case where you would want to use the software
> version over the hardware version.

Then the code can be added back, not an issue.

But you forgot this same check in
drivers/staging/sm750fb/ddk750_hwi2c.c, right?

Also, what about removing the sm750_sw_i2c_write_reg() and other
functions that are now never referenced?  Can you add that to this patch
series?  A single series that just removes the use of USE_HW_I2C and the
now unneeded functions would be best, as it's not really a "coding
style" fix, but rather a code cleanup thing, right?

thanks,

greg k-h

Re: [PATCH 1/8] staging: sm250fb: remove USE_HW_I2C check

[Ruben Wauters]

On Fri, 2025-04-18 at 12:33 +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 17, 2025 at 08:02:49PM +0100, Ruben Wauters wrote:
> > Removes the USE_HW_I2C check and function defines in
> > ddk750_sii164.c.
> > 
> > The software equivalents were never used due to
> > USE_HW_I2C being defined just before the ifdef, meaning
> > the hardware versions were always used.
> > 
> > The define names were also triggering checkpatch.pl's
> > camel case check.
> > 
> > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > 
> > ---
> > 
> > I am somewhat unsure whether this is the way to go or
> > the correct way would be to add an option/opportunity for
> > the software version to be used. Currently the hardware
> > version is always used, but I am unsure if there ever even
> > would be a case where you would want to use the software
> > version over the hardware version.
> 
> Then the code can be added back, not an issue.
> 
> But you forgot this same check in
> drivers/staging/sm750fb/ddk750_hwi2c.c, right?

This check can indeed be removed I suppose, might be worth also
removing the USE_DVICHIP checks also, especially when defined

There's also a USE_HW_I2C check in ddk750.h, which defines which header
to use, however I'm somewhat unsure exactly what the best way to go
about addressing that is, since it's not defined before including it
does make me wonder whether the HW files are even used at all.

Something about this seems entirely wrong to me, again I don't have the
hardware to test, but why would SW files be used when the HW files work
fine? Is it intended that the HW files are used instead? it's a bit
inconsistent, especially since in ddk750_sii164.c the HW ones are
explicitly used over the SW ones
Why is the SW files preferred in sm750_hw.c then?
I believe this is something of an oversight and the files should
probably use the HW ones instead of the SW ones.

I am curious to know your thoughts on this (and anyone else's)

> Also, what about removing the sm750_sw_i2c_write_reg() and other
> functions that are now never referenced?  Can you add that to this
> patch
> series?  A single series that just removes the use of USE_HW_I2C and
> the
> now unneeded functions would be best, as it's not really a "coding
> style" fix, but rather a code cleanup thing, right?

I will create a separate patch series for removing unneeded code, since
it does look like a lot more code removal might be needed than I
originally thought.

> thanks,
> 
> greg k-h

[PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName

[Ruben Wauters]

Renames gDviCtrlChipName to dvi_controller_chip_name
This fixes checkpatch.pl's camel case check.

Signed-off-by: Ruben Wauters <rubenru09@aol.com>

---

I changed the name to dvi_controller_chip_name as I
believe it is somewhat more descriptive than
g_dvi_ctrl_chip_name. If the second one is wanted instead
please let me know and I will change it
---
 drivers/staging/sm750fb/ddk750_sii164.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index dd7811b18bf6..d4309e0d807f 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -14,7 +14,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
 
 /*
@@ -254,7 +254,7 @@ void sii164_reset_chip(void)
  */
 char *sii164_get_chip_string(void)
 {
-	return gDviCtrlChipName;
+	return dvi_controller_chip_name;
 }
 
 /*
-- 
2.45.2

Re: [PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName

[Greg Kroah-Hartman]

On Thu, Apr 17, 2025 at 08:02:50PM +0100, Ruben Wauters wrote:
> Renames gDviCtrlChipName to dvi_controller_chip_name
> This fixes checkpatch.pl's camel case check.
> 
> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> 
> ---
> 
> I changed the name to dvi_controller_chip_name as I
> believe it is somewhat more descriptive than
> g_dvi_ctrl_chip_name. If the second one is wanted instead
> please let me know and I will change it
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> index dd7811b18bf6..d4309e0d807f 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -14,7 +14,7 @@
>  
>  #ifdef SII164_FULL_FUNCTIONS

This is never defined, so instead of papering over variable names that
are crazy, why not just remove all of the code in the blocks for this
define entirely?

thanks,

greg k-h

Re: [PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName

[Ruben Wauters]

On Fri, 2025-04-18 at 12:36 +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 17, 2025 at 08:02:50PM +0100, Ruben Wauters wrote:
> > Renames gDviCtrlChipName to dvi_controller_chip_name
> > This fixes checkpatch.pl's camel case check.
> > 
> > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > 
> > ---
> > 
> > I changed the name to dvi_controller_chip_name as I
> > believe it is somewhat more descriptive than
> > g_dvi_ctrl_chip_name. If the second one is wanted instead
> > please let me know and I will change it
> > ---
> >  drivers/staging/sm750fb/ddk750_sii164.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/ddk750_sii164.c
> > b/drivers/staging/sm750fb/ddk750_sii164.c
> > index dd7811b18bf6..d4309e0d807f 100644
> > --- a/drivers/staging/sm750fb/ddk750_sii164.c
> > +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> > @@ -14,7 +14,7 @@
> >  
> >  #ifdef SII164_FULL_FUNCTIONS
> 
> This is never defined, so instead of papering over variable names
> that
> are crazy, why not just remove all of the code in the blocks for this
> define entirely?

Given the amount of code that is never used and the time went into
writing this, it does make me wonder whether this code *should* be used
instead of being removed. I don't know exactly how it would be
integrated however, removal as of now might be the easiest option, but
I'm not entirely sure whether it would be the best option in terms of
functionality.

> thanks,
> 
> greg k-h

Re: [PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName

[Greg Kroah-Hartman]

On Fri, Apr 18, 2025 at 12:45:28PM +0100, Ruben Wauters wrote:
> On Fri, 2025-04-18 at 12:36 +0200, Greg Kroah-Hartman wrote:
> > On Thu, Apr 17, 2025 at 08:02:50PM +0100, Ruben Wauters wrote:
> > > Renames gDviCtrlChipName to dvi_controller_chip_name
> > > This fixes checkpatch.pl's camel case check.
> > > 
> > > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > > 
> > > ---
> > > 
> > > I changed the name to dvi_controller_chip_name as I
> > > believe it is somewhat more descriptive than
> > > g_dvi_ctrl_chip_name. If the second one is wanted instead
> > > please let me know and I will change it
> > > ---
> > >  drivers/staging/sm750fb/ddk750_sii164.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/sm750fb/ddk750_sii164.c
> > > b/drivers/staging/sm750fb/ddk750_sii164.c
> > > index dd7811b18bf6..d4309e0d807f 100644
> > > --- a/drivers/staging/sm750fb/ddk750_sii164.c
> > > +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> > > @@ -14,7 +14,7 @@
> > >  
> > >  #ifdef SII164_FULL_FUNCTIONS
> > 
> > This is never defined, so instead of papering over variable names
> > that
> > are crazy, why not just remove all of the code in the blocks for this
> > define entirely?
> 
> Given the amount of code that is never used and the time went into
> writing this, it does make me wonder whether this code *should* be used
> instead of being removed. I don't know exactly how it would be
> integrated however, removal as of now might be the easiest option, but
> I'm not entirely sure whether it would be the best option in terms of
> functionality.

Just remove it, odds are it was written a long time ago for other
hardware.  If someone needs it in the future, the git history has it
there for their use.

thanks,

greg k-h

[PATCH 3/8] staging: sm750fb: rename vendorID to vendor_id

[Ruben Wauters]

Fixes camel case check reported by checkpatch.pl

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 drivers/staging/sm750fb/ddk750_sii164.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index d4309e0d807f..9f660a9be5d6 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -26,14 +26,14 @@ static char *dvi_controller_chip_name = "Silicon Image SiI 164";
  */
 unsigned short sii164_get_vendor_id(void)
 {
-	unsigned short vendorID;
+	unsigned short vendor_id;
 
-	vendorID = ((unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
+	vendor_id = ((unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
 							  SII164_VENDOR_ID_HIGH) << 8) |
-		   (unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
-							 SII164_VENDOR_ID_LOW);
+		    (unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
+							  SII164_VENDOR_ID_LOW);
 
-	return vendorID;
+	return vendor_id;
 }
 
 /*
-- 
2.45.2

Re: [PATCH 3/8] staging: sm750fb: rename vendorID to vendor_id

[Greg Kroah-Hartman]

On Thu, Apr 17, 2025 at 08:02:51PM +0100, Ruben Wauters wrote:
> Fixes camel case check reported by checkpatch.pl
> 
> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> index d4309e0d807f..9f660a9be5d6 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -26,14 +26,14 @@ static char *dvi_controller_chip_name = "Silicon Image SiI 164";
>   */
>  unsigned short sii164_get_vendor_id(void)
>  {
> -	unsigned short vendorID;
> +	unsigned short vendor_id;
>  
> -	vendorID = ((unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
> +	vendor_id = ((unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
>  							  SII164_VENDOR_ID_HIGH) << 8) |
> -		   (unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
> -							 SII164_VENDOR_ID_LOW);
> +		    (unsigned short)sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS,
> +							  SII164_VENDOR_ID_LOW);
>  
> -	return vendorID;
> +	return vendor_id;

Why is the temporary variable needed at all?  Why not just return the
value directly?

thanks,

greg k-h

[PATCH 4/8] staging: sm750fb: rename sii164_init_chip params

[Ruben Wauters]

Renames sii164_init_chip's params in the header
file to be identical to the params in the c file.

This also fixes (one of) checkpatch.pl's camel case checks

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 drivers/staging/sm750fb/ddk750_sii164.h | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
index ebc173658f0e..465631d3868a 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);
-- 
2.45.2

[PATCH 5/8] staging: sm750fb: rename sii164_set_power's param

[Ruben Wauters]

Renames sii164_set_power's param from powerUp to power

This fixes checkpatch.pl's camel case check

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

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 9f660a9be5d6..e2da110fab81 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -262,14 +262,14 @@ 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 = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
-	if (powerUp == 1) {
+	if (power == 1) {
 		/* Power up the chip */
 		config &= ~SII164_CONFIGURATION_POWER_MASK;
 		config |= SII164_CONFIGURATION_POWER_NORMAL;
diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
index 465631d3868a..1730c2116b72 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.h
+++ b/drivers/staging/sm750fb/ddk750_sii164.h
@@ -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);
-- 
2.45.2

Re: [PATCH 5/8] staging: sm750fb: rename sii164_set_power's param

[Greg Kroah-Hartman]

On Thu, Apr 17, 2025 at 08:02:53PM +0100, Ruben Wauters wrote:
> Renames sii164_set_power's param from powerUp to power
> 
> This fixes checkpatch.pl's camel case check
> 
> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 6 +++---
>  drivers/staging/sm750fb/ddk750_sii164.h | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> index 9f660a9be5d6..e2da110fab81 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -262,14 +262,14 @@ 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

But now we don't know if it's "up" or "down", right?  Why not pick
"power_up"?

And shouldn't this be a boolean, and not an unsigned char?

thanks,

greg k-h

[PATCH 6/8] staging: sm750fb: rename sii164SelectHotPlugDetectionMode

[Ruben Wauters]

Renames sii164SelectHotPlugDetectionMode to
sii164_select_hot_plug_detection_mode, and the param
hotPlugMode to hot_plug_mode.

This fixes checkpatch.pl's camel case check.

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 drivers/staging/sm750fb/ddk750_sii164.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index e2da110fab81..2ca72bfc67f2 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -283,17 +283,17 @@ void sii164_set_power(unsigned char power)
 }
 
 /*
- *  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;
 
 	detectReg = sm750_hw_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;
 		break;
@@ -329,9 +329,9 @@ void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug)
 	 * 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);
 }
 
 /*
-- 
2.45.2

[PATCH 7/8] staging: sm750fb: rename detectReg to detect_reg

[Ruben Wauters]

Renames detectReg to detect_reg in a few functions

Fixes checkpatch.pl's camel case check

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 drivers/staging/sm750fb/ddk750_sii164.c | 38 ++++++++++++-------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 2ca72bfc67f2..769cbe768c49 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -289,28 +289,28 @@ void sii164_set_power(unsigned char power)
 static
 void sii164_select_hot_plug_detection_mode(enum sii164_hot_plug_mode hot_plug_mode)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
-	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
-		    ~SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG;
+	detect_reg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
+		     ~SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG;
 	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;
 	}
 
-	sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_DETECT, detectReg);
+	sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_DETECT, detect_reg);
 }
 
 /*
@@ -321,9 +321,9 @@ void sii164_select_hot_plug_detection_mode(enum sii164_hot_plug_mode hot_plug_mo
  */
 void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
-	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT);
+	detect_reg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT);
 
 	/* Depending on each DVI controller, need to enable the hot plug based
 	 * on each individual chip design.
@@ -364,11 +364,11 @@ unsigned char sii164_is_connected(void)
  */
 unsigned char sii164_check_interrupt(void)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
-	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
-		    SII164_DETECT_MONITOR_STATE_MASK;
-	if (detectReg == SII164_DETECT_MONITOR_STATE_CHANGE)
+	detect_reg = sm750_hw_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;
@@ -380,12 +380,12 @@ unsigned char sii164_check_interrupt(void)
  */
 void sii164_clear_interrupt(void)
 {
-	unsigned char detectReg;
+	unsigned char detect_reg;
 
 	/* Clear the MDI interrupt */
-	detectReg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT);
+	detect_reg = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT);
 	sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, SII164_DETECT,
-			       detectReg | SII164_DETECT_MONITOR_STATE_CLEAR);
+			       detect_reg | SII164_DETECT_MONITOR_STATE_CLEAR);
 }
 
 #endif
-- 
2.45.2

[PATCH 8/8] staging: sm750fb: rename hotPlugValue to hot_plug_value

[Ruben Wauters]

Renames hotPlugValue to hot_plug_value

fixes checkpatch.pl's camel case check.

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 drivers/staging/sm750fb/ddk750_sii164.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 769cbe768c49..86490c87156a 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -344,11 +344,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_value;
 
-	hotPlugValue = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
-		       SII164_DETECT_HOT_PLUG_STATUS_MASK;
-	if (hotPlugValue == SII164_DETECT_HOT_PLUG_STATUS_ON)
+	hot_plug_value = sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, SII164_DETECT) &
+			 SII164_DETECT_HOT_PLUG_STATUS_MASK;
+	if (hot_plug_value == SII164_DETECT_HOT_PLUG_STATUS_ON)
 		return 1;
 	else
 		return 0;
-- 
2.45.2

Re: [PATCH 8/8] staging: sm750fb: rename hotPlugValue to hot_plug_value

[Greg Kroah-Hartman]

On Thu, Apr 17, 2025 at 08:02:56PM +0100, Ruben Wauters wrote:
> Renames hotPlugValue to hot_plug_value
> 
> fixes checkpatch.pl's camel case check.
> 
> Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> index 769cbe768c49..86490c87156a 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -344,11 +344,11 @@ void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug)
>   */
>  unsigned char sii164_is_connected(void)

This should be returning a boolean, right?  Not your fault, just noticed
it for further potential cleanups if you want to do that.

thanks,

greg k-h