Claude review: drm/msm: Attach a driver to the GMU

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

Patch Review

**Verdict: The approach works but has a fragile drvdata handoff that should be reconsidered.**

This is the main patch. It registers a platform driver for the GMU, making it a component device on the same master as the GPU.

**Issue 1 -- Cross-component drvdata coupling creates a dangling pointer risk:**

In `a6xx_gpu_init`, the GPU component's bind sets the GMU pdev's drvdata:

```c
	gmu_pdev = of_find_device_by_node(node);
	of_node_put(node);
	if (gmu_pdev) {
		platform_set_drvdata(gmu_pdev, a6xx_gpu);
		put_device(&gmu_pdev->dev);
	}
```

Then in `a6xx_gmu_bind`, the GMU component reads it back:

```c
	struct a6xx_gpu *a6xx_gpu = dev_get_drvdata(dev);
```

If the GMU component's bind fails, `component_bind_all` will unwind by calling the GPU component's unbind, which calls `a6xx_destroy` -> `kfree(a6xx_gpu)`. But the GMU pdev's drvdata still points to the freed `a6xx_gpu` -- it was never cleared because `a6xx_gmu_unbind` was never called (the GMU bind failed, not succeeded). This leaves a dangling pointer.

In practice it may be benign since nothing accesses the GMU pdev's drvdata after the master bind fails, but it's still a latent use-after-free. Consider clearing the GMU's drvdata in the GPU's unbind/destroy path, or using a different mechanism (e.g., passing the `a6xx_gpu` pointer through the component `data` parameter from the master).

**Issue 2 -- `BUG_ON` to `WARN_ON` without early return:**

```c
	node = of_parse_phandle(pdev->dev.of_node, "qcom,gmu", 0);
-	/* FIXME: How do we gracefully handle this? */
-	BUG_ON(!node);
+	WARN_ON(!node);
```

While removing `BUG_ON` is good, if `node` is NULL the function continues and eventually passes `NULL` to `of_find_device_by_node`, `of_node_put`, etc. These happen to be NULL-safe, and `of_device_is_compatible(NULL, ...)` returns 0, so it won't crash. But the GPU will be initialized without a GMU, which will cause problems later at runtime. This should be an early `return ERR_PTR(-ENODEV)` instead of a bare `WARN_ON`.

**Issue 3 -- GMU init/destroy no longer called from a6xx_gpu_init/a6xx_destroy:**

The original code had:

```c
-	if (adreno_has_gmu_wrapper(adreno_gpu) || adreno_has_rgmu(adreno_gpu))
-		ret = a6xx_gmu_wrapper_init(a6xx_gpu, node);
-	else
-		ret = a6xx_gmu_init(a6xx_gpu, node);
```

And `a6xx_destroy` had:

```c
-	a6xx_gmu_remove(a6xx_gpu);
```

These are now handled by the GMU component's bind/unbind. This means the GMU init/destroy ordering is now determined by the component framework rather than being explicitly controlled. The ordering relies on the GPU being listed before the GMU in the match list, and unbinding happening in reverse order. This is correct behavior for the component framework, but it's implicit rather than explicit -- worth a comment.

**Issue 4 -- `adreno_gmu_register()` / `adreno_gmu_unregister()` lack `__init`/`__exit` annotations in the header:**

```c
+void adreno_gmu_register(void);
+void adreno_gmu_unregister(void);
```

The implementations have `__init`/`__exit`:

```c
+void __init adreno_gmu_register(void)
+void __exit adreno_gmu_unregister(void)
```

The declarations in `adreno_gpu.h` should also carry the `__init`/`__exit` annotations to avoid section mismatch warnings when building with certain configs.

**Issue 5 -- Error return from `platform_driver_register` is ignored:**

```c
+void __init adreno_gmu_register(void)
+{
+	platform_driver_register(&adreno_gmu_driver);
+}
```

The return value is silently discarded. This matches the pattern used by `adreno_register` itself (which also ignores `platform_driver_register`'s return), so it's consistent, but it's worth noting.

**Minor -- `add_gpu_components` now always adds the GMU to the component match:**

In the non-standalone path, `add_gpu_components` is called from `msm_drv_probe` when `!msm_gpu_use_separate_drm_dev()`. This means the main msm_drm component master now waits for the GMU component in addition to GPU + display components. This is the intended behavior but represents a behavioral change for all a6xx+ SoCs with KMS, not just standalone GPU configurations. If the GMU driver fails to probe for any reason, the entire DRM master will fail to bind. Make sure this is tested with KMS-enabled SoCs (the common case), not just standalone GPU configurations.

---
Generated by Claude Code Patch Reviewer