staging: sm750fb: remove unused variable

[PATCH v2] staging: sm750fb: remove unused variable

Posted by Onish Sharma 1 week, 3 days ago

Remove the set_all_eng_off flag and its associated cleanup logic.
The variable is redundant as the hardware should be initialized to a
known state regardless of prior usage.

Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Onish Sharma <neharora23587@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.c | 28 ---------------------------
 drivers/staging/sm750fb/ddk750_chip.h |  7 -------
 drivers/staging/sm750fb/sm750.c       |  1 -
 drivers/staging/sm750fb/sm750.h       |  1 -
 4 files changed, 37 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 0bb56bbec43f..553654a77170 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -262,34 +262,6 @@ int ddk750_init_hw(struct initchip_param *p_init_param)
 		reg |= MISC_CTRL_LOCALMEM_RESET;
 		poke32(MISC_CTRL, reg);
 	}
-
-	if (p_init_param->set_all_eng_off == 1) {
-		sm750_enable_2d_engine(0);
-
-		/* Disable Overlay, if a former application left it on */
-		reg = peek32(VIDEO_DISPLAY_CTRL);
-		reg &= ~DISPLAY_CTRL_PLANE;
-		poke32(VIDEO_DISPLAY_CTRL, reg);
-
-		/* Disable video alpha, if a former application left it on */
-		reg = peek32(VIDEO_ALPHA_DISPLAY_CTRL);
-		reg &= ~DISPLAY_CTRL_PLANE;
-		poke32(VIDEO_ALPHA_DISPLAY_CTRL, reg);
-
-		/* Disable alpha plane, if a former application left it on */
-		reg = peek32(ALPHA_DISPLAY_CTRL);
-		reg &= ~DISPLAY_CTRL_PLANE;
-		poke32(ALPHA_DISPLAY_CTRL, reg);
-
-		/* Disable DMA Channel, if a former application left it on */
-		reg = peek32(DMA_ABORT_INTERRUPT);
-		reg |= DMA_ABORT_INTERRUPT_ABORT_1;
-		poke32(DMA_ABORT_INTERRUPT, reg);
-
-		/* Disable DMA Power, if a former application left it on */
-		sm750_enable_dma(0);
-	}
-
 	/* We can add more initialization as needed. */
 
 	return 0;
diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
index ee2e9d90f7dd..2a13debc179f 100644
--- a/drivers/staging/sm750fb/ddk750_chip.h
+++ b/drivers/staging/sm750fb/ddk750_chip.h
@@ -76,13 +76,6 @@ struct initchip_param {
 	 */
 	unsigned short master_clock;
 
-	/*
-	 * 0 = leave all engine state untouched.
-	 * 1 = make sure they are off: 2D, Overlay,
-	 * video alpha, alpha, hardware cursors
-	 */
-	unsigned short set_all_eng_off;
-
 	/*
 	 * 0 = Do not reset the memory controller
 	 * 1 = Reset the memory controller
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c2d2864f135b..127db29883d2 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -848,7 +848,6 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
 	sm750_dev->init_parm.mem_clk = 0;
 	sm750_dev->init_parm.master_clk = 0;
 	sm750_dev->init_parm.power_mode = 0;
-	sm750_dev->init_parm.set_all_eng_off = 0;
 	sm750_dev->init_parm.reset_memory = 1;
 
 	/* defaultly turn g_hwcursor on for both view */
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index c42800313c6a..7ab13da5d7cc 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -44,7 +44,6 @@ struct init_status {
 	ushort chip_clk;
 	ushort mem_clk;
 	ushort master_clk;
-	ushort set_all_eng_off;
 	ushort reset_memory;
 };
 
-- 
2.54.0

Re: [PATCH v2] staging: sm750fb: remove unused variable

Posted by Dan Carpenter 1 week, 3 days ago

On Fri, May 29, 2026 at 03:42:42PM +0530, Onish Sharma wrote:
> Remove the set_all_eng_off flag and its associated cleanup logic.
> The variable is redundant as the hardware should be initialized to a
> known state regardless of prior usage.
> 
> Suggested-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Onish Sharma <neharora23587@gmail.com>
> ---

Sorry, miscommunication.  This breaks the driver.  This is also a bit
more involved than I thought...

There are two structs:

struct init_status {
        ushort power_mode;
        /* below three clocks are in unit of MHZ*/
        ushort chip_clk;
        ushort mem_clk;
        ushort master_clk;
        ushort setAllEngOff;
        ushort reset_memory;
};

And struct initchip_param.  The initchip_param is exactly the same but
with all the struct members renamed and comments added.  They have to
match because we cast back and forth.

Why do we have two different struct that have to be the same?  You might
think it is for API, but as near as I can see that is not the case.
Maybe it was at some point?  We should get rid of one struct.  Which
everyone is API is the one we should keep.  If neither is API then get
rid of init_status and keep initchip_param.

After that we can talk about getting rid of setAllEngOff/set_all_eng_off.

regards,
dan carpenter

Re: [PATCH v2] staging: sm750fb: remove unused variable

Posted by neha arora 1 week, 2 days ago

Hi Dan,

After looking into the structural dependencies and the cross-casting
between init_status and initchip_param, I've decided that this
refactoring is outside the scope of what I want to work on at this
time.
Please feel free to drop my previous patch. I'm going to shift my
focus to other areas.

Regards,
Onish


On Fri, May 29, 2026 at 4:09 PM Dan Carpenter <error27@gmail.com> wrote:
>
> On Fri, May 29, 2026 at 03:42:42PM +0530, Onish Sharma wrote:
> > Remove the set_all_eng_off flag and its associated cleanup logic.
> > The variable is redundant as the hardware should be initialized to a
> > known state regardless of prior usage.
> >
> > Suggested-by: Dan Carpenter <error27@gmail.com>
> > Signed-off-by: Onish Sharma <neharora23587@gmail.com>
> > ---
>
> Sorry, miscommunication.  This breaks the driver.  This is also a bit
> more involved than I thought...
>
> There are two structs:
>
> struct init_status {
>         ushort power_mode;
>         /* below three clocks are in unit of MHZ*/
>         ushort chip_clk;
>         ushort mem_clk;
>         ushort master_clk;
>         ushort setAllEngOff;
>         ushort reset_memory;
> };
>
> And struct initchip_param.  The initchip_param is exactly the same but
> with all the struct members renamed and comments added.  They have to
> match because we cast back and forth.
>
> Why do we have two different struct that have to be the same?  You might
> think it is for API, but as near as I can see that is not the case.
> Maybe it was at some point?  We should get rid of one struct.  Which
> everyone is API is the one we should keep.  If neither is API then get
> rid of init_status and keep initchip_param.
>
> After that we can talk about getting rid of setAllEngOff/set_all_eng_off.
>
> regards,
> dan carpenter
>

Re: [PATCH v2] staging: sm750fb: remove unused variable

Posted by Dan Carpenter 1 week, 3 days ago

On Fri, May 29, 2026 at 01:39:11PM +0300, Dan Carpenter wrote:
> Which everyone is API is the one we should keep.

s/everyone/ever one/

regards,
dan carpenter