* 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
* Re: [PATCH 2/8] staging: sm750fb: rename gDviCtrlChipName
From: Greg Kroah-Hartman @ 2025-04-18 12:09 UTC (permalink / raw)
To: Ruben Wauters
Cc: Sudip Mukherjee, Teddy Wang, Sudip Mukherjee, linux-fbdev,
linux-staging, linux-kernel
In-Reply-To: <6a47dc48a803b6a07a7fcd33eec8df9e60e86144.camel@aol.com>
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
^ permalink raw reply
* [PATCH][next] fbdev/carminefb: Fix spelling mistake of CARMINE_TOTAL_DIPLAY_MEM
From: Colin Ian King @ 2025-04-18 12:51 UTC (permalink / raw)
To: Helge Deller, Colin Ian King, linux-fbdev, dri-devel
Cc: kernel-janitors, linux-kernel
There is a spelling mistake in macro CARMINE_TOTAL_DIPLAY_MEM. Fix it.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
---
drivers/video/fbdev/carminefb.c | 8 ++++----
drivers/video/fbdev/carminefb.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/carminefb.c b/drivers/video/fbdev/carminefb.c
index e56065cdba97..2bdd67595891 100644
--- a/drivers/video/fbdev/carminefb.c
+++ b/drivers/video/fbdev/carminefb.c
@@ -649,13 +649,13 @@ static int carminefb_probe(struct pci_dev *dev, const struct pci_device_id *ent)
* is required for that largest resolution to avoid remaps at run
* time
*/
- if (carminefb_fix.smem_len > CARMINE_TOTAL_DIPLAY_MEM)
- carminefb_fix.smem_len = CARMINE_TOTAL_DIPLAY_MEM;
+ if (carminefb_fix.smem_len > CARMINE_TOTAL_DISPLAY_MEM)
+ carminefb_fix.smem_len = CARMINE_TOTAL_DISPLAY_MEM;
- else if (carminefb_fix.smem_len < CARMINE_TOTAL_DIPLAY_MEM) {
+ else if (carminefb_fix.smem_len < CARMINE_TOTAL_DISPLAY_MEM) {
printk(KERN_ERR "carminefb: Memory bar is only %d bytes, %d "
"are required.", carminefb_fix.smem_len,
- CARMINE_TOTAL_DIPLAY_MEM);
+ CARMINE_TOTAL_DISPLAY_MEM);
goto err_unmap_vregs;
}
diff --git a/drivers/video/fbdev/carminefb.h b/drivers/video/fbdev/carminefb.h
index 297688eba469..c9825481d96b 100644
--- a/drivers/video/fbdev/carminefb.h
+++ b/drivers/video/fbdev/carminefb.h
@@ -7,7 +7,7 @@
#define MAX_DISPLAY 2
#define CARMINE_DISPLAY_MEM (800 * 600 * 4)
-#define CARMINE_TOTAL_DIPLAY_MEM (CARMINE_DISPLAY_MEM * MAX_DISPLAY)
+#define CARMINE_TOTAL_DISPLAY_MEM (CARMINE_DISPLAY_MEM * MAX_DISPLAY)
#define CARMINE_USE_DISPLAY0 (1 << 0)
#define CARMINE_USE_DISPLAY1 (1 << 1)
--
2.49.0
^ permalink raw reply related
* [PATCH 1/4] staging: sm750fb: Remove ddk750_sii164
From: Ruben Wauters @ 2025-04-18 15:17 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: <20250418151755.42624-1-rubenru09@aol.com>
Removes unused functions and files ddk750_sii164.
Functions were used in ddk750_dvi.c, which is itself
unused. Removal will be in the second patch in the series.
Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
I have checked the entire driver, and found that the code
present within these files is used nowhere (except for in
ddk750_dvi.c, which is also used nowhere).
As such I have decided to remove the code in this file, as
it is essentially deadcode.
---
drivers/staging/sm750fb/Makefile | 4 +-
drivers/staging/sm750fb/ddk750_dvi.c | 21 +-
drivers/staging/sm750fb/ddk750_sii164.c | 408 ------------------------
drivers/staging/sm750fb/ddk750_sii164.h | 174 ----------
4 files changed, 4 insertions(+), 603 deletions(-)
delete mode 100644 drivers/staging/sm750fb/ddk750_sii164.c
delete mode 100644 drivers/staging/sm750fb/ddk750_sii164.h
diff --git a/drivers/staging/sm750fb/Makefile b/drivers/staging/sm750fb/Makefile
index b89aa4c12e9d..1926376664ca 100644
--- a/drivers/staging/sm750fb/Makefile
+++ b/drivers/staging/sm750fb/Makefile
@@ -3,5 +3,5 @@ obj-$(CONFIG_FB_SM750) += sm750fb.o
sm750fb-objs := sm750.o sm750_hw.o sm750_accel.o sm750_cursor.o \
ddk750_chip.o ddk750_power.o ddk750_mode.o \
- ddk750_display.o ddk750_swi2c.o ddk750_sii164.o \
- ddk750_dvi.o ddk750_hwi2c.o
+ ddk750_display.o ddk750_swi2c.o ddk750_dvi.o \
+ ddk750_hwi2c.o
diff --git a/drivers/staging/sm750fb/ddk750_dvi.c b/drivers/staging/sm750fb/ddk750_dvi.c
index 6fef1ab484c1..9842c4e4cdf8 100644
--- a/drivers/staging/sm750fb/ddk750_dvi.c
+++ b/drivers/staging/sm750fb/ddk750_dvi.c
@@ -4,31 +4,14 @@
#include "ddk750_chip.h"
#include "ddk750_reg.h"
#include "ddk750_dvi.h"
-#include "ddk750_sii164.h"
/*
* This global variable contains all the supported driver and its corresponding
* function API. Please set the function pointer to NULL whenever the function
* is not supported.
*/
-static struct dvi_ctrl_device dcft_supported_dvi_controller[] = {
-#ifdef DVI_CTRL_SII164
- {
- .init = sii164_init_chip,
- .get_vendor_id = sii164_get_vendor_id,
- .get_device_id = sii164_get_device_id,
-#ifdef SII164_FULL_FUNCTIONS
- .reset_chip = sii164_reset_chip,
- .get_chip_string = sii164_get_chip_string,
- .set_power = sii164_set_power,
- .enable_hot_plug_detection = sii164_enable_hot_plug_detection,
- .is_connected = sii164_is_connected,
- .check_interrupt = sii164_check_interrupt,
- .clear_interrupt = sii164_clear_interrupt,
-#endif
- },
-#endif
-};
+
+static struct dvi_ctrl_device dcft_supported_dvi_controller[] = { };
int dvi_init(unsigned char edge_select,
unsigned char bus_select,
diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
deleted file mode 100644
index 89700fc5dd2e..000000000000
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ /dev/null
@@ -1,408 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#define USE_DVICHIP
-#ifdef USE_DVICHIP
-
-#include "ddk750_sii164.h"
-#include "ddk750_hwi2c.h"
-
-/* 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
-
-#ifdef SII164_FULL_FUNCTIONS
-/* Name of the DVI Controller chip */
-static char *gDviCtrlChipName = "Silicon Image SiI 164";
-#endif
-
-/*
- * sii164_get_vendor_id
- * This function gets the vendor ID of the DVI controller chip.
- *
- * Output:
- * Vendor ID
- */
-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);
-
- return vendorID;
-}
-
-/*
- * sii164_get_device_id
- * This function gets the device ID of the DVI controller chip.
- *
- * Output:
- * Device ID
- */
-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);
-
- return device_id;
-}
-
-/*
- * DVI.C will handle all SiI164 chip stuffs and try its best to make code
- * minimal and useful
- */
-
-/*
- * sii164_init_chip
- * This function initialize and detect the DVI controller chip.
- *
- * Input:
- * edge_select - Edge Select:
- * 0 = Input data is falling edge latched (falling
- * edge latched first in dual edge mode)
- * 1 = Input data is rising edge latched (rising
- * edge latched first in dual edge mode)
- * bus_select - Input Bus Select:
- * 0 = Input data bus is 12-bits wide
- * 1 = Input data bus is 24-bits wide
- * dual_edge_clk_select - Dual Edge Clock Select
- * 0 = Input data is single edge latched
- * 1 = Input data is dual edge latched
- * hsync_enable - Horizontal Sync Enable:
- * 0 = HSYNC input is transmitted as fixed LOW
- * 1 = HSYNC input is transmitted as is
- * vsync_enable - Vertical Sync Enable:
- * 0 = VSYNC input is transmitted as fixed LOW
- * 1 = VSYNC input is transmitted as is
- * deskew_enable - De-skewing Enable:
- * 0 = De-skew disabled
- * 1 = De-skew enabled
- * deskew_setting - De-skewing Setting (increment of 260psec)
- * 0 = 1 step --> minimum setup / maximum hold
- * 1 = 2 step
- * 2 = 3 step
- * 3 = 4 step
- * 4 = 5 step
- * 5 = 6 step
- * 6 = 7 step
- * 7 = 8 step --> maximum setup / minimum hold
- * continuous_sync_enable- SYNC Continuous:
- * 0 = Disable
- * 1 = Enable
- * pll_filter_enable - PLL Filter Enable
- * 0 = Disable PLL Filter
- * 1 = Enable PLL Filter
- * pll_filter_value - PLL Filter characteristics:
- * 0~7 (recommended value is 4)
- *
- * Output:
- * 0 - Success
- * -1 - Fail.
- */
-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 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) &&
- (sii164_get_device_id() == SII164_DEVICE_ID)) {
- /*
- * Initialize SII164 controller chip.
- */
-
- /* Select the edge */
- if (edge_select == 0)
- config = SII164_CONFIGURATION_LATCH_FALLING;
- else
- config = SII164_CONFIGURATION_LATCH_RISING;
-
- /* Select bus wide */
- if (bus_select == 0)
- config |= SII164_CONFIGURATION_BUS_12BITS;
- else
- config |= SII164_CONFIGURATION_BUS_24BITS;
-
- /* Select Dual/Single Edge Clock */
- if (dual_edge_clk_select == 0)
- config |= SII164_CONFIGURATION_CLOCK_SINGLE;
- else
- config |= SII164_CONFIGURATION_CLOCK_DUAL;
-
- /* Select HSync Enable */
- if (hsync_enable == 0)
- config |= SII164_CONFIGURATION_HSYNC_FORCE_LOW;
- else
- config |= SII164_CONFIGURATION_HSYNC_AS_IS;
-
- /* Select VSync Enable */
- if (vsync_enable == 0)
- config |= SII164_CONFIGURATION_VSYNC_FORCE_LOW;
- else
- config |= SII164_CONFIGURATION_VSYNC_AS_IS;
-
- i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
-
- /*
- * De-skew enabled with default 111b value.
- * This fixes some artifacts problem in some mode on board 2.2.
- * Somehow this fix does not affect board 2.1.
- */
- if (deskew_enable == 0)
- config = SII164_DESKEW_DISABLE;
- else
- config = SII164_DESKEW_ENABLE;
-
- switch (deskew_setting) {
- case 0:
- config |= SII164_DESKEW_1_STEP;
- break;
- case 1:
- config |= SII164_DESKEW_2_STEP;
- break;
- case 2:
- config |= SII164_DESKEW_3_STEP;
- break;
- case 3:
- config |= SII164_DESKEW_4_STEP;
- break;
- case 4:
- config |= SII164_DESKEW_5_STEP;
- break;
- case 5:
- config |= SII164_DESKEW_6_STEP;
- break;
- case 6:
- config |= SII164_DESKEW_7_STEP;
- break;
- case 7:
- config |= SII164_DESKEW_8_STEP;
- break;
- }
- i2cWriteReg(SII164_I2C_ADDRESS, SII164_DESKEW, config);
-
- /* Enable/Disable Continuous Sync. */
- if (continuous_sync_enable == 0)
- config = SII164_PLL_FILTER_SYNC_CONTINUOUS_DISABLE;
- else
- config = SII164_PLL_FILTER_SYNC_CONTINUOUS_ENABLE;
-
- /* Enable/Disable PLL Filter */
- if (pll_filter_enable == 0)
- config |= SII164_PLL_FILTER_DISABLE;
- else
- config |= SII164_PLL_FILTER_ENABLE;
-
- /* Set the PLL Filter value */
- config |= ((pll_filter_value & 0x07) << 1);
-
- i2cWriteReg(SII164_I2C_ADDRESS, SII164_PLL, config);
-
- /* Recover from Power Down and enable output. */
- config = i2cReadReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION);
- config |= SII164_CONFIGURATION_POWER_NORMAL;
- i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
-
- return 0;
- }
-
- /* Return -1 if initialization fails. */
- return -1;
-}
-
-/* below sii164 function is not necessary */
-
-#ifdef SII164_FULL_FUNCTIONS
-
-/*
- * sii164_reset_chip
- * This function resets the DVI Controller Chip.
- */
-void sii164_reset_chip(void)
-{
- /* Power down */
- sii164_set_power(0);
- sii164_set_power(1);
-}
-
-/*
- * sii164_get_chip_string
- * This function returns a char string name of the current DVI Controller
- * chip.
- *
- * It's convenient for application need to display the chip name.
- */
-char *sii164_get_chip_string(void)
-{
- return gDviCtrlChipName;
-}
-
-/*
- * sii164_set_power
- * This function sets the power configuration of the DVI Controller Chip.
- *
- * Input:
- * powerUp - Flag to set the power down or up
- */
-void sii164_set_power(unsigned char powerUp)
-{
- unsigned char config;
-
- config = i2cReadReg(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);
- } else {
- /* Power down the chip */
- config &= ~SII164_CONFIGURATION_POWER_MASK;
- config |= SII164_CONFIGURATION_POWER_DOWN;
- i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
- }
-}
-
-/*
- * sii164SelectHotPlugDetectionMode
- * This function selects the mode of the hot plug detection.
- */
-static
-void sii164SelectHotPlugDetectionMode(enum sii164_hot_plug_mode hotPlugMode)
-{
- unsigned char detectReg;
-
- detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
- ~SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG;
- switch (hotPlugMode) {
- case SII164_HOTPLUG_DISABLE:
- detectReg |= 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;
- break;
- case SII164_HOTPLUG_USE_RSEN:
- detectReg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_RSEN;
- break;
- case SII164_HOTPLUG_USE_HTPLG:
- detectReg |= SII164_DETECT_MONITOR_SENSE_OUTPUT_HTPLG;
- break;
- }
-
- i2cWriteReg(SII164_I2C_ADDRESS, SII164_DETECT, detectReg);
-}
-
-/*
- * sii164_enable_hot_plug_detection
- * This function enables the Hot Plug detection.
- *
- * enable_hot_plug - Enable (=1) / disable (=0) Hot Plug detection
- */
-void sii164_enable_hot_plug_detection(unsigned char enable_hot_plug)
-{
- unsigned char detectReg;
-
- detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT);
-
- /* 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);
- else
- sii164SelectHotPlugDetectionMode(SII164_HOTPLUG_DISABLE);
-}
-
-/*
- * sii164_is_connected
- * Check if the DVI Monitor is connected.
- *
- * Output:
- * 0 - Not Connected
- * 1 - Connected
- */
-unsigned char sii164_is_connected(void)
-{
- unsigned char hotPlugValue;
-
- hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
- SII164_DETECT_HOT_PLUG_STATUS_MASK;
- if (hotPlugValue == SII164_DETECT_HOT_PLUG_STATUS_ON)
- return 1;
- else
- return 0;
-}
-
-/*
- * sii164_check_interrupt
- * Checks if interrupt has occurred.
- *
- * Output:
- * 0 - No interrupt
- * 1 - Interrupt occurs
- */
-unsigned char sii164_check_interrupt(void)
-{
- unsigned char detectReg;
-
- detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) &
- SII164_DETECT_MONITOR_STATE_MASK;
- if (detectReg == SII164_DETECT_MONITOR_STATE_CHANGE)
- return 1;
- else
- return 0;
-}
-
-/*
- * sii164_clear_interrupt
- * Clear the hot plug interrupt.
- */
-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);
-}
-
-#endif
-
-#endif
diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
deleted file mode 100644
index ebc173658f0e..000000000000
--- a/drivers/staging/sm750fb/ddk750_sii164.h
+++ /dev/null
@@ -1,174 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef DDK750_SII164_H__
-#define DDK750_SII164_H__
-
-#define USE_DVICHIP
-
-/* Hot Plug detection mode structure */
-enum sii164_hot_plug_mode {
- SII164_HOTPLUG_DISABLE = 0, /* Disable Hot Plug output bit
- * (always high).
- */
-
- SII164_HOTPLUG_USE_MDI, /* Use Monitor Detect Interrupt bit. */
- SII164_HOTPLUG_USE_RSEN, /* Use Receiver Sense detect bit. */
- SII164_HOTPLUG_USE_HTPLG /* Use Hot Plug detect bit. */
-};
-
-/* 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);
-
-unsigned short sii164_get_vendor_id(void);
-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_enable_hot_plug_detection(unsigned char enable_hot_plug);
-unsigned char sii164_is_connected(void);
-unsigned char sii164_check_interrupt(void);
-void sii164_clear_interrupt(void);
-#endif
-/*
- * below register definition is used for
- * Silicon Image SiI164 DVI controller chip
- */
-/*
- * Vendor ID registers
- */
-#define SII164_VENDOR_ID_LOW 0x00
-#define SII164_VENDOR_ID_HIGH 0x01
-
-/*
- * Device ID registers
- */
-#define SII164_DEVICE_ID_LOW 0x02
-#define SII164_DEVICE_ID_HIGH 0x03
-
-/*
- * Device Revision
- */
-#define SII164_DEVICE_REVISION 0x04
-
-/*
- * Frequency Limitation registers
- */
-#define SII164_FREQUENCY_LIMIT_LOW 0x06
-#define SII164_FREQUENCY_LIMIT_HIGH 0x07
-
-/*
- * Power Down and Input Signal Configuration registers
- */
-#define SII164_CONFIGURATION 0x08
-
-/* Power down (PD) */
-#define SII164_CONFIGURATION_POWER_DOWN 0x00
-#define SII164_CONFIGURATION_POWER_NORMAL 0x01
-#define SII164_CONFIGURATION_POWER_MASK 0x01
-
-/* Input Edge Latch Select (EDGE) */
-#define SII164_CONFIGURATION_LATCH_FALLING 0x00
-#define SII164_CONFIGURATION_LATCH_RISING 0x02
-
-/* Bus Select (BSEL) */
-#define SII164_CONFIGURATION_BUS_12BITS 0x00
-#define SII164_CONFIGURATION_BUS_24BITS 0x04
-
-/* Dual Edge Clock Select (DSEL) */
-#define SII164_CONFIGURATION_CLOCK_SINGLE 0x00
-#define SII164_CONFIGURATION_CLOCK_DUAL 0x08
-
-/* Horizontal Sync Enable (HEN) */
-#define SII164_CONFIGURATION_HSYNC_FORCE_LOW 0x00
-#define SII164_CONFIGURATION_HSYNC_AS_IS 0x10
-
-/* Vertical Sync Enable (VEN) */
-#define SII164_CONFIGURATION_VSYNC_FORCE_LOW 0x00
-#define SII164_CONFIGURATION_VSYNC_AS_IS 0x20
-
-/*
- * Detection registers
- */
-#define SII164_DETECT 0x09
-
-/* Monitor Detect Interrupt (MDI) */
-#define SII164_DETECT_MONITOR_STATE_CHANGE 0x00
-#define SII164_DETECT_MONITOR_STATE_NO_CHANGE 0x01
-#define SII164_DETECT_MONITOR_STATE_CLEAR 0x01
-#define SII164_DETECT_MONITOR_STATE_MASK 0x01
-
-/* Hot Plug detect Input (HTPLG) */
-#define SII164_DETECT_HOT_PLUG_STATUS_OFF 0x00
-#define SII164_DETECT_HOT_PLUG_STATUS_ON 0x02
-#define SII164_DETECT_HOT_PLUG_STATUS_MASK 0x02
-
-/* Receiver Sense (RSEN) */
-#define SII164_DETECT_RECEIVER_SENSE_NOT_DETECTED 0x00
-#define SII164_DETECT_RECEIVER_SENSE_DETECTED 0x04
-
-/* Interrupt Generation Method (TSEL) */
-#define SII164_DETECT_INTERRUPT_BY_RSEN_PIN 0x00
-#define SII164_DETECT_INTERRUPT_BY_HTPLG_PIN 0x08
-#define SII164_DETECT_INTERRUPT_MASK 0x08
-
-/* Monitor Sense Output (MSEN) */
-#define SII164_DETECT_MONITOR_SENSE_OUTPUT_HIGH 0x00
-#define SII164_DETECT_MONITOR_SENSE_OUTPUT_MDI 0x10
-#define SII164_DETECT_MONITOR_SENSE_OUTPUT_RSEN 0x20
-#define SII164_DETECT_MONITOR_SENSE_OUTPUT_HTPLG 0x30
-#define SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG 0x30
-
-/*
- * Skewing registers
- */
-#define SII164_DESKEW 0x0A
-
-/* General Purpose Input (CTL[3:1]) */
-#define SII164_DESKEW_GENERAL_PURPOSE_INPUT_MASK 0x0E
-
-/* De-skewing Enable bit (DKEN) */
-#define SII164_DESKEW_DISABLE 0x00
-#define SII164_DESKEW_ENABLE 0x10
-
-/* De-skewing Setting (DK[3:1])*/
-#define SII164_DESKEW_1_STEP 0x00
-#define SII164_DESKEW_2_STEP 0x20
-#define SII164_DESKEW_3_STEP 0x40
-#define SII164_DESKEW_4_STEP 0x60
-#define SII164_DESKEW_5_STEP 0x80
-#define SII164_DESKEW_6_STEP 0xA0
-#define SII164_DESKEW_7_STEP 0xC0
-#define SII164_DESKEW_8_STEP 0xE0
-
-/*
- * User Configuration Data registers (CFG 7:0)
- */
-#define SII164_USER_CONFIGURATION 0x0B
-
-/*
- * PLL registers
- */
-#define SII164_PLL 0x0C
-
-/* PLL Filter Value (PLLF) */
-#define SII164_PLL_FILTER_VALUE_MASK 0x0E
-
-/* PLL Filter Enable (PFEN) */
-#define SII164_PLL_FILTER_DISABLE 0x00
-#define SII164_PLL_FILTER_ENABLE 0x01
-
-/* Sync Continuous (SCNT) */
-#define SII164_PLL_FILTER_SYNC_CONTINUOUS_DISABLE 0x00
-#define SII164_PLL_FILTER_SYNC_CONTINUOUS_ENABLE 0x80
-
-#endif
--
2.48.1
^ permalink raw reply related
* [PATCH 0/4] staging: sm750fb: remove unused code/files
From: Ruben Wauters @ 2025-04-18 15:17 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: <20250418151755.42624-1-rubenru09.ref@aol.com>
This patch series removes three unused files and their
associated headers. The code was not used in any other
files (other than themselves).
The TODO mentions that this was sample code, however given
it was never implemented into the driver, I have elected to
remove it entirely.
Ruben Wauters (4):
staging: sm750fb: Remove ddk750_sii164
staging: sm750fb: remove ddk750_dvi
staging: sm750fb: remove ddk750_hwi2c
staging: sm750fb: remove irrelevant TODO line
drivers/staging/sm750fb/Makefile | 3 +-
drivers/staging/sm750fb/TODO | 3 -
drivers/staging/sm750fb/ddk750.h | 3 -
drivers/staging/sm750fb/ddk750_display.c | 1 -
drivers/staging/sm750fb/ddk750_dvi.c | 62 ----
drivers/staging/sm750fb/ddk750_dvi.h | 57 ----
drivers/staging/sm750fb/ddk750_hwi2c.c | 247 --------------
drivers/staging/sm750fb/ddk750_hwi2c.h | 12 -
drivers/staging/sm750fb/ddk750_sii164.c | 408 -----------------------
drivers/staging/sm750fb/ddk750_sii164.h | 174 ----------
10 files changed, 1 insertion(+), 969 deletions(-)
delete mode 100644 drivers/staging/sm750fb/ddk750_dvi.c
delete mode 100644 drivers/staging/sm750fb/ddk750_dvi.h
delete mode 100644 drivers/staging/sm750fb/ddk750_hwi2c.c
delete mode 100644 drivers/staging/sm750fb/ddk750_hwi2c.h
delete mode 100644 drivers/staging/sm750fb/ddk750_sii164.c
delete mode 100644 drivers/staging/sm750fb/ddk750_sii164.h
--
2.48.1
^ permalink raw reply
* [PATCH 3/4] staging: sm750fb: remove ddk750_hwi2c
From: Ruben Wauters @ 2025-04-18 15:17 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: <20250418151755.42624-1-rubenru09@aol.com>
With the removal of ddk750_sii164.c, the functions in
ddk750_hwi2c are now also unused. This patch removes them
and the files they are in.
Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
drivers/staging/sm750fb/Makefile | 2 +-
drivers/staging/sm750fb/ddk750.h | 3 -
drivers/staging/sm750fb/ddk750_hwi2c.c | 247 -------------------------
drivers/staging/sm750fb/ddk750_hwi2c.h | 12 --
4 files changed, 1 insertion(+), 263 deletions(-)
delete mode 100644 drivers/staging/sm750fb/ddk750_hwi2c.c
delete mode 100644 drivers/staging/sm750fb/ddk750_hwi2c.h
diff --git a/drivers/staging/sm750fb/Makefile b/drivers/staging/sm750fb/Makefile
index b3cb973e2672..f7e227df0e54 100644
--- a/drivers/staging/sm750fb/Makefile
+++ b/drivers/staging/sm750fb/Makefile
@@ -3,4 +3,4 @@ obj-$(CONFIG_FB_SM750) += sm750fb.o
sm750fb-objs := sm750.o sm750_hw.o sm750_accel.o sm750_cursor.o \
ddk750_chip.o ddk750_power.o ddk750_mode.o \
- ddk750_display.o ddk750_swi2c.o ddk750_hwi2c.o
+ ddk750_display.o ddk750_swi2c.o
diff --git a/drivers/staging/sm750fb/ddk750.h b/drivers/staging/sm750fb/ddk750.h
index 64ef4d258a91..8a32f8cf3a98 100644
--- a/drivers/staging/sm750fb/ddk750.h
+++ b/drivers/staging/sm750fb/ddk750.h
@@ -14,8 +14,5 @@
#include "ddk750_chip.h"
#include "ddk750_display.h"
#include "ddk750_power.h"
-#ifdef USE_HW_I2C
-#include "ddk750_hwi2c.h"
-#endif
#include "ddk750_swi2c.h"
#endif
diff --git a/drivers/staging/sm750fb/ddk750_hwi2c.c b/drivers/staging/sm750fb/ddk750_hwi2c.c
deleted file mode 100644
index 8482689b665b..000000000000
--- a/drivers/staging/sm750fb/ddk750_hwi2c.c
+++ /dev/null
@@ -1,247 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#define USE_HW_I2C
-#ifdef USE_HW_I2C
-#include "ddk750_chip.h"
-#include "ddk750_reg.h"
-#include "ddk750_hwi2c.h"
-#include "ddk750_power.h"
-
-#define MAX_HWI2C_FIFO 16
-#define HWI2C_WAIT_TIMEOUT 0xF0000
-
-int sm750_hw_i2c_init(unsigned char bus_speed_mode)
-{
- unsigned int value;
-
- /* Enable GPIO 30 & 31 as IIC clock & data */
- value = peek32(GPIO_MUX);
-
- value |= (GPIO_MUX_30 | GPIO_MUX_31);
- poke32(GPIO_MUX, value);
-
- /*
- * Enable Hardware I2C power.
- * TODO: Check if we need to enable GPIO power?
- */
- sm750_enable_i2c(1);
-
- /* Enable the I2C Controller and set the bus speed mode */
- value = peek32(I2C_CTRL) & ~(I2C_CTRL_MODE | I2C_CTRL_EN);
- if (bus_speed_mode)
- value |= I2C_CTRL_MODE;
- value |= I2C_CTRL_EN;
- poke32(I2C_CTRL, value);
-
- return 0;
-}
-
-void sm750_hw_i2c_close(void)
-{
- unsigned int value;
-
- /* Disable I2C controller */
- value = peek32(I2C_CTRL) & ~I2C_CTRL_EN;
- poke32(I2C_CTRL, value);
-
- /* Disable I2C Power */
- sm750_enable_i2c(0);
-
- /* Set GPIO 30 & 31 back as GPIO pins */
- value = peek32(GPIO_MUX);
- value &= ~GPIO_MUX_30;
- value &= ~GPIO_MUX_31;
- poke32(GPIO_MUX, value);
-}
-
-static long hw_i2c_wait_tx_done(void)
-{
- unsigned int timeout;
-
- /* Wait until the transfer is completed. */
- timeout = HWI2C_WAIT_TIMEOUT;
- while (!(peek32(I2C_STATUS) & I2C_STATUS_TX) && (timeout != 0))
- timeout--;
-
- if (timeout == 0)
- return -1;
-
- return 0;
-}
-
-/*
- * This function writes data to the i2c slave device registers.
- *
- * Parameters:
- * addr - i2c Slave device address
- * length - Total number of bytes to be written to the device
- * buf - The buffer that contains the data to be written to the
- * i2c device.
- *
- * Return Value:
- * Total number of bytes those are actually written.
- */
-static unsigned int hw_i2c_write_data(unsigned char addr,
- unsigned int length,
- unsigned char *buf)
-{
- unsigned char count, i;
- unsigned int total_bytes = 0;
-
- /* Set the Device Address */
- poke32(I2C_SLAVE_ADDRESS, addr & ~0x01);
-
- /*
- * Write data.
- * Note:
- * Only 16 byte can be accessed per i2c start instruction.
- */
- do {
- /*
- * Reset I2C by writing 0 to I2C_RESET register to
- * clear the previous status.
- */
- poke32(I2C_RESET, 0);
-
- /* Set the number of bytes to be written */
- if (length < MAX_HWI2C_FIFO)
- count = length - 1;
- else
- count = MAX_HWI2C_FIFO - 1;
- poke32(I2C_BYTE_COUNT, count);
-
- /* Move the data to the I2C data register */
- for (i = 0; i <= count; i++)
- poke32(I2C_DATA0 + i, *buf++);
-
- /* Start the I2C */
- poke32(I2C_CTRL, peek32(I2C_CTRL) | I2C_CTRL_CTRL);
-
- /* Wait until the transfer is completed. */
- if (hw_i2c_wait_tx_done() != 0)
- break;
-
- /* Subtract length */
- length -= (count + 1);
-
- /* Total byte written */
- total_bytes += (count + 1);
-
- } while (length > 0);
-
- return total_bytes;
-}
-
-/*
- * This function reads data from the slave device and stores them
- * in the given buffer
- *
- * Parameters:
- * addr - i2c Slave device address
- * length - Total number of bytes to be read
- * buf - Pointer to a buffer to be filled with the data read
- * from the slave device. It has to be the same size as the
- * length to make sure that it can keep all the data read.
- *
- * Return Value:
- * Total number of actual bytes read from the slave device
- */
-static unsigned int hw_i2c_read_data(unsigned char addr,
- unsigned int length,
- unsigned char *buf)
-{
- unsigned char count, i;
- unsigned int total_bytes = 0;
-
- /* Set the Device Address */
- poke32(I2C_SLAVE_ADDRESS, addr | 0x01);
-
- /*
- * Read data and save them to the buffer.
- * Note:
- * Only 16 byte can be accessed per i2c start instruction.
- */
- do {
- /*
- * Reset I2C by writing 0 to I2C_RESET register to
- * clear all the status.
- */
- poke32(I2C_RESET, 0);
-
- /* Set the number of bytes to be read */
- if (length <= MAX_HWI2C_FIFO)
- count = length - 1;
- else
- count = MAX_HWI2C_FIFO - 1;
- poke32(I2C_BYTE_COUNT, count);
-
- /* Start the I2C */
- poke32(I2C_CTRL, peek32(I2C_CTRL) | I2C_CTRL_CTRL);
-
- /* Wait until transaction done. */
- if (hw_i2c_wait_tx_done() != 0)
- break;
-
- /* Save the data to the given buffer */
- for (i = 0; i <= count; i++)
- *buf++ = peek32(I2C_DATA0 + i);
-
- /* Subtract length by 16 */
- length -= (count + 1);
-
- /* Number of bytes read. */
- total_bytes += (count + 1);
-
- } while (length > 0);
-
- return total_bytes;
-}
-
-/*
- * This function reads the slave device's register
- *
- * Parameters:
- * deviceAddress - i2c Slave device address which register
- * to be read from
- * registerIndex - Slave device's register to be read
- *
- * Return Value:
- * Register value
- */
-unsigned char sm750_hw_i2c_read_reg(unsigned char addr, unsigned char reg)
-{
- unsigned char value = 0xFF;
-
- if (hw_i2c_write_data(addr, 1, ®) == 1)
- hw_i2c_read_data(addr, 1, &value);
-
- return value;
-}
-
-/*
- * This function writes a value to the slave device's register
- *
- * Parameters:
- * deviceAddress - i2c Slave device address which register
- * to be written
- * registerIndex - Slave device's register to be written
- * data - Data to be written to the register
- *
- * Result:
- * 0 - Success
- * -1 - Fail
- */
-int sm750_hw_i2c_write_reg(unsigned char addr,
- unsigned char reg,
- unsigned char data)
-{
- unsigned char value[2];
-
- value[0] = reg;
- value[1] = data;
- if (hw_i2c_write_data(addr, 2, value) == 2)
- return 0;
-
- return -1;
-}
-
-#endif
diff --git a/drivers/staging/sm750fb/ddk750_hwi2c.h b/drivers/staging/sm750fb/ddk750_hwi2c.h
deleted file mode 100644
index 337c6493ca61..000000000000
--- a/drivers/staging/sm750fb/ddk750_hwi2c.h
+++ /dev/null
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef DDK750_HWI2C_H__
-#define DDK750_HWI2C_H__
-
-/* hwi2c functions */
-int sm750_hw_i2c_init(unsigned char bus_speed_mode);
-void sm750_hw_i2c_close(void);
-
-unsigned char sm750_hw_i2c_read_reg(unsigned char addr, unsigned char reg);
-int sm750_hw_i2c_write_reg(unsigned char addr, unsigned char reg,
- unsigned char data);
-#endif
--
2.48.1
^ permalink raw reply related
* [PATCH 4/4] staging: sm750fb: remove irrelevant TODO line
From: Ruben Wauters @ 2025-04-18 15:17 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: <20250418151755.42624-1-rubenru09@aol.com>
As all code referencing USE_HW_I2C and USE_DVICHIP has now
been deleted, this patch removes the TODO line referencing
it.
Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
drivers/staging/sm750fb/TODO | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/sm750fb/TODO b/drivers/staging/sm750fb/TODO
index 9dd57c566257..7ce632d040b3 100644
--- a/drivers/staging/sm750fb/TODO
+++ b/drivers/staging/sm750fb/TODO
@@ -3,9 +3,6 @@ TODO:
- use kernel coding style
- refine the code and remove unused code
- Implement hardware acceleration for imageblit if image->depth > 1
-- check on hardware effects of removal of USE_HW_I2C and USE_DVICHIP (these two
- are supposed to be sample code which is given here if someone wants to
- use those functionalities)
- must be ported to the atomic kms framework in the drm subsystem (which will
give you a basic fbdev driver for free)
--
2.48.1
^ permalink raw reply related
* [PATCH 2/4] staging: sm750fb: remove ddk750_dvi
From: Ruben Wauters @ 2025-04-18 15:17 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: <20250418151755.42624-1-rubenru09@aol.com>
This file and the code present was unused in the whole
driver, therefore this patch removes the file and unused
reference to the header.
Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
drivers/staging/sm750fb/Makefile | 3 +-
drivers/staging/sm750fb/ddk750_display.c | 1 -
drivers/staging/sm750fb/ddk750_dvi.c | 45 -------------------
drivers/staging/sm750fb/ddk750_dvi.h | 57 ------------------------
4 files changed, 1 insertion(+), 105 deletions(-)
delete mode 100644 drivers/staging/sm750fb/ddk750_dvi.c
delete mode 100644 drivers/staging/sm750fb/ddk750_dvi.h
diff --git a/drivers/staging/sm750fb/Makefile b/drivers/staging/sm750fb/Makefile
index 1926376664ca..b3cb973e2672 100644
--- a/drivers/staging/sm750fb/Makefile
+++ b/drivers/staging/sm750fb/Makefile
@@ -3,5 +3,4 @@ obj-$(CONFIG_FB_SM750) += sm750fb.o
sm750fb-objs := sm750.o sm750_hw.o sm750_accel.o sm750_cursor.o \
ddk750_chip.o ddk750_power.o ddk750_mode.o \
- ddk750_display.o ddk750_swi2c.o ddk750_dvi.o \
- ddk750_hwi2c.o
+ ddk750_display.o ddk750_swi2c.o ddk750_hwi2c.o
diff --git a/drivers/staging/sm750fb/ddk750_display.c b/drivers/staging/sm750fb/ddk750_display.c
index 172624ff98b0..4e390541a507 100644
--- a/drivers/staging/sm750fb/ddk750_display.c
+++ b/drivers/staging/sm750fb/ddk750_display.c
@@ -3,7 +3,6 @@
#include "ddk750_chip.h"
#include "ddk750_display.h"
#include "ddk750_power.h"
-#include "ddk750_dvi.h"
static void set_display_control(int ctrl, int disp_state)
{
diff --git a/drivers/staging/sm750fb/ddk750_dvi.c b/drivers/staging/sm750fb/ddk750_dvi.c
deleted file mode 100644
index 9842c4e4cdf8..000000000000
--- a/drivers/staging/sm750fb/ddk750_dvi.c
+++ /dev/null
@@ -1,45 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#define USE_DVICHIP
-#ifdef USE_DVICHIP
-#include "ddk750_chip.h"
-#include "ddk750_reg.h"
-#include "ddk750_dvi.h"
-
-/*
- * This global variable contains all the supported driver and its corresponding
- * function API. Please set the function pointer to NULL whenever the function
- * is not supported.
- */
-
-static struct dvi_ctrl_device dcft_supported_dvi_controller[] = { };
-
-int dvi_init(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)
-{
- struct dvi_ctrl_device *current_dvi_ctrl;
-
- current_dvi_ctrl = dcft_supported_dvi_controller;
- if (current_dvi_ctrl->init) {
- return current_dvi_ctrl->init(edge_select,
- bus_select,
- dual_edge_clk_select,
- hsync_enable,
- vsync_enable,
- deskew_enable,
- deskew_setting,
- continuous_sync_enable,
- pll_filter_enable,
- pll_filter_value);
- }
- return -1; /* error */
-}
-
-#endif
diff --git a/drivers/staging/sm750fb/ddk750_dvi.h b/drivers/staging/sm750fb/ddk750_dvi.h
deleted file mode 100644
index c2518b73bdbd..000000000000
--- a/drivers/staging/sm750fb/ddk750_dvi.h
+++ /dev/null
@@ -1,57 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef DDK750_DVI_H__
-#define DDK750_DVI_H__
-
-/* dvi chip stuffs structros */
-
-typedef long (*PFN_DVICTRL_INIT)(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);
-
-typedef void (*PFN_DVICTRL_RESETCHIP)(void);
-typedef char* (*PFN_DVICTRL_GETCHIPSTRING)(void);
-typedef unsigned short (*PFN_DVICTRL_GETVENDORID)(void);
-typedef unsigned short (*PFN_DVICTRL_GETDEVICEID)(void);
-typedef void (*PFN_DVICTRL_SETPOWER)(unsigned char power_up);
-typedef void (*PFN_DVICTRL_HOTPLUGDETECTION)(unsigned char enable_hot_plug);
-typedef unsigned char (*PFN_DVICTRL_ISCONNECTED)(void);
-typedef unsigned char (*PFN_DVICTRL_CHECKINTERRUPT)(void);
-typedef void (*PFN_DVICTRL_CLEARINTERRUPT)(void);
-
-/* Structure to hold all the function pointer to the DVI Controller. */
-struct dvi_ctrl_device {
- PFN_DVICTRL_INIT init;
- PFN_DVICTRL_RESETCHIP reset_chip;
- PFN_DVICTRL_GETCHIPSTRING get_chip_string;
- PFN_DVICTRL_GETVENDORID get_vendor_id;
- PFN_DVICTRL_GETDEVICEID get_device_id;
- PFN_DVICTRL_SETPOWER set_power;
- PFN_DVICTRL_HOTPLUGDETECTION enable_hot_plug_detection;
- PFN_DVICTRL_ISCONNECTED is_connected;
- PFN_DVICTRL_CHECKINTERRUPT check_interrupt;
- PFN_DVICTRL_CLEARINTERRUPT clear_interrupt;
-};
-
-#define DVI_CTRL_SII164
-
-/* dvi functions prototype */
-int dvi_init(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);
-
-#endif
-
--
2.48.1
^ permalink raw reply related
* Re: [PATCH AUTOSEL 5.10 8/8] fbdev: omapfb: Add 'plane' value check
From: Pavel Machek @ 2025-04-18 17:07 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Leonid Arapov, Helge Deller,
krzysztof.kozlowski, u.kleine-koenig, linux, linux-omap,
linux-fbdev, dri-devel
In-Reply-To: <20250403192031.2682315-8-sashal@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]
Hi!
> Function dispc_ovl_setup is not intended to work with the value OMAP_DSS_WB
> of the enum parameter plane.
>
> The value of this parameter is initialized in dss_init_overlays and in the
> current state of the code it cannot take this value so it's not a real
> problem.
>
> For the purposes of defensive coding it wouldn't be superfluous to check
> the parameter value, because some functions down the call stack process
> this value correctly and some not.
>
> For example, in dispc_ovl_setup_global_alpha it may lead to buffer
> overflow.
>
> Add check for this value.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE static
> analysis tool.
As changelog explains, this is robustness, not really a bug fix. We
should not need it in -stable. (Or maybe rules file should be updated,
because noone seems to be following this rule).
Best regards,
Pavel
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> @@ -2751,9 +2751,13 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
> bool mem_to_mem)
> {
> int r;
> - enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
> + enum omap_overlay_caps caps;
> enum omap_channel channel;
>
> + if (plane == OMAP_DSS_WB)
> + return -EINVAL;
> +
> + caps = dss_feat_get_overlay_caps(plane);
> channel = dispc_ovl_get_channel_out(plane);
>
> DSSDBG("dispc_ovl_setup %d, pa %pad, pa_uv %pad, sw %d, %d,%d, %dx%d ->"
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] backlight: ktz8866: add slave handler
From: Pengyu Luo @ 2025-04-18 18:14 UTC (permalink / raw)
To: krzk
Cc: conor+dt, danielt, deller, devicetree, dri-devel, jingoohan1,
krzk+dt, lee, linux-fbdev, linux-kernel, linux-leds, lujianhua000,
mitltlatltl, pavel, robh
In-Reply-To: <eb23737f-5b6c-47fd-8b39-637e059bd5f1@kernel.org>
On Mon, Apr 7, 2025 at 6:00 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 07/04/2025 11:51, Pengyu Luo wrote:
> > Kinetic ktz8866, found in many android devices, nowadays, some oem use
> > dual ktz8866 to support a larger panel and higher brightness, original
> > driver would only handle half backlight region on these devices,
> > registering it twice is unreasonable, so adding the slave handler to
> > support it.
[...]
>
> I wrote on IRC - phandle to express the relationship between hardware -
> and I do not see it implemented.
>
> If you have devices depending on each other, you need to express it -
> for links, for resource dependencies etc. phandle is for that usually.
> Or OF graph. Or parent-child relationship.
>
I got you now, as a non-native speaker, I often misunderstood the first
time, you expected that accessing node phandle in relationship or graph
way, I did only access node phandle regardless of relationship or graph
description, I only implied it in compatible string, but there would be
a better way.
> You did not provide any description of the hardware in the binding, so I
> really do not have any idea what is this setup thus how to represent it.
> Use hardware terms, diagrams etc to explain the hardware in the bindings
> commit. What are the addresses? Are there any shared resources? What
> buses are devices sitting on, etc.
Agree.
Best wishes,
Pengyu
^ permalink raw reply
* Re: [PATCH 2/4] backlight: ktz8866: add slave handler
From: Pengyu Luo @ 2025-04-18 18:17 UTC (permalink / raw)
To: krzk
Cc: conor+dt, danielt, deller, devicetree, dri-devel, jingoohan1,
krzk+dt, lee, linux-fbdev, linux-kernel, linux-leds, lujianhua000,
mitltlatltl, pavel, robh
In-Reply-To: <eb23737f-5b6c-47fd-8b39-637e059bd5f1@kernel.org>
On Tue, Apr 8, 2025 at 12:27 AM Daniel Thompson <daniel@riscstar.com> wrote:
> On Mon, Apr 07, 2025 at 05:51:17PM +0800, Pengyu Luo wrote:
> > Kinetic ktz8866, found in many android devices, nowadays, some oem use
> > dual ktz8866 to support a larger panel and higher brightness, original
> > driver would only handle half backlight region on these devices,
> > registering it twice is unreasonable, so adding the slave handler to
> > support it.
>
> Is there anything unique about KTZ8866 that allows it to be used like
> this? I think it would be better to add support for secondary backlight
> controllers into the backlight framework, rather than having to
> implement driver specific hacks for every backlight controller that
> appears in a primary/secondary configuration.
>
According to my understanding, if I add the new api to backlight framework,
with a minimal modification, then I either do A or do B(I doubt it is my
fixed mindset)
A:
Tied two devices, registering the primary and the secondary device during
one probe, to do that, I access another KTZ8866 when probing. Those hack
is still here, that doesn't seem to help.
B:
Uncoupled, probing separately, the later one is registered as the
secondary one. Brightness control is a little uncoupled, there are two
sysfs, I doubt if userspace programs will write brightness to two
devices. Then we need synchronization, write primary => write primary
and write secondary, viceversa.
> Also, the kernel seeks to avoid adding new instances of master/slave
> terminology. See the coding style doc for suggested alternatives:
> https://www.kernel.org/doc/html/latest/process/coding-style.html#naming
>
Agree.
Best wishes,
Pengyu
^ permalink raw reply
* Re: [PATCH 2/4] backlight: ktz8866: add slave handler
From: Pengyu Luo @ 2025-04-18 18:19 UTC (permalink / raw)
To: daniel
Cc: conor+dt, danielt, deller, devicetree, dri-devel, jingoohan1,
krzk+dt, lee, linux-fbdev, linux-kernel, linux-leds, lujianhua000,
mitltlatltl, pavel, robh
In-Reply-To: <Z_P9AEGq2sBYShgv@aspen.lan>
On Tue, Apr 8, 2025 at 12:27 AM Daniel Thompson <daniel@riscstar.com> wrote:
> On Mon, Apr 07, 2025 at 05:51:17PM +0800, Pengyu Luo wrote:
> > Kinetic ktz8866, found in many android devices, nowadays, some oem use
> > dual ktz8866 to support a larger panel and higher brightness, original
> > driver would only handle half backlight region on these devices,
> > registering it twice is unreasonable, so adding the slave handler to
> > support it.
>
> Is there anything unique about KTZ8866 that allows it to be used like
> this? I think it would be better to add support for secondary backlight
> controllers into the backlight framework, rather than having to
> implement driver specific hacks for every backlight controller that
> appears in a primary/secondary configuration.
>
According to my understanding, if I add the new api to backlight framework,
with a minimal modification, then I either do A or do B(I doubt it is my
fixed mindset)
A:
Tied two devices, registering the primary and the secondary device during
one probe, to do that, I access another KTZ8866 when probing. Those hack
is still here, that doesn't seem to help.
B:
Uncoupled, probing separately, the later one is registered as the
secondary one. Brightness control is a little uncoupled, there are two
sysfs, I doubt if userspace programs will write brightness to two
devices. Then we need synchronization, write primary => write primary
and write secondary, viceversa.
> Also, the kernel seeks to avoid adding new instances of master/slave
> terminology. See the coding style doc for suggested alternatives:
> https://www.kernel.org/doc/html/latest/process/coding-style.html#naming
>
Agree.
Best wishes,
Pengyu
^ permalink raw reply
* [PATCH] staging: sm750fb: change `enum dpms` to snake_case
From: Eric Florin @ 2025-04-23 4:03 UTC (permalink / raw)
To: teddy.wang
Cc: sudipm.mukherjee, gregkh, linux-fbdev, linux-staging,
linux-kernel, Eric Florin
Change the entries in `enum dpms` to snake_case in order to conform to
kernel code styles as reported by checkpatch.pl
CHECK: Avoid CamelCase: <crtDPMS_ON>
CHECK: Avoid CamelCase: <crtDPMS_STANDBY>
CHECK: Avoid CamelCase: <crtDPMS_SUSPEND>
CHECK: Avoid CamelCase: <crtDPMS_OFF>
Signed-off-by: Eric Florin <ericflorin.kernel@gmail.com>
---
drivers/staging/sm750fb/ddk750_power.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_power.h b/drivers/staging/sm750fb/ddk750_power.h
index 63c9e8b6ffb3..5cbb11986bb8 100644
--- a/drivers/staging/sm750fb/ddk750_power.h
+++ b/drivers/staging/sm750fb/ddk750_power.h
@@ -3,10 +3,10 @@
#define DDK750_POWER_H__
enum dpms {
- crtDPMS_ON = 0x0,
- crtDPMS_STANDBY = 0x1,
- crtDPMS_SUSPEND = 0x2,
- crtDPMS_OFF = 0x3,
+ CRT_DPMS_ON = 0x0,
+ CRT_DPMS_STANDBY = 0x1,
+ CRT_DPMS_SUSPEND = 0x2,
+ CRT_DPMS_OFF = 0x3,
};
#define set_DAC(off) { \
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] fbdev: via: use new GPIO line value setter callbacks
From: Bartosz Golaszewski @ 2025-04-23 15:14 UTC (permalink / raw)
To: Florian Tobias Schandinat, Helge Deller, Linus Walleij,
Bartosz Golaszewski
Cc: linux-fbdev, dri-devel, linux-kernel, linux-gpio,
Bartosz Golaszewski
In-Reply-To: <20250408-gpiochip-set-rv-video-v1-1-200ea4d24a29@linaro.org>
On Tue, Apr 8, 2025 at 9:42 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> Commit 98ce1eb1fd87e ("gpiolib: introduce gpio_chip setters that return
> values") added new line setter callbacks to struct gpio_chip. They allow
> to indicate failures to callers. We're in the process of converting all
> GPIO controllers to using them before removing the old ones.
> ---
Gentle ping.
Bart
^ permalink raw reply
* Re: [PATCH] fbdev: via: use new GPIO line value setter callbacks
From: Linus Walleij @ 2025-04-24 8:52 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Florian Tobias Schandinat, Helge Deller, linux-fbdev, dri-devel,
linux-kernel, linux-gpio, Bartosz Golaszewski
In-Reply-To: <20250408-gpiochip-set-rv-video-v1-1-200ea4d24a29@linaro.org>
On Tue, Apr 8, 2025 at 9:43 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] fbdev: via: use new GPIO line value setter callbacks
From: Helge Deller @ 2025-04-24 10:39 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: Florian Tobias Schandinat, linux-fbdev, dri-devel, linux-kernel,
linux-gpio, Bartosz Golaszewski
In-Reply-To: <CACRpkdY0d_a8qzN2bJD+yzZ0P_twwPM21yV771YoABuVQzXAUg@mail.gmail.com>
On 4/24/25 10:52, Linus Walleij wrote:
> On Tue, Apr 8, 2025 at 9:43 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> struct gpio_chip now has callbacks for setting line values that return
>> an integer, allowing to indicate failures. Convert the driver to using
>> them.
>>
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
added to fbdev for-next tree.
Thanks!
Helge
^ permalink raw reply
* [PATCH v1 1/1] atyfb: Remove unused PCI vendor ID
From: Andy Shevchenko @ 2025-04-24 11:56 UTC (permalink / raw)
To: Andy Shevchenko, linux-fbdev, dri-devel, linux-kernel; +Cc: Helge Deller
The custom definition of PCI vendor ID in video/mach64.h is unused.
Remove it. Note, that the proper one is available in pci_ids.h.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/video/mach64.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/include/video/mach64.h b/include/video/mach64.h
index d96e3c189634..f1709f7c8421 100644
--- a/include/video/mach64.h
+++ b/include/video/mach64.h
@@ -934,9 +934,6 @@
#define MEM_BNDRY_EN 0x00040000
#define ONE_MB 0x100000
-/* ATI PCI constants */
-#define PCI_ATI_VENDOR_ID 0x1002
-
/* CNFG_CHIP_ID register constants */
#define CFG_CHIP_TYPE 0x0000FFFF
--
2.47.2
^ permalink raw reply related
* Re: [PATCH][next] fbdev/carminefb: Fix spelling mistake of CARMINE_TOTAL_DIPLAY_MEM
From: Helge Deller @ 2025-04-24 22:12 UTC (permalink / raw)
To: Colin Ian King, linux-fbdev, dri-devel; +Cc: kernel-janitors, linux-kernel
In-Reply-To: <20250418125135.539908-1-colin.i.king@gmail.com>
On 4/18/25 14:51, Colin Ian King wrote:
> There is a spelling mistake in macro CARMINE_TOTAL_DIPLAY_MEM. Fix it.
>
> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> ---
> drivers/video/fbdev/carminefb.c | 8 ++++----
> drivers/video/fbdev/carminefb.h | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v1 1/1] atyfb: Remove unused PCI vendor ID
From: Helge Deller @ 2025-04-24 22:15 UTC (permalink / raw)
To: Andy Shevchenko, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20250424115652.2451062-1-andriy.shevchenko@linux.intel.com>
On 4/24/25 13:56, Andy Shevchenko wrote:
> The custom definition of PCI vendor ID in video/mach64.h is unused.
> Remove it. Note, that the proper one is available in pci_ids.h.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> include/video/mach64.h | 3 ---
> 1 file changed, 3 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] fbdev/nvidiafb: Correct const string length in nvidiafb_setup()
From: Helge Deller @ 2025-04-24 22:18 UTC (permalink / raw)
To: Zijun Hu, Antonino Daplas; +Cc: linux-fbdev, dri-devel, linux-kernel, Zijun Hu
In-Reply-To: <20250407-fix_nvidia-v1-1-843f8d031c7d@quicinc.com>
On 4/7/25 13:55, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> The actual length of const string "noaccel" is 7, but the strncmp()
> branch in nvidiafb_setup() wrongly hard codes it as 6.
>
> Fix by using actual length 7 as argument of the strncmp().
>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
> drivers/video/fbdev/nvidia/nvidia.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH] fbdev/nvidiafb: Correct const string length in nvidiafb_setup()
From: Jani Nikula @ 2025-04-25 8:55 UTC (permalink / raw)
To: Zijun Hu, Antonino Daplas, Helge Deller
Cc: Zijun Hu, linux-fbdev, dri-devel, linux-kernel, Zijun Hu
In-Reply-To: <20250407-fix_nvidia-v1-1-843f8d031c7d@quicinc.com>
On Mon, 07 Apr 2025, Zijun Hu <zijun_hu@icloud.com> wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> The actual length of const string "noaccel" is 7, but the strncmp()
> branch in nvidiafb_setup() wrongly hard codes it as 6.
>
> Fix by using actual length 7 as argument of the strncmp().
>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
> drivers/video/fbdev/nvidia/nvidia.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/nvidia/nvidia.c b/drivers/video/fbdev/nvidia/nvidia.c
> index 8900f181f1952acd2acc16a6ab49a5a42ec056ac..cfaf9454014d8161bedc3598fb68855e04ea9408 100644
> --- a/drivers/video/fbdev/nvidia/nvidia.c
> +++ b/drivers/video/fbdev/nvidia/nvidia.c
> @@ -1484,7 +1484,7 @@ static int nvidiafb_setup(char *options)
> flatpanel = 1;
> } else if (!strncmp(this_opt, "hwcur", 5)) {
> hwcur = 1;
> - } else if (!strncmp(this_opt, "noaccel", 6)) {
> + } else if (!strncmp(this_opt, "noaccel", 7)) {
> noaccel = 1;
> } else if (!strncmp(this_opt, "noscale", 7)) {
> noscale = 1;
A further cleanup could be to replace all of the strncmp's with
str_has_prefix(this_opt, "...") to avoid this class of errors
altogether. It also returns the length of the prefix on match, and that
can be used to drop more magic numbers.
BR,
Jani.
>
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250407-fix_nvidia-a9d72c98a808
>
> Best regards,
--
Jani Nikula, Intel
^ permalink raw reply
* Re: [PATCH] fbdev/nvidiafb: Correct const string length in nvidiafb_setup()
From: Zijun Hu @ 2025-04-25 11:21 UTC (permalink / raw)
To: Jani Nikula, Antonino Daplas, Helge Deller
Cc: linux-fbdev, dri-devel, linux-kernel, Zijun Hu
In-Reply-To: <87o6wky613.fsf@intel.com>
On 2025/4/25 16:55, Jani Nikula wrote:
>> } else if (!strncmp(this_opt, "noscale", 7)) {
>> noscale = 1;
> A further cleanup could be to replace all of the strncmp's with
> str_has_prefix(this_opt, "...") to avoid this class of errors
> altogether. It also returns the length of the prefix on match, and that
> can be used to drop more magic numbers.
very agree with your point. may use strstarts().
there are many strncmp() usages with long const string and hardcoded
string length. some usages are wrong.
^ permalink raw reply
* [PATCH] video: fbdev: arkfb: Cast ics5342_init() allocation type
From: Kees Cook @ 2025-04-26 6:23 UTC (permalink / raw)
To: Helge Deller
Cc: Kees Cook, Javier Martinez Canillas, Thomas Zimmermann, Zheyu Ma,
Samuel Thibault, Jiapeng Chong, linux-fbdev, dri-devel,
linux-kernel, linux-hardening
In preparation for making the kmalloc family of allocators type aware,
we need to make sure that the returned type from the allocation matches
the type of the variable being assigned. (Before, the allocator would
always return "void *", which can be implicitly cast to any pointer type.)
The assigned type is "struct dac_info *" but the returned type will be
"struct ics5342_info *", which has a larger allocation size. This is
by design, as struct ics5342_info contains struct dac_info as its first
member. Cast the allocation type to match the assignment.
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Helge Deller <deller@gmx.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Zheyu Ma <zheyuma97@gmail.com>
Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
Cc: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc: <linux-fbdev@vger.kernel.org>
Cc: <dri-devel@lists.freedesktop.org>
---
drivers/video/fbdev/arkfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index 082501feceb9..7d131e3d159a 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -431,7 +431,7 @@ static struct dac_ops ics5342_ops = {
static struct dac_info * ics5342_init(dac_read_regs_t drr, dac_write_regs_t dwr, void *data)
{
- struct dac_info *info = kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
+ struct dac_info *info = (struct dac_info *)kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
if (! info)
return NULL;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] video: fbdev: arkfb: Cast ics5342_init() allocation type
From: Helge Deller @ 2025-04-26 11:32 UTC (permalink / raw)
To: Kees Cook
Cc: Javier Martinez Canillas, Thomas Zimmermann, Zheyu Ma,
Samuel Thibault, Jiapeng Chong, linux-fbdev, dri-devel,
linux-kernel, linux-hardening
In-Reply-To: <20250426062305.work.819-kees@kernel.org>
On 4/26/25 08:23, Kees Cook wrote:
> In preparation for making the kmalloc family of allocators type aware,
> we need to make sure that the returned type from the allocation matches
> the type of the variable being assigned. (Before, the allocator would
> always return "void *", which can be implicitly cast to any pointer type.)
>
> The assigned type is "struct dac_info *" but the returned type will be
> "struct ics5342_info *", which has a larger allocation size. This is
> by design, as struct ics5342_info contains struct dac_info as its first
> member. Cast the allocation type to match the assignment.
>
> Signed-off-by: Kees Cook <kees@kernel.org>
Thanks Kees!
I applied your patch, but wouldn't this untested patch be cleaner and fulfill the
same purpose to match a kzalloc return type?
diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index 7d131e3d159a..a57c8a992e11 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -431,7 +431,8 @@ static struct dac_ops ics5342_ops = {
static struct dac_info * ics5342_init(dac_read_regs_t drr, dac_write_regs_t dwr, void *data)
{
- struct dac_info *info = (struct dac_info *)kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
+ struct ics5342_info *ics_info = kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
+ struct dac_info *info = &ics_info->dac;
if (! info)
Helge
---
> Cc: Helge Deller <deller@gmx.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Zheyu Ma <zheyuma97@gmail.com>
> Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Cc: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> Cc: <linux-fbdev@vger.kernel.org>
> Cc: <dri-devel@lists.freedesktop.org>
> ---
> drivers/video/fbdev/arkfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
> index 082501feceb9..7d131e3d159a 100644
> --- a/drivers/video/fbdev/arkfb.c
> +++ b/drivers/video/fbdev/arkfb.c
> @@ -431,7 +431,7 @@ static struct dac_ops ics5342_ops = {
>
> static struct dac_info * ics5342_init(dac_read_regs_t drr, dac_write_regs_t dwr, void *data)
> {
> - struct dac_info *info = kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
> + struct dac_info *info = (struct dac_info *)kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
>
> if (! info)
> return NULL;
^ permalink raw reply related
* Re: [PATCH RFC 1/1] vgacon: Add check for vc_origin address range in vgacon_scroll()
From: Helge Deller @ 2025-04-27 15:34 UTC (permalink / raw)
To: GONG Ruiqi, Greg Kroah-Hartman, Jiri Slaby, linux-fbdev
Cc: security, Kees Cook, Yi Yang, Lu Jialin, Xiu Jianfeng
In-Reply-To: <20250427025303.888320-2-gongruiqi1@huawei.com>
On 4/27/25 04:53, GONG Ruiqi wrote:
> Our in-house Syzkaller reported the following BUG (twice), which we
> believed was the same issue with [1]:
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in vcs_scr_readw+0xc2/0xd0 drivers/tty/vt/vt.c:4740
> Read of size 2 at addr ffff88800f5bef60 by task syz.7.2620/12393
> ...
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0x72/0xa0 lib/dump_stack.c:106
> print_address_description.constprop.0+0x6b/0x3d0 mm/kasan/report.c:364
> print_report+0xba/0x280 mm/kasan/report.c:475
> kasan_report+0xa9/0xe0 mm/kasan/report.c:588
> vcs_scr_readw+0xc2/0xd0 drivers/tty/vt/vt.c:4740
> vcs_write_buf_noattr drivers/tty/vt/vc_screen.c:493 [inline]
> vcs_write+0x586/0x840 drivers/tty/vt/vc_screen.c:690
> vfs_write+0x219/0x960 fs/read_write.c:584
> ksys_write+0x12e/0x260 fs/read_write.c:639
> do_syscall_x64 arch/x86/entry/common.c:51 [inline]
> do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81
> entry_SYSCALL_64_after_hwframe+0x78/0xe2
> ...
> </TASK>
>
> Allocated by task 5614:
> kasan_save_stack+0x20/0x40 mm/kasan/common.c:45
> kasan_set_track+0x25/0x30 mm/kasan/common.c:52
> ____kasan_kmalloc mm/kasan/common.c:374 [inline]
> __kasan_kmalloc+0x8f/0xa0 mm/kasan/common.c:383
> kasan_kmalloc include/linux/kasan.h:201 [inline]
> __do_kmalloc_node mm/slab_common.c:1007 [inline]
> __kmalloc+0x62/0x140 mm/slab_common.c:1020
> kmalloc include/linux/slab.h:604 [inline]
> kzalloc include/linux/slab.h:721 [inline]
> vc_do_resize+0x235/0xf40 drivers/tty/vt/vt.c:1193
> vgacon_adjust_height+0x2d4/0x350 drivers/video/console/vgacon.c:1007
> vgacon_font_set+0x1f7/0x240 drivers/video/console/vgacon.c:1031
> con_font_set drivers/tty/vt/vt.c:4628 [inline]
> con_font_op+0x4da/0xa20 drivers/tty/vt/vt.c:4675
> vt_k_ioctl+0xa10/0xb30 drivers/tty/vt/vt_ioctl.c:474
> vt_ioctl+0x14c/0x1870 drivers/tty/vt/vt_ioctl.c:752
> tty_ioctl+0x655/0x1510 drivers/tty/tty_io.c:2779
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:871 [inline]
> __se_sys_ioctl+0x12d/0x190 fs/ioctl.c:857
> do_syscall_x64 arch/x86/entry/common.c:51 [inline]
> do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81
> entry_SYSCALL_64_after_hwframe+0x78/0xe2
>
> Last potentially related work creation:
> kasan_save_stack+0x20/0x40 mm/kasan/common.c:45
> __kasan_record_aux_stack+0x94/0xa0 mm/kasan/generic.c:492
> __call_rcu_common.constprop.0+0xc3/0xa10 kernel/rcu/tree.c:2713
> netlink_release+0x620/0xc20 net/netlink/af_netlink.c:802
> __sock_release+0xb5/0x270 net/socket.c:663
> sock_close+0x1e/0x30 net/socket.c:1425
> __fput+0x408/0xab0 fs/file_table.c:384
> __fput_sync+0x4c/0x60 fs/file_table.c:465
> __do_sys_close fs/open.c:1580 [inline]
> __se_sys_close+0x68/0xd0 fs/open.c:1565
> do_syscall_x64 arch/x86/entry/common.c:51 [inline]
> do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81
> entry_SYSCALL_64_after_hwframe+0x78/0xe2
>
> Second to last potentially related work creation:
> kasan_save_stack+0x20/0x40 mm/kasan/common.c:45
> __kasan_record_aux_stack+0x94/0xa0 mm/kasan/generic.c:492
> __call_rcu_common.constprop.0+0xc3/0xa10 kernel/rcu/tree.c:2713
> netlink_release+0x620/0xc20 net/netlink/af_netlink.c:802
> __sock_release+0xb5/0x270 net/socket.c:663
> sock_close+0x1e/0x30 net/socket.c:1425
> __fput+0x408/0xab0 fs/file_table.c:384
> task_work_run+0x154/0x240 kernel/task_work.c:239
> exit_task_work include/linux/task_work.h:45 [inline]
> do_exit+0x8e5/0x1320 kernel/exit.c:874
> do_group_exit+0xcd/0x280 kernel/exit.c:1023
> get_signal+0x1675/0x1850 kernel/signal.c:2905
> arch_do_signal_or_restart+0x80/0x3b0 arch/x86/kernel/signal.c:310
> exit_to_user_mode_loop kernel/entry/common.c:111 [inline]
> exit_to_user_mode_prepare include/linux/entry-common.h:328 [inline]
> __syscall_exit_to_user_mode_work kernel/entry/common.c:207 [inline]
> syscall_exit_to_user_mode+0x1b3/0x1e0 kernel/entry/common.c:218
> do_syscall_64+0x66/0x110 arch/x86/entry/common.c:87
> entry_SYSCALL_64_after_hwframe+0x78/0xe2
>
> The buggy address belongs to the object at ffff88800f5be000
> which belongs to the cache kmalloc-2k of size 2048
> The buggy address is located 2656 bytes to the right of
> allocated 1280-byte region [ffff88800f5be000, ffff88800f5be500)
>
> ...
>
> Memory state around the buggy address:
> ffff88800f5bee00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88800f5bee80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff88800f5bef00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff88800f5bef80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88800f5bf000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
> By analyzing the vmcore, we found that vc->vc_origin was somehow placed
> one line prior to vc->vc_screenbuf when vc was in KD_TEXT mode, and
> further writings to /dev/vcs caused out-of-bounds reads (and writes
> right after) in vcs_write_buf_noattr().
>
> Our further experiments show that in most cases, vc->vc_origin equals to
> vga_vram_base when the console is in KD_TEXT mode, and it's around
> vc->vc_screenbuf for the KD_GRAPHICS mode. But via triggerring a
> TIOCL_SETVESABLANK ioctl beforehand, we can make vc->vc_origin be around
> vc->vc_screenbuf while the console is in KD_TEXT mode, and then by
> writing the special 'ESC M' control sequence to the tty certain times
> (depends on the value of `vc->state.y - vc->vc_top`), we can eventually
> move vc->vc_origin prior to vc->vc_screenbuf. Here's the PoC, tested on
> QEMU:
>
> ```
> int main() {
> const int RI_NUM = 10; // should be greater than `vc->state.y - vc->vc_top`
> int tty_fd, vcs_fd;
> const char *tty_path = "/dev/tty0";
> const char *vcs_path = "/dev/vcs";
> const char escape_seq[] = "\x1bM"; // ESC + M
> const char trigger_seq[] = "Let's trigger an OOB write.";
> struct vt_sizes vt_size = { 70, 2 };
> int blank = TIOCL_BLANKSCREEN;
>
> tty_fd = open(tty_path, O_RDWR);
>
> char vesa_mode[] = { TIOCL_SETVESABLANK, 1 };
> ioctl(tty_fd, TIOCLINUX, vesa_mode);
>
> ioctl(tty_fd, TIOCLINUX, &blank);
> ioctl(tty_fd, VT_RESIZE, &vt_size);
>
> for (int i = 0; i < RI_NUM; ++i)
> write(tty_fd, escape_seq, sizeof(escape_seq) - 1);
>
> vcs_fd = open(vcs_path, O_RDWR);
> write(vcs_fd, trigger_seq, sizeof(trigger_seq));
>
> close(vcs_fd);
> close(tty_fd);
> return 0;
> }
> ```
>
> To solve this problem, add an address range validation check in
> vgacon_scroll(), ensuring vc->vc_origin never precedes vc_screenbuf.
>
> Reported-by: syzbot+9c09fda97a1a65ea859b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9c09fda97a1a65ea859b [1]
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Co-developed-by: Yi Yang <yiyang13@huawei.com>
> Signed-off-by: Yi Yang <yiyang13@huawei.com>
> Signed-off-by: GONG Ruiqi <gongruiqi1@huawei.com>
> ---
> drivers/video/console/vgacon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 37bd18730fe0..f9cdbf8c53e3 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -1168,7 +1168,7 @@ static bool vgacon_scroll(struct vc_data *c, unsigned int t, unsigned int b,
> c->vc_screenbuf_size - delta);
> c->vc_origin = vga_vram_end - c->vc_screenbuf_size;
> vga_rolled_over = 0;
> - } else
> + } else if (oldo - delta >= (unsigned long)c->vc_screenbuf)
> c->vc_origin -= delta;
> c->vc_scr_end = c->vc_origin + c->vc_screenbuf_size;
> scr_memsetw((u16 *) (c->vc_origin), c->vc_video_erase_char,
The patch is not wrong, but I'm not yet sure if it hides another issue.
I've applied it to the fbdev tree for now (unless Greg wants to handle it).
Thanks!
Helge
^ 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