Claude review: drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability

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

Patch Review

**Verdict: Has bugs that need fixing.**

**Bug 1 (Critical): Inverted `wait_for_completion_timeout` check**

At lines 868-871 of the patched file:

```c
if (wait_for_completion_timeout(&dsi->pll_stabilized,
                                usecs_to_jiffies(timeout))) {
    dev_err(dsi->dev, "PLL failed to stabilize\n");
    return 0;
}
```

`wait_for_completion_timeout()` returns **0 on timeout** and **>0 on success**. The condition is inverted — the error path runs on success, and the success path runs on timeout. This should be:

```c
if (!wait_for_completion_timeout(...)) {
```

**Bug 2 (Critical): PLL_STABLE interrupt is masked**

Looking at the interrupt handler at line 1596-1602, after `DSIM_INT_SW_RST_RELEASE`, the interrupt mask is set to block everything except `DSIM_INT_RX_DONE`, `DSIM_INT_SFR_FIFO_EMPTY`, `DSIM_INT_SFR_HDR_FIFO_EMPTY`, `DSIM_INT_RX_ECC_ERR`, and `DSIM_INT_SW_RST_RELEASE`. Notably, `DSIM_INT_PLL_STABLE` is **not** in this unmasked set — it remains masked. This means the PLL_STABLE interrupt will never be delivered after reset, and `wait_for_completion_timeout` will always time out.

The mask at line 1597 needs to also include `DSIM_INT_PLL_STABLE`:

```c
unsigned long mask = ~(DSIM_INT_RX_DONE |
                       DSIM_INT_SFR_FIFO_EMPTY |
                       DSIM_INT_SFR_HDR_FIFO_EMPTY |
                       DSIM_INT_RX_ECC_ERR |
                       DSIM_INT_SW_RST_RELEASE |
                       DSIM_INT_PLL_STABLE);
```

(Note: since bug 1 inverts the sense, on current hardware these two bugs may accidentally cancel out — the masked interrupt causes a timeout, which the inverted check treats as success. But that's fragile and wrong.)

**Minor: Typo in comment**

Line 855: `Calcutate` should be `Calculate`.

**Minor: Assumption about clk_data[0] being bus clock**

The comment says "It is assumed that the bus clock is the first clock in the provided bulk clock data." This holds for all current variants (exynos3 has `bus_clk`, exynos4 has `bus_clk`, exynos5433 has `bus_clk`, exynos7870 has `bus`), but for the IMX variants the first clock is also `bus_clk`. Still, this is fragile — if a future driver data entry reorders clocks, this silently breaks. Consider whether a named lookup would be more robust, though this is a minor concern.

**Minor: `usecs_to_jiffies` with small values**

With a typical HZ=250, `usecs_to_jiffies(100)` returns 1 jiffy (4ms), which is already much larger than 100µs. The "gracious lower bound of 100 microseconds" described in the commit message is somewhat misleading since the actual wait granularity is jiffies. This isn't a bug but worth being aware of.

---
Generated by Claude Code Patch Reviewer