Claude review: drm:bridge: Add Lontiun LT7911EXC eDP to MIPI DSI bridge

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

Patch Review

#### Major Issues

**1. Missing atomic state management helpers.** The bridge uses `atomic_pre_enable`, `atomic_disable`, and `atomic_post_disable` callbacks, but does not provide the required state helpers:

```c
static const struct drm_bridge_funcs lt7911exc_bridge_funcs = {
	.attach = lt7911exc_attach,
	.atomic_pre_enable = lt7911exc_atomic_pre_enable,
	.atomic_disable = lt7911exc_atomic_disable,
	.atomic_post_disable = lt7911exc_atomic_post_disable,
};
```

Every other bridge driver using atomic callbacks provides these (see `lontium-lt9611.c`):
```c
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.atomic_reset = drm_atomic_helper_bridge_reset,
```

Without them, the atomic state machine cannot function correctly.

**2. No MIPI DSI device registration.** The bridge outputs MIPI DSI but never registers as a DSI device. Other Lontium bridge drivers with DSI output (e.g., lt9611, lt8912b) call `of_find_mipi_dsi_host_by_node()`, `devm_mipi_dsi_device_register_full()`, and `devm_mipi_dsi_attach()`. Without this, the DSI link to the downstream panel cannot be established by the framework.

**3. No `atomic_enable` or mode configuration.** The bridge has `atomic_pre_enable` (power-on + reset) but no `atomic_enable`. There is no code to configure the bridge for a particular display mode — no lane count, format, timing, or link training. The bridge currently just powers up and hopes the hardware auto-configures, which seems unlikely for an eDP-to-DSI converter.

**4. Regulators left enabled after probe.** In `lt7911exc_probe`:

```c
ret = regulator_bulk_enable(ARRAY_SIZE(lt7911exc->supplies), lt7911exc->supplies);
// ... version check, possible fw upgrade ...
lt7911exc_unlock(lt7911exc);
lt7911exc->bridge.of_node = dev->of_node;
devm_drm_bridge_add(dev, &lt7911exc->bridge);
return 0;
```

The success path never disables the regulators. They stay enabled until the first `atomic_post_disable`. The regulators should be disabled after the version/firmware check completes successfully, since `atomic_pre_enable` will re-enable them when the display is actually used.

#### Medium Issues

**5. `lt7911exc_attach` uses `bridge.encoder` instead of the `encoder` parameter:**

```c
static int lt7911exc_attach(struct drm_bridge *bridge,
			    struct drm_encoder *encoder,
			    enum drm_bridge_attach_flags flags)
{
	struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
	return drm_bridge_attach(lt7911exc->bridge.encoder, lt7911exc->bridge.next_bridge,
				 &lt7911exc->bridge, flags);
}
```

Other Lontium drivers use the `encoder` parameter directly:
```c
return drm_bridge_attach(encoder, lt9611->next_bridge, bridge, flags);
```

Use `encoder` from the parameter and `bridge` directly rather than going through the container.

**6. `lt7911exc_block_erase` ignores `regmap_multi_reg_write` return value:**

```c
regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write));
msleep(200);
```

A failed flash erase will silently proceed to write firmware data, corrupting the device. The return value must be checked, and the function should return `int`, not `void`.

**7. `lt7911exc_prog_init` also ignores its regmap return value.** Same issue — a failed write during flash programming should abort the operation.

**8. `kzalloc` immediately overwritten with `memset(buffer, 0xff, ...)`:**

```c
buffer = kzalloc(total_size, GFP_KERNEL);
...
memset(buffer, 0xff, total_size);
```

Use `kmalloc` instead of `kzalloc` since the zeroed memory is immediately overwritten with 0xff.

**9. `atomic_disable` only contains a sleep:**

```c
static void lt7911exc_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
{
	msleep(20);
}
```

If the delay is required for sequencing between panel disable and regulator shutdown, it would be better placed in `atomic_post_disable` before the regulator disable, with a comment explaining why. A callback that only sleeps with no bridge-specific action is suspect.

#### Minor Issues

**10. Missing headers.** The driver uses `msleep`/`usleep_range` but does not include `<linux/delay.h>`, and uses regulator APIs but does not include `<linux/regulator/consumer.h>`. These likely compile via transitive includes, but explicit includes are preferred practice.

**11. `cal_crc32_custom` does not validate length alignment.** The function assumes `length` is a multiple of 4 and reads `data[i+3]` without bounds checking. While callers currently pass aligned lengths, the function should either assert/document this requirement or handle the tail bytes:

```c
static u32 cal_crc32_custom(const u8 *data, u64 length)
```

**12. `lt7911exc_read_version` redundantly writes OCM register.** It writes `0xe0ee = 0x01`, but it's always called with the OCM lock already held (which already wrote the same value in `lt7911exc_lock`).

**13. Kconfig selects `DRM_PANEL` unnecessarily.** The driver does not use any panel API (`drm_panel_*`). Remove this select. Also verify whether `DRM_KMS_HELPER` is actually needed — the driver doesn't appear to use any KMS helper functions directly.

**14. `#include <linux/platform_device.h>` is unnecessary.** This is an I2C driver, not a platform driver.

**15. `fw_version` stored but never exposed.** The firmware version is read into `lt7911exc->fw_version` but never made available via sysfs, debugfs, or dev_info. Consider at least logging it at probe time so users/developers can verify the firmware level.

---
Generated by Claude Code Patch Reviewer