Claude review: drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks

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

Patch Review

**BUG (critical): `memcmp` compares wrong addresses**

```c
while (!ret && 0 == memcmp(&edid_header, &buf, 8) && retry_cnt-- > 0) {
```

`buf` is a `u8 *` (pointer). Taking `&buf` gives the address of the pointer variable on the stack, **not** the address of the buffer contents. This means `memcmp` compares 8 bytes of stack memory (the pointer value + whatever follows) against the EDID header, which will almost never match. The retry loop will effectively never execute. Similarly, `&edid_header` works by accident here since `edid_header` is an array (so `&edid_header == edid_header` address-wise), but for consistency both should just be `edid_header` and `buf`:

```c
while (!ret && memcmp(edid_header, buf, 8) == 0 && retry_cnt-- > 0) {
```

Also, the Yoda-style `0 == memcmp(...)` is not idiomatic kernel style.

**BUG: Missing `edid_data` cleanup in `lt9611uxc_remove`**

The patch adds `edid_data` caching but never frees it in `lt9611uxc_remove()`. If the device is removed while a display is connected (and EDID is cached), the `drm_edid` allocation leaks. `lt9611uxc_remove` needs:

```c
drm_edid_free(lt9611uxc->edid_data);
```

**Issue: Missing `regmap_write(0xb00b, 0x10)` for blocks 2–3**

The original code writes `regmap_write(lt9611uxc->regmap, 0xb00b, 0x10)` before reading an EDID block. The new `lt9611uxc_read_edid_block()` helper omits this write entirely. For blocks 0 and 1 this may work because the old code path previously set it, but the intent of that register write is unclear and dropping it silently could cause issues. If register `0xb00b` is needed to set up EDID reading, it should be included in `lt9611uxc_read_edid_block`. If it's not needed, its removal from the block 0/1 path should be explicitly noted in the commit message.

**Issue: Concurrency on `edid_data` cache**

`edid_data` is read/written from `lt9611uxc_bridge_edid_read()` and `lt9611uxc_hpd_work()` without any locking. The HPD work function clears `edid_data` while `edid_read` could be simultaneously accessing it. The `ocm_lock` mutex is held for `hdmi_connected` access but the new `edid_data` manipulation in `lt9611uxc_hpd_work` happens *outside* that lock:

```c
mutex_unlock(&lt9611uxc->ocm_lock);

if (!connected) {
    lt9611uxc->edid_read = false;
    drm_edid_free(lt9611uxc->edid_data);
    lt9611uxc->edid_data = NULL;
}
```

This creates a TOCTOU race: `lt9611uxc_bridge_edid_read` could see `edid_data != NULL`, then HPD work frees it, then `edid_read` calls `drm_edid_dup` on freed memory. The cache operations need to be protected (either extend the existing `ocm_lock` scope or add separate locking).

**Issue: `edid_read` flag set outside lock**

```c
lt9611uxc->edid_read = false;
```

This is set in `lt9611uxc_hpd_work` without holding `ocm_lock`, but it's set by the IRQ handler under `ocm_lock` (line 150 in the original). Inconsistent locking on a shared flag.

**Nit: Coding style**

Missing space before modulo operator:
```c
regmap_write(lt9611uxc->regmap, 0xb00a, (block%2) * EDID_BLOCK_SIZE);
```
Should be `(block % 2)`.

**Nit: Unnecessary double-space**

```c
u8 *buf,  size_t len)
```

Two spaces before `size_t`.

**Nit: Return value masking**

```c
ret = regmap_noinc_read(lt9611uxc->regmap, 0xb0b0, buf, len);
if (ret) {
    dev_err(lt9611uxc->dev, "edid block %d read failed: %d\n", block, ret);
    lt9611uxc_unlock(lt9611uxc);
    return -EINVAL;
}
```

This replaces `ret` (which is the actual regmap error code) with `-EINVAL`. It should just `return ret` to propagate the real error. Alternatively, use a `goto` with a single unlock path to avoid the duplicated `lt9611uxc_unlock` call.

**Design: Retry logic is fragile**

The retry checks whether block 2 data still looks like block 0 (EDID header) as a staleness indicator. This is clever but fragile — if block 2 legitimately starts with `00 FF FF FF FF FF FF 00` (very unlikely for a CEA extension but not formally impossible), the retry loop would spin all 10 iterations pointlessly. A comment explaining the hardware behavior would help. Also, the 10 retries × 100ms = 1 second total, plus the initial 100ms, gives 1.1 seconds maximum. It would be good to document whether this is sufficient for the hardware.

**Summary of required changes:**
1. Fix `&buf` → `buf` in the `memcmp` call (critical bug)
2. Add `drm_edid_free(lt9611uxc->edid_data)` in `lt9611uxc_remove` (leak)
3. Add locking around `edid_data` accesses (race condition)
4. Clarify or restore the `0xb00b` register write
5. Minor style fixes

---
Generated by Claude Code Patch Reviewer