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

[Claude Code Review Bot <claude-review@example.com>]

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