linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: sm750fb: fix unused tmp in sw_i2c_wait
@ 2026-01-05  2:10 Sun Jian
  2026-01-05  6:28 ` Greg KH
  2026-01-05  7:49 ` [PATCH v2] " Sun Jian
  0 siblings, 2 replies; 6+ messages in thread
From: Sun Jian @ 2026-01-05  2:10 UTC (permalink / raw)
  To: linux-staging; +Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, Sun Jian

clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().

Replace the dummy loop with cpu_relax() to keep the delay loop without
triggering -Wunused-but-set-variable.

No functional change intended.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/staging/sm750fb/ddk750_swi2c.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 0ef8d4ff2ef9..279a1a10f132 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -92,12 +92,11 @@ static void sw_i2c_wait(void)
      * it's more reliable than counter loop ..
      * write 0x61 to 0x3ce and read from 0x3cf
      */
-	int i, tmp;
+	int i;
+
+	for (i = 0; i < 600; i++)
+		cpu_relax();
 
-	for (i = 0; i < 600; i++) {
-		tmp = i;
-		tmp += i;
-	}
 }
 
 /*
-- 
2.43.0


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

* Re: [PATCH] staging: sm750fb: fix unused tmp in sw_i2c_wait
  2026-01-05  2:10 [PATCH] staging: sm750fb: fix unused tmp in sw_i2c_wait Sun Jian
@ 2026-01-05  6:28 ` Greg KH
  2026-01-05  7:49 ` [PATCH v2] " Sun Jian
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2026-01-05  6:28 UTC (permalink / raw)
  To: Sun Jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev

On Mon, Jan 05, 2026 at 10:10:26AM +0800, Sun Jian wrote:
> clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
> 
> Replace the dummy loop with cpu_relax() to keep the delay loop without
> triggering -Wunused-but-set-variable.
> 
> No functional change intended.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_swi2c.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index 0ef8d4ff2ef9..279a1a10f132 100644
> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> @@ -92,12 +92,11 @@ static void sw_i2c_wait(void)
>       * it's more reliable than counter loop ..
>       * write 0x61 to 0x3ce and read from 0x3cf
>       */
> -	int i, tmp;
> +	int i;
> +
> +	for (i = 0; i < 600; i++)
> +		cpu_relax();
>  
> -	for (i = 0; i < 600; i++) {
> -		tmp = i;
> -		tmp += i;
> -	}

You just slowed down this loop a lot, are you sure it still works?  And
really, either way, this isn't the proper way to "sleep", as it will
be different speeds on different cpus/systems, and not work properly at
all.  Can you fix this correctly?

thanks,

greg k-h

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

* [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
  2026-01-05  2:10 [PATCH] staging: sm750fb: fix unused tmp in sw_i2c_wait Sun Jian
  2026-01-05  6:28 ` Greg KH
@ 2026-01-05  7:49 ` Sun Jian
  2026-01-05  8:05   ` Greg KH
  1 sibling, 1 reply; 6+ messages in thread
From: Sun Jian @ 2026-01-05  7:49 UTC (permalink / raw)
  To: linux-staging; +Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, Sun Jian

clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().

sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
dependent timing and fix the warning.

Compile-tested with clang W=1; no hardware available to validate timing.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
v2:
- Replace cpu_relax() delay loop with time-based udelay(1) to avoid
  CPU-dependent timing (per Greg's feedback).

v1:
- Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
---
 drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 0ef8d4ff2ef9..d5843fa69bfa 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -11,6 +11,7 @@
 #include "ddk750_reg.h"
 #include "ddk750_swi2c.h"
 #include "ddk750_power.h"
+#include <linux/delay.h>
 
 /*
  * I2C Software Master Driver:
@@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
      * it's more reliable than counter loop ..
      * write 0x61 to 0x3ce and read from 0x3cf
      */
-	int i, tmp;
-
-	for (i = 0; i < 600; i++) {
-		tmp = i;
-		tmp += i;
-	}
+	udelay(1);
 }
 
 /*
-- 
2.43.0


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

* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
  2026-01-05  7:49 ` [PATCH v2] " Sun Jian
@ 2026-01-05  8:05   ` Greg KH
  2026-01-05  8:57     ` sun jian
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2026-01-05  8:05 UTC (permalink / raw)
  To: Sun Jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev

On Mon, Jan 05, 2026 at 03:49:17PM +0800, Sun Jian wrote:
> clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
> 
> sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
> Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
> dependent timing and fix the warning.

Why is udelay(1) the same here?

> Compile-tested with clang W=1; no hardware available to validate timing.

That's going to prevent us from being able to take this, sorry :(


> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> v2:
> - Replace cpu_relax() delay loop with time-based udelay(1) to avoid
>   CPU-dependent timing (per Greg's feedback).
> 
> v1:
> - Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
> ---
>  drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index 0ef8d4ff2ef9..d5843fa69bfa 100644
> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> @@ -11,6 +11,7 @@
>  #include "ddk750_reg.h"
>  #include "ddk750_swi2c.h"
>  #include "ddk750_power.h"
> +#include <linux/delay.h>

Shouldn't this be at the top of the include lines?

>  
>  /*
>   * I2C Software Master Driver:
> @@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
>       * it's more reliable than counter loop ..
>       * write 0x61 to 0x3ce and read from 0x3cf
>       */
> -	int i, tmp;
> -
> -	for (i = 0; i < 600; i++) {
> -		tmp = i;
> -		tmp += i;
> -	}
> +	udelay(1);

You are ignoring the comments in this function.

Also, if you reduce this to a single call, shouldn't this whole function
be removed?

thanks,

greg k-h

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

* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
  2026-01-05  8:05   ` Greg KH
@ 2026-01-05  8:57     ` sun jian
  2026-01-05  9:12       ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: sun jian @ 2026-01-05  8:57 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev

Hi Greg,

Sorry about that — I mistakenly replied only to you.

As you pointed out, sw_i2c_wait() sits on the bit-banged I2C GPIO
transitions, so changing the delay semantics without hardware validation
is risky. I don't have access to the hardware to validate timing/behavior,
and I can't justify that udelay(1) is equivalent to the existing loop.

Please ignore v2 (and v1). I won't resend a warning-only workaround.

If someone with the hardware can help validate a proper fix (e.g. a
well-justified time-based delay, or reworking this to use a proper I2C
bit-banging helper), I'm happy to revisit.

Thanks,
Sun

On Mon, Jan 5, 2026 at 4:05 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jan 05, 2026 at 03:49:17PM +0800, Sun Jian wrote:
> > clang W=1 warns that 'tmp' is set but not used in sw_i2c_wait().
> >
> > sw_i2c_wait() provides the delay between bit-banged I2C GPIO transitions.
> > Replace the loop-count delay with a time-based udelay(1) to avoid CPU-
> > dependent timing and fix the warning.
>
> Why is udelay(1) the same here?
>
> > Compile-tested with clang W=1; no hardware available to validate timing.
>
> That's going to prevent us from being able to take this, sorry :(
>
>
> >
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > ---
> > v2:
> > - Replace cpu_relax() delay loop with time-based udelay(1) to avoid
> >   CPU-dependent timing (per Greg's feedback).
> >
> > v1:
> > - Used cpu_relax() in the loop to silence -Wunused-but-set-variable.
> > ---
> >  drivers/staging/sm750fb/ddk750_swi2c.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> > index 0ef8d4ff2ef9..d5843fa69bfa 100644
> > --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> > +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> > @@ -11,6 +11,7 @@
> >  #include "ddk750_reg.h"
> >  #include "ddk750_swi2c.h"
> >  #include "ddk750_power.h"
> > +#include <linux/delay.h>
>
> Shouldn't this be at the top of the include lines?
>
> >
> >  /*
> >   * I2C Software Master Driver:
> > @@ -92,12 +93,7 @@ static void sw_i2c_wait(void)
> >       * it's more reliable than counter loop ..
> >       * write 0x61 to 0x3ce and read from 0x3cf
> >       */
> > -     int i, tmp;
> > -
> > -     for (i = 0; i < 600; i++) {
> > -             tmp = i;
> > -             tmp += i;
> > -     }
> > +     udelay(1);
>
> You are ignoring the comments in this function.
>
> Also, if you reduce this to a single call, shouldn't this whole function
> be removed?
>
> thanks,
>
> greg k-h

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

* Re: [PATCH v2] staging: sm750fb: fix unused tmp in sw_i2c_wait
  2026-01-05  8:57     ` sun jian
@ 2026-01-05  9:12       ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2026-01-05  9:12 UTC (permalink / raw)
  To: sun jian; +Cc: linux-staging, sudipm.mukherjee, teddy.wang, linux-fbdev

On Mon, Jan 05, 2026 at 04:57:48PM +0800, sun jian wrote:
> Hi Greg,
> 
> Sorry about that — I mistakenly replied only to you.
> 
> As you pointed out, sw_i2c_wait() sits on the bit-banged I2C GPIO
> transitions, so changing the delay semantics without hardware validation
> is risky. I don't have access to the hardware to validate timing/behavior,
> and I can't justify that udelay(1) is equivalent to the existing loop.
> 
> Please ignore v2 (and v1). I won't resend a warning-only workaround.
> 
> If someone with the hardware can help validate a proper fix (e.g. a
> well-justified time-based delay, or reworking this to use a proper I2C
> bit-banging helper), I'm happy to revisit.

A time-based one is going to be the correct solution as every cpu will
run that "let's count some numbers" loop at a different speed :(

thanks,

greg k-h

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

end of thread, other threads:[~2026-01-05  9:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-05  2:10 [PATCH] staging: sm750fb: fix unused tmp in sw_i2c_wait Sun Jian
2026-01-05  6:28 ` Greg KH
2026-01-05  7:49 ` [PATCH v2] " Sun Jian
2026-01-05  8:05   ` Greg KH
2026-01-05  8:57     ` sun jian
2026-01-05  9:12       ` Greg KH

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