* Re: [PATCH] backlight: ktd2801: depend on GPIOLIB
From: Randy Dunlap @ 2025-04-11 21:07 UTC (permalink / raw)
To: duje.mihanovic, Lee Jones, Daniel Thompson, Jingoo Han,
Helge Deller
Cc: dri-devel, linux-fbdev, linux-kernel
In-Reply-To: <20250411-ktd-fix-v1-1-e7425d273268@skole.hr>
On 4/11/25 10:22 AM, Duje Mihanović via B4 Relay wrote:
> From: Duje Mihanović <duje.mihanovic@skole.hr>
>
> The ExpressWire library used by the driver depends on GPIOLIB, and by
> extension the driver does as well. This is not reflected in the driver's
> Kconfig entry, potentially causing Kconfig warnings. Fix this by adding
> the dependency.
>
> Link: https://lore.kernel.org/all/5cf231e1-0bba-4595-9441-46acc5255512@infradead.org
s/Link:/Closes:/
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>o
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.
> ---
> drivers/video/backlight/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index d9374d208ceebbf8b3c27976e9cb4d725939b942..37341474beb9982f7099711e5e2506081061df46 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -185,6 +185,7 @@ config BACKLIGHT_KTD253
>
> config BACKLIGHT_KTD2801
> tristate "Backlight Driver for Kinetic KTD2801"
> + depends on GPIOLIB || COMPILE_TEST
> select LEDS_EXPRESSWIRE
> help
> Say Y to enable the backlight driver for the Kinetic KTD2801 1-wire
>
> ---
> base-commit: 01c6df60d5d4ae00cd5c1648818744838bba7763
> change-id: 20250411-ktd-fix-7a5e5d8657b8
>
> Best regards,
--
~Randy
^ permalink raw reply
* [GIT PULL] Immutable branch between Backlight, fbdev and LEDs for the v6.16 merge window
From: Lee Jones @ 2025-04-15 17:27 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: pavel, danielt, jingoohan1, deller, simona, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Enjoy!
The following changes since commit 0af2f6be1b4281385b618cb86ad946eded089ac8:
Linux 6.15-rc1 (2025-04-06 13:11:33 -0700)
are available in the Git repository at:
ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git tags/ib-backlight-leds-fbdev-v6.16
for you to fetch changes up to d32a0b567a8a8b6e677d35c4f8eb8bd32b7029a0:
fbdev: Remove constants of unused events (2025-04-10 10:39:14 +0100)
----------------------------------------------------------------
Immutable branch between Backlight, fbdev and LEDs for the v6.16 merge window
----------------------------------------------------------------
Thomas Zimmermann (11):
fbdev: Rework fb_blank()
fbdev: Track display blanking state
fbdev: Send old blank state in FB_EVENT_BLANK
backlight: Implement fbdev tracking with blank state from event
backlight: Move blank-state handling into helper
backlight: Replace fb events with a dedicated function call
backlight: lcd: Move event handling into helpers
backlight: lcd: Replace fb events with a dedicated function call
leds: backlight trigger: Move blank-state handling into helper
leds: backlight trigger: Replace fb events with a dedicated function call
fbdev: Remove constants of unused events
drivers/leds/trigger/ledtrig-backlight.c | 48 +++++++-------
drivers/video/backlight/backlight.c | 93 ++++++--------------------
drivers/video/backlight/lcd.c | 108 ++++++++++++-------------------
drivers/video/fbdev/core/fb_backlight.c | 12 ++++
drivers/video/fbdev/core/fb_info.c | 1 +
drivers/video/fbdev/core/fbmem.c | 82 +++++++++++++++++++----
drivers/video/fbdev/core/fbsysfs.c | 8 +--
include/linux/backlight.h | 32 ++++-----
include/linux/fb.h | 12 ++--
include/linux/lcd.h | 21 +++++-
include/linux/leds.h | 6 ++
11 files changed, 215 insertions(+), 208 deletions(-)
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH] staging: sm750fb: fix instances of camel case
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
In-Reply-To: <20250417153101.353645-1-rubenru09.ref@aol.com>
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
* Re: [PATCH] staging: sm750fb: fix instances of camel case
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
In-Reply-To: <20250417153101.353645-1-rubenru09@aol.com>
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
* Re: [PATCH] staging: sm750fb: fix instances of camel case
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
In-Reply-To: <2025041758-mounting-populace-458f@gregkh>
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
* Re: [PATCH] staging: sm750fb: fix instances of camel case
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
In-Reply-To: <6ec1fa494ee823549fb97a48121cb28e37f1cc4d.camel@aol.com>
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
* [PATCH 1/8] staging: sm250fb: remove USE_HW_I2C check
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH 4/8] staging: sm750fb: rename sii164_init_chip params
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH 3/8] staging: sm750fb: rename vendorID to vendor_id
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH 0/8] staging: sm750fb: cleanup ddk750_sii164
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09.ref@aol.com>
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
^ permalink raw reply
* [PATCH 5/8] staging: sm750fb: rename sii164_set_power's param
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH 6/8] staging: sm750fb: rename sii164SelectHotPlugDetectionMode
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH 7/8] staging: sm750fb: rename detectReg to detect_reg
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH 8/8] staging: sm750fb: rename hotPlugValue to hot_plug_value
From: Ruben Wauters @ 2025-04-17 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sudip Mukherjee, Teddy Wang, Sudip Mukherjee
Cc: Ruben Wauters, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-1-rubenru09@aol.com>
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
^ permalink raw reply related
* [PATCH] staging: sm750fb: clean-up `else`-blocks
From: Eric Florin @ 2025-04-18 3:50 UTC (permalink / raw)
To: gregkh
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel, Eric Florin
Clean-up `else`-blocks in `hw_sm750_map` that occur after `if`-blocks that
terminate function execution.
Signed-off-by: Eric Florin <ericflorin.kernel@gmail.com>
---
drivers/staging/sm750fb/sm750_hw.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 4bc89218c11c..64b199061d14 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -55,9 +55,8 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
pr_err("mmio failed\n");
ret = -EFAULT;
goto exit;
- } else {
- pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
}
+ pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
sm750_dev->accel.dprBase = sm750_dev->pvReg + DE_BASE_ADDR_TYPE1;
sm750_dev->accel.dpPortBase = sm750_dev->pvReg + DE_PORT_ADDR_TYPE1;
@@ -84,9 +83,8 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
pr_err("Map video memory failed\n");
ret = -EFAULT;
goto exit;
- } else {
- pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
}
+ pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
exit:
return ret;
}
--
2.39.5
^ permalink raw reply related
* [PATCH v2] backlight: ktd2801: depend on GPIOLIB
From: Duje Mihanović via B4 Relay @ 2025-04-18 8:43 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Helge Deller
Cc: Randy Dunlap, dri-devel, linux-fbdev, linux-kernel,
Duje Mihanović
From: Duje Mihanović <duje.mihanovic@skole.hr>
The ExpressWire library used by the driver depends on GPIOLIB, and by
extension the driver does as well. This is not reflected in the driver's
Kconfig entry, potentially causing Kconfig warnings. Fix this by adding
the dependency.
Closes: https://lore.kernel.org/all/5cf231e1-0bba-4595-9441-46acc5255512@infradead.org
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
Changes in v2:
- s/Link/Closes
- Pull Randy's tags
- Link to v1: https://lore.kernel.org/r/20250411-ktd-fix-v1-1-e7425d273268@skole.hr
---
drivers/video/backlight/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index d9374d208ceebbf8b3c27976e9cb4d725939b942..37341474beb9982f7099711e5e2506081061df46 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -185,6 +185,7 @@ config BACKLIGHT_KTD253
config BACKLIGHT_KTD2801
tristate "Backlight Driver for Kinetic KTD2801"
+ depends on GPIOLIB || COMPILE_TEST
select LEDS_EXPRESSWIRE
help
Say Y to enable the backlight driver for the Kinetic KTD2801 1-wire
---
base-commit: 01c6df60d5d4ae00cd5c1648818744838bba7763
change-id: 20250411-ktd-fix-7a5e5d8657b8
Best regards,
--
Duje Mihanović <duje.mihanovic@skole.hr>
^ permalink raw reply related
* Re: [PATCH 1/8] staging: sm250fb: remove USE_HW_I2C check
From: Greg Kroah-Hartman @ 2025-04-18 10:33 UTC (permalink / raw)
To: Ruben Wauters
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-2-rubenru09@aol.com>
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
^ permalink raw reply
* Re: [PATCH 5/8] staging: sm750fb: rename sii164_set_power's param
From: Greg Kroah-Hartman @ 2025-04-18 10:34 UTC (permalink / raw)
To: Ruben Wauters
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-6-rubenru09@aol.com>
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
^ permalink raw reply
* Re: [PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName
From: Greg Kroah-Hartman @ 2025-04-18 10:36 UTC (permalink / raw)
To: Ruben Wauters
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-3-rubenru09@aol.com>
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
^ permalink raw reply
* Re: [PATCH 3/8] staging: sm750fb: rename vendorID to vendor_id
From: Greg Kroah-Hartman @ 2025-04-18 10:37 UTC (permalink / raw)
To: Ruben Wauters
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-4-rubenru09@aol.com>
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
^ permalink raw reply
* Re: [PATCH 8/8] staging: sm750fb: rename hotPlugValue to hot_plug_value
From: Greg Kroah-Hartman @ 2025-04-18 10:38 UTC (permalink / raw)
To: Ruben Wauters
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <20250417190302.13811-9-rubenru09@aol.com>
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
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: clean-up `else`-blocks
From: Greg KH @ 2025-04-18 10:39 UTC (permalink / raw)
To: Eric Florin
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20250418035023.27067-1-ericflorin.kernel@gmail.com>
On Thu, Apr 17, 2025 at 08:50:23PM -0700, Eric Florin wrote:
> Clean-up `else`-blocks in `hw_sm750_map` that occur after `if`-blocks that
> terminate function execution.
>
> Signed-off-by: Eric Florin <ericflorin.kernel@gmail.com>
> ---
> drivers/staging/sm750fb/sm750_hw.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
> index 4bc89218c11c..64b199061d14 100644
> --- a/drivers/staging/sm750fb/sm750_hw.c
> +++ b/drivers/staging/sm750fb/sm750_hw.c
> @@ -55,9 +55,8 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
> pr_err("mmio failed\n");
> ret = -EFAULT;
> goto exit;
> - } else {
> - pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
> }
> + pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
When drivers work properly, they are "quiet". This driver is not quiet
at all. So this really should be a call to dev_dbg(), right?
>
> sm750_dev->accel.dprBase = sm750_dev->pvReg + DE_BASE_ADDR_TYPE1;
> sm750_dev->accel.dpPortBase = sm750_dev->pvReg + DE_PORT_ADDR_TYPE1;
> @@ -84,9 +83,8 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
> pr_err("Map video memory failed\n");
> ret = -EFAULT;
> goto exit;
> - } else {
> - pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
> }
> + pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
Same here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName
From: Ruben Wauters @ 2025-04-18 11:45 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <2025041803-clutter-harmonica-7047@gregkh>
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
^ permalink raw reply
* Re: [PATCH 1/8] staging: sm250fb: remove USE_HW_I2C check
From: Ruben Wauters @ 2025-04-18 11:42 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <2025041836-debug-unstopped-9a88@gregkh>
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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox