[PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()

[PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()

[Anas Iqbal]

Replace udelay() with usleep_range() for a 250 microsecond delay
as recommended by checkpatch.pl. usleep_range() avoids busy
waiting and allows the scheduler to schedule other tasks.

Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>
---
 drivers/staging/fbtft/fb_tinylcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
index 9469248f2c50..51e6493b050f 100644
--- a/drivers/staging/fbtft/fb_tinylcd.c
+++ b/drivers/staging/fbtft/fb_tinylcd.c
@@ -38,10 +38,10 @@ static int init_display(struct fbtft_par *par)
 	write_reg(par, 0xE5, 0x00);
 	write_reg(par, 0xF0, 0x36, 0xA5, 0x53);
 	write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00,
-		       0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
+		  0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
 	write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55);
 	write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
-	udelay(250);
+	usleep_range(250, 500);
 	write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
 
 	return 0;
-- 
2.43.0

Re: [PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()

[Dan Carpenter]

On Wed, Mar 11, 2026 at 02:24:07PM +0000, Anas Iqbal wrote:
> Replace udelay() with usleep_range() for a 250 microsecond delay
> as recommended by checkpatch.pl. usleep_range() avoids busy
> waiting and allows the scheduler to schedule other tasks.
> 
> Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>
> ---
>  drivers/staging/fbtft/fb_tinylcd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
> index 9469248f2c50..51e6493b050f 100644
> --- a/drivers/staging/fbtft/fb_tinylcd.c
> +++ b/drivers/staging/fbtft/fb_tinylcd.c
> @@ -38,10 +38,10 @@ static int init_display(struct fbtft_par *par)
>  	write_reg(par, 0xE5, 0x00);
>  	write_reg(par, 0xF0, 0x36, 0xA5, 0x53);
>  	write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00,
> -		       0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
> +		  0x00, 0x35, 0x33, 0x00, 0x00, 0x00);

Don't do this.
1) It's unrelated.
2) The previous alignment was done deliberate to make it easier to read.
   Checkpatch is not perfect so don't always do what it says.

>  	write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55);
>  	write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
> -	udelay(250);
> +	usleep_range(250, 500);

Do a search for init_display on lore

https://lore.kernel.org/all/?q=init_display

regards,
dan carpenter

Re: [PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()

[Greg KH]

On Wed, Mar 11, 2026 at 02:24:07PM +0000, Anas Iqbal wrote:
> Replace udelay() with usleep_range() for a 250 microsecond delay
> as recommended by checkpatch.pl. usleep_range() avoids busy
> waiting and allows the scheduler to schedule other tasks.
> 
> Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>
> ---
>  drivers/staging/fbtft/fb_tinylcd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c
> index 9469248f2c50..51e6493b050f 100644
> --- a/drivers/staging/fbtft/fb_tinylcd.c
> +++ b/drivers/staging/fbtft/fb_tinylcd.c
> @@ -38,10 +38,10 @@ static int init_display(struct fbtft_par *par)
>  	write_reg(par, 0xE5, 0x00);
>  	write_reg(par, 0xF0, 0x36, 0xA5, 0x53);
>  	write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00,
> -		       0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
> +		  0x00, 0x35, 0x33, 0x00, 0x00, 0x00);

Don't you think the original formatting makes more sense?  And you did
not describe this change in the changelog :(

>  	write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55);
>  	write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE);
> -	udelay(250);
> +	usleep_range(250, 500);

This is a totally different change than above.  And also one that keeps
getting rejected, please see the mailing list archives for details.

thanks,

greg k-h

Re: [PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()

[Andy Shevchenko]

On Wed, Mar 11, 2026 at 02:24:07PM +0000, Anas Iqbal wrote:
> Replace udelay() with usleep_range() for a 250 microsecond delay
> as recommended by checkpatch.pl. usleep_range() avoids busy
> waiting and allows the scheduler to schedule other tasks.

Now, read README here:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/staging/fbtft/README
and act accordingly.

If you have been mentored on this, show the above link to your mentors.

...

>  	write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00,
> -		       0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
> +		  0x00, 0x35, 0x33, 0x00, 0x00, 0x00);

Use the common sense here. This change won't be accepted.

...

> -	udelay(250);
> +	usleep_range(250, 500);

No.

-- 
With Best Regards,
Andy Shevchenko

Claude review: Re: [PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()

[Claude Code Review Bot]

Overall Series Review

Subject: Re: [PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()
Author: Dan Carpenter <dan.carpenter@linaro.org>
Patches: 4
Reviewed: 2026-03-13T14:44:57.584370

---

This is a single-patch series making a simple cleanup change in a staging driver: replacing `udelay(250)` with `usleep_range(250, 500)` in `fb_tinylcd.c`. The intent is reasonable — checkpatch recommends `usleep_range()` over `udelay()` for delays ≥ 10µs — but the patch has an unrelated whitespace change and a potential correctness concern regarding the calling context.

---
Generated by Claude Code Patch Reviewer

Claude review: Re: [PATCH] staging: fbtft: fb_tinylcd: replace udelay() with usleep_range()

[Claude Code Review Bot]

Patch Review

**Unrelated whitespace change included:**

The patch changes the continuation-line alignment of the `write_reg` call at line 40-41, which is unrelated to the `udelay` → `usleep_range` conversion:

```diff
 	write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00,
-		       0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
+		  0x00, 0x35, 0x33, 0x00, 0x00, 0x00);
```

This should either be dropped from this patch or submitted as a separate cleanup. Mixing unrelated changes makes review and bisection harder.

**Sleeping context concern:**

`usleep_range()` can only be called from process/sleepable context — it cannot be used in atomic context (interrupts, spinlock-held sections, etc.). The `init_display` function is called via `fbtft_ops.init_display`, which is typically invoked during probe or from sysfs, so it should be in sleepable context. However, the commit message should mention that this was verified, or the author should confirm that `init_display` is never called from atomic context. Looking at the fbtft framework, this appears safe, but it's worth calling out.

**Upper bound of the range:**

The chosen range `usleep_range(250, 500)` doubles the maximum delay compared to the original. For a post-sleep-exit delay on a display controller, this is likely fine — MIPI DCS `EXIT_SLEEP_MODE` typically requires at least 120ms per spec (this 250µs delay seems like it may already be too short, but that's a pre-existing issue). The 250–500µs range is acceptable.

**Verdict:** The core change is fine but the patch should drop the unrelated whitespace change. A v2 with only the `udelay` → `usleep_range` hunk would be clean and acceptable.

---
Generated by Claude Code Patch Reviewer