Re: [PATCH] staging: sm750fb: Replace busy-wait loop with udelay()

[Dan Carpenter <dan.carpenter@linaro.org>]

Hi Sudip,

What the status on a replacement for the sm750fb driver?

On Sun, Mar 15, 2026 at 11:05:32PM +0800, OaroraEtimis wrote:
> The empty for-loop in sw_i2c_wait() triggers a -Wunused-but-set-variable
> warning and can be optimized away by modern compilers.
> 
> Fix this by replacing the unreliable loop with a standard udelay(2).
> This also cleans up the unused 'tmp' variable and outdated comments.
> Include <linux/delay.h> to resolve the implicit function declaration.
> 
> Signed-off-by: OaroraEtimis <OaroraEtimis@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_swi2c.c | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index 0ef8d4ff2ef9..d0aeb917be92 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:
> @@ -80,24 +81,7 @@ static unsigned long sw_i2c_data_gpio_data_dir_reg = GPIO_DATA_DIRECTION;
>   */
>  static void sw_i2c_wait(void)
>  {
> -	/* find a bug:
> -	 * peekIO method works well before suspend/resume
> -	 * but after suspend, peekIO(0x3ce,0x61) & 0x10
> -	 * always be non-zero,which makes the while loop
> -	 * never finish.
> -	 * use non-ultimate for loop below is safe
> -	 */
> -
> -    /* Change wait algorithm to use PCI bus clock,
> -     * 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(2);

What an interesting thing...  This function has always been nonsense
since it was first merged.  The comment talks about the old code which
did:

	while (peekIO(0x3ce, 0x61) & 0x10)
		;

Which apparently "works well" except that after suspend it becomes
a forever loop so instead of that they have this bonkers "count to 600"
loop.

Your patch looks reasonable, but generally we don't merge that sort
of patch without anyone to test it.  The problem with merging your code
is that it looks so reasonable, that people might assume it's correct.
Meanwhile if we just leave it as-is, everyone can see it's totally buggy
so when people report bugs we'll know exactly where to look.

Sudip was working on a replacement for this driver.  Let's ask him for
advice.

regards,
dan carpenter