* [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).