Claude review: drm/bridge: imx: Add i.MX93 parallel display format configuration support

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

Patch Review

This is the main driver patch. Several observations:

**1. Potential uninitialized variable `val` (minor):**

```c
unsigned int val;

bridge_state = drm_atomic_get_new_bridge_state(state, bridge);

switch (bridge_state->output_bus_cfg.format) {
case MEDIA_BUS_FMT_RGB888_1X24:
case MEDIA_BUS_FMT_FIXED:
	val = FORMAT_RGB888_TO_RGB888;
	...
	break;
case MEDIA_BUS_FMT_RGB666_1X18:
	val = FORMAT_RGB888_TO_RGB666;
	break;
case MEDIA_BUS_FMT_RGB565_1X16:
	val = FORMAT_RGB565_TO_RGB565;
	break;
}

regmap_update_bits(pdfc->regmap, IMX93_DISPLAY_MUX_REG, mask, val);
```

The switch has no `default` case. While `atomic_check` should prevent an unsupported format from reaching this point, the compiler will likely warn about `val` being potentially used uninitialized. Adding a `default: return;` (or initializing `val = FORMAT_RGB888_TO_RGB888;`) would silence the warning and be more defensive.

**2. Missing `nxp,imx91-pdfc` in of_device_id:**

```c
static const struct of_device_id imx93_pdfc_dt_ids[] = {
	{ .compatible = "nxp,imx93-pdfc", },
	{ /* sentinel */ }
};
```

As noted above, the binding defines `nxp,imx91-pdfc` but the driver doesn't match it. Since the cover letter explicitly states "I added the support for both," adding `{ .compatible = "nxp,imx91-pdfc", }` here would be consistent.

**3. `kmalloc_obj` usage looks correct:**

```c
input_fmts = kmalloc_obj(*input_fmts);
```

`*input_fmts` dereferences to `u32`, so this allocates `sizeof(u32)` with default GFP flags. Correct for a single-element array.

**4. `bus-width` of 16 is accepted but not fully handled:**

The dt-binding allows `bus-width` of 16, 18, or 24. The driver only handles `phy_bus_width == 18` specially (in `atomic_enable` to select RGB888-to-RGB666 truncation). If `bus-width` is set to 16 in DT, `pdfc->phy_bus_width` will be 16 but the driver doesn't act on it at all — it won't affect the format selection when the output format is `MEDIA_BUS_FMT_RGB888_1X24`. This may be intentional (the downstream bridge's format negotiation handles it via `MEDIA_BUS_FMT_RGB565_1X16`), but it's worth confirming.

**5. Kconfig dependency:**

The driver uses `syscon_node_to_regmap()` which comes from `MFD_SYSCON`. There's no explicit `select MFD_SYSCON` or `depends on MFD_SYSCON` in the Kconfig entry. This typically works because the parent blk-ctrl driver already selects it, but an explicit dependency would be safer for build-testing scenarios (where `COMPILE_TEST` could enable this driver without the blk-ctrl).

**6. Bridge funcs struct naming:**

```c
static const struct drm_bridge_funcs funcs = {
```

A generic name like `funcs` works but `imx93_pdfc_bridge_funcs` would be more consistent with other drivers in the same directory and easier to find in debugging/stack traces.

**7. Overall driver structure is clean.** The use of `devm_drm_bridge_alloc`, `dev_err_probe`, `syscon_node_to_regmap` from the parent, and the bridge format negotiation callbacks all follow modern kernel patterns correctly.

---
Generated by Claude Code Patch Reviewer