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