Claude review: drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver

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

Patch Review

This is the largest patch (~2244 lines across 6 files) and deserves the most scrutiny.

**Deprecated include:**
```c
#include <linux/of_device.h>
```
`of_device.h` is deprecated in recent kernels. Use `linux/of.h` and `linux/platform_device.h` instead.

**`cdns_mhdp8501_dt_parse` returns `true` on success:**
```c
return true;
```
But the caller checks `if (ret < 0)`. Since `true == 1`, which is `>= 0`, this works accidentally, but returning a boolean from a function that uses `int` return type and `-EINVAL` error codes is confusing. Should return `0` on success.

**Lane mapping validation is ineffective:**
```c
lane_value = (data_lanes[i] >= 0 && data_lanes[i] <= 3) ? data_lanes[i] : 0;
```
Since `data_lanes[i]` is `u32`, the `>= 0` check is always true. Also, silently clamping invalid values to 0 instead of returning an error is wrong — an invalid lane mapping in DT should be an error.

**`endpoint` device_node leak:**
```c
endpoint = of_graph_get_endpoint_by_regs(np, 1, -1);
// ... used but never of_node_put(endpoint)
```
Missing `of_node_put(endpoint)` — this leaks a refcount.

**`kmalloc_obj(*input_fmts)` usage:**
```c
input_fmts = kmalloc_obj(*input_fmts);
```
This uses the newer `kmalloc_obj` API which allocates based on the type of the argument. This should work but is worth verifying it compiles.

**`struct video_info` in `cdns_mhdp8501_device`:**
```c
struct video_info video_info;
```
This stores `bpc` and `color_fmt` in the device struct, but in the DP path these are only used between `atomic_check` and `atomic_enable`. This is fragile since atomic state should be the source of truth — storing it in the device struct means it could become stale. The HDMI path correctly uses `conn_state->hdmi` directly.

**DP `cdns_dp_check_link_state` accesses connector state unsafely:**
```c
conn_state = connector->state;
crtc = conn_state->crtc;
...
crtc_state = crtc->state;
```
Accessing `connector->state` and `crtc->state` outside of an atomic commit or without holding appropriate locks is racy. The same issue exists in `cdns_hdmi_handle_hotplug()`.

**HDMI `reset_pipe()` function:**
```c
static int reset_pipe(struct drm_crtc *crtc)
{
    state = drm_atomic_state_alloc(crtc->dev);
    ...
    crtc_state->connectors_changed = true;
    ret = drm_atomic_commit(state);
```
Performing a full modeset from a workqueue (hotplug handler) without `drm_modeset_lock_all` is problematic. The `drm_modeset_acquire_init(&ctx, 0)` is used but the retry loop for `-EDEADLK` is missing, which can cause deadlocks. Compare with `intel_display_reset_finish()` or `drm_client_modeset_commit_atomic()` for proper patterns.

**HDMI I2C adapter is limited but claims broad functionality:**
```c
static u32 cdns_hdmi_i2c_func(struct i2c_adapter *adapter)
{
    return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
}
```
But the `xfer` function only supports `SCDC_I2C_SLAVE_ADDRESS` (0x54). Advertising `I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL` when only a single slave address is supported is misleading.

**HDMI `cdns_hdmi_i2c_read` reads uninitialized vars:**
```c
static int cdns_hdmi_i2c_read(struct cdns_mhdp8501_device *mhdp,
                              struct i2c_msg *msgs, int num)
{
    u8 addr, offset, *buf, len;
    ...
    for (i = 0; i < num; i++) {
        if (msgs[i].flags & I2C_M_RD) {
            addr = msgs[i].addr;
            buf = msgs[i].buf;
            len = msgs[i].len;
        } else {
            offset = msgs[i].buf[0];
        }
    }
    msg[0] = addr;
```
If no message has `I2C_M_RD` set, `addr`, `buf`, and `len` are used uninitialized. There's no validation of message ordering.

**Missing `DRM_BRIDGE_OP_HDMI_AUDIO` for audio support**: The Kconfig selects `DRM_CDNS_AUDIO` but `bridge.ops` doesn't include `DRM_BRIDGE_OP_HDMI_AUDIO`, and no audio infoframe callbacks are implemented. Either drop the `DRM_CDNS_AUDIO` select or add audio support.

---
Generated by Claude Code Patch Reviewer