Claude review: drm/logicvc: Avoid use-after-free with devm_kzalloc()

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

Patch Review

**logicvc_crtc.c** — Clean and straightforward.

The conversion from `devm_kzalloc` + `drm_crtc_init_with_planes` to `drmm_crtc_alloc_with_planes` is correct. The macro arguments match the expected signature:

```c
crtc = drmm_crtc_alloc_with_planes(drm_dev, struct logicvc_crtc,
                                    drm_crtc, &layer_primary->drm_plane,
                                    NULL, &logicvc_crtc_funcs, NULL);
```

Removal of `.destroy = drm_crtc_cleanup` is correct — `drmm_crtc_alloc_with_planes` registers cleanup automatically.

No issues.

---

**logicvc_interface.c** — Also clean.

Smart reordering: the patch moves `drm_of_find_panel_or_bridge()` before the allocation by using local `panel`/`bridge` temporaries, then copies them into the `interface` struct after `drmm_encoder_alloc`. This avoids needing the struct before it exists.

```c
struct drm_bridge *bridge;
struct drm_panel *panel;
...
ret = drm_of_find_panel_or_bridge(of_node, 0, 0, &panel, &bridge);
...
interface = drmm_encoder_alloc(drm_dev, struct logicvc_interface, drm_encoder,
                               NULL, encoder_type, NULL);
...
interface->drm_panel = panel;
interface->drm_bridge = bridge;
```

The `drmm_connector_init` call correctly adds the `ddc` parameter (`NULL`):

```c
ret = drmm_connector_init(drm_dev, &interface->drm_connector,
                           &logicvc_connector_funcs,
                           connector_type, NULL);
```

Removal of `logicvc_encoder_funcs` (which only had `.destroy = drm_encoder_cleanup`) and `.destroy = drm_connector_cleanup` from `logicvc_connector_funcs` are both correct.

Error path simplification from goto-labels to direct returns is safe since all DRM-managed resources will be cleaned up automatically on DRM device release during probe failure.

No issues.

---

**logicvc_layer.c** — More involved refactoring, but well done.

The key challenge here is that config must be parsed *before* allocation (format lookup depends on config, and format is needed for `drmm_universal_plane_alloc`). The patch handles this by parsing into a temporary stack struct:

```c
struct logicvc_layer_config config = { 0 };
...
ret = logicvc_layer_config_parse(logicvc, of_node, &config);
...
formats = logicvc_layer_formats_lookup(&config);
...
layer = drmm_universal_plane_alloc(drm_dev, ...);
...
logicvc_layer_set_config(layer, &config);
```

The refactoring of `logicvc_layer_config_parse` and `logicvc_layer_formats_lookup` to take direct parameters instead of a `layer` pointer is clean.

The `drmm_add_action_or_reset` for `list_del` is correct — the `_or_reset` variant calls the action if registration fails, so the list entry is removed in all cases. LIFO ordering guarantees `list_del` runs before the plane cleanup (which frees the layer memory).

The background layer early-return path is correct: since `layer` hasn't been allocated yet at that point (config is in a local), no cleanup is needed.

**Minor nit 1:** `logicvc_layer_set_config` does a field-by-field copy that could be a simple struct assignment:

```c
// Current:
static void logicvc_layer_set_config(struct logicvc_layer *layer,
                                     struct logicvc_layer_config *config)
{
    layer->config.colorspace = config->colorspace;
    layer->config.depth = config->depth;
    ...
}

// Simpler alternative:
layer->config = *config;
```

This would be shorter and less error-prone (no risk of forgetting a field if `logicvc_layer_config` gains new members).

**Minor nit 2:** Extra blank line before `return 0;` at the end of `logicvc_layer_init`:

```c
	ret = drmm_add_action_or_reset(drm_dev, logicvc_layer_fini,
				       layer);
	if (ret)
		return ret;


	return 0;
```

One blank line would be sufficient.

**Pre-existing issue (not introduced by this patch):** When `logicvc_layer_init` fails inside the `for_each_child_of_node` loop, neither the old nor new code calls `of_node_put(layer_node)`, leaking the OF node reference. This isn't a regression from this patch, but worth noting for a future fix.

---
Generated by Claude Code Patch Reviewer