Claude review: drm/bridge: Add ITE IT6162 MIPI DSI to HDMI bridge driver

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

Patch Review

**Critical bugs:**

1. **Use-before-init of `dsi` variable** in `it6162_attach_dsi()`:
```c
if (!it6162->dsi) {
    ret = it6162_load_mipi_pars_from_port(it6162, port);
    if (ret <= 0)
        return ret;
    dev_info(dev, "DSI host loaded paras for port %d", port);
    it6162->dsi = dsi;  // BUG: 'dsi' is uninitialized here
}

dsi = devm_mipi_dsi_device_register_full(dev, dsi_host, &info);
```
The assignment `it6162->dsi = dsi` happens before `dsi` is assigned by `devm_mipi_dsi_device_register_full()`. On the first iteration, `dsi` is an uninitialized local variable. The `dsi` registration and the `it6162->dsi` assignment need to be reordered.

2. **Missing `DRM_BRIDGE_OP_HDMI` flag**: The bridge sets ops for `DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_MODES` but implements HDMI-specific callbacks (`hdmi_write_avi_infoframe`, `hdmi_tmds_char_rate_valid`, etc.) that require `DRM_BRIDGE_OP_HDMI` to be set:
```c
it6162->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
                     DRM_BRIDGE_OP_MODES;
```
Without `DRM_BRIDGE_OP_HDMI`, the HDMI connector framework won't use these callbacks. Similarly, `DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME` is needed for the SPD infoframe callbacks to be invoked.

3. **Missing `DRM_BRIDGE_OP_HPD` flag**: The driver calls `drm_bridge_hpd_notify()` in `it6162_interrupt_handler()` but never sets `DRM_BRIDGE_OP_HPD` in `bridge.ops`. Without this flag, `drm_bridge_hpd_notify()` will not function properly.

4. **Sample width mapping is swapped**:
```c
case 24:
    config->sample_width = WORD_LENGTH_18BIT;  // Should be WORD_LENGTH_24BIT
    break;
case 20:
    config->sample_width = WORD_LENGTH_24BIT;  // Should be WORD_LENGTH_20BIT
    break;
```
The 24-bit and 20-bit cases are assigned wrong enum values.

**Significant issues:**

5. **Regulator disable order is wrong**: In `it6162_platform_clear_power()`, regulators are disabled in the same order they were enabled (`ivdd` → `pwr1833` → `ovdd`). Standard practice is to disable in reverse order of enable (`ovdd` → `pwr1833` → `ivdd`). Also, if `pwr1833` disable fails, `ovdd` will be left enabled (no cleanup on partial failure).

6. **`readx_poll_timeout` condition bug** in `it6162_infoblock_request_data()`:
```c
bool val = 0;
...
status = readx_poll_timeout(it6162_infoblock_complete,
    it6162, val,
    val <= 0 && it6162->data_buf_sts == BUF_READY,
    50 * 1000, TIMEOUT_INFOBLOCK_MS * 1000);
```
`it6162_infoblock_complete()` returns an `int` but `val` is declared as `bool`. This will truncate the return value and break the `val <= 0` condition check. Should be `int val`.

7. **Duplicate register offset definition**:
```c
#define OFFSET_VERSION_L 0x03
#define OFFSET_VERSION_M 0x04
#define OFFSET_VERSION_H 0x03  // BUG: same as OFFSET_VERSION_L
```
`OFFSET_VERSION_H` should presumably be `0x05`, not `0x03`.

8. **`it6162_bridge_atomic_check` modifies `adjusted_mode` but doesn't update `htotal`**: When HFP is adjusted, `hsync_start` and `hsync_end` are moved, but `htotal` is not updated. This means the effective HBP changes implicitly, and the adjustment may produce inconsistent timing.

9. **Missing `mutex_init` before first use**: `mutex_init(&it6162->lock)` is called in `it6162_probe()` after `it6162_detect_devices()`, which calls `it6162_platform_set_power()` and `it6162_wait_mcu_ready()`. But `it6162_set_default_config()` (called later via `it6162_poweron()` → `it6162_reset_init()`) uses `guard(mutex)(&it6162->lock)`. The mutex should be initialized earlier in the probe sequence, before any function that might acquire it. Looking more carefully, `it6162_detect_devices()` doesn't use the mutex, but `it6162_poweron()` (called from `it6162_attach_dsi()`) does, and the mutex IS initialized before that call. However, it's fragile — better to initialize it earlier.

10. **`it6162_poweron` / `it6162_poweroff` are marked `__maybe_unused`**: These are called directly from `it6162_attach_dsi()`. The `__maybe_unused` annotation is incorrect and misleading — these functions are definitely used. If the intent was for PM callbacks, they aren't actually wired up as PM ops.

11. **No power management (runtime PM or system PM)**: The driver powers on during probe/attach but has no suspend/resume support. `pm_runtime` is included but never used.

12. **`it6162_platform_clear_power` never called on normal remove**: `it6162_remove()` only disables IRQ and cancels work, but never powers down the chip. Regulators remain enabled after driver unbind.

13. **Copyright header inconsistency**: The SPDX tag says `GPL-2.0-only` but the comment block says "either version 2 of the License, or (at your option) any later version" which is `GPL-2.0-or-later`. Pick one and remove the redundant boilerplate text.

14. **Typo**: `"evnet change"` in the debug message at the interrupt handler should be `"event change"`.

15. **Excessive `dev_info` logging**: Many `dev_info` calls (chip ID, probe status, DSI host info, lane config, poweron end) should be `dev_dbg` to avoid spamming the kernel log during normal operation.

16. **`it6162_hdcp_handler` accesses `cp_status` potentially uninitialized**: If `it6162->hdcp_version != NO_HDCP` but `it6162->hdcp_sts == hdcp_sts` and `it6162->hdcp_sts != NO_HDCP_STATE`, the code falls through to `drm_hdcp_update_content_protection()` with `cp_status` uninitialized (it's only set inside the inner `if` block).

17. **Infoframe write/clear functions not protected by mutex**: `it6162_write_infoframe()` and `it6162_clear_infoframe()` access registers and call `it6162_infoblock_host_set()` but are not wrapped in `guard(mutex)`, unlike other infoblock operations.

---
Generated by Claude Code Patch Reviewer