Claude review: drm: dp_tunnel: use kzalloc_flex

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

Patch Review

**The good:**

- The conversion from a separate `groups` pointer to a flexible array member `groups[]` with `__counted_by(group_count)` is clean and appropriate.
- Removing the second `kzalloc` for `mgr->groups` and the corresponding `kfree(mgr->groups)` in `destroy_mgr` is correct — with a flex array, a single `kfree(mgr)` suffices.
- Using `kzalloc_flex(*mgr, groups, max_group_count)` matches the macro's expected signature.

**Bug — error handling dropped from `init_group` loop:**

The patch changes:
```c
-	for (i = 0; i < max_group_count; i++) {
-		if (!init_group(mgr, &mgr->groups[i])) {
-			destroy_mgr(mgr);
-
-			return ERR_PTR(-ENOMEM);
-		}
-
-		mgr->group_count++;
-	}
+	for (i = 0; i < max_group_count; i++)
+		init_group(mgr, &mgr->groups[i]);
```

The commit message claims "It always returns true. That is, max_group_count is always equal to group_count." Looking at `init_group` (line 1597), this is indeed **currently true** — the function unconditionally returns `true`. So the behavioral change is safe today.

However, the patch also **silently changes the return type expectation**: by removing the error check entirely and ignoring the return value, any future change to `init_group` that introduces a failure path would silently be ignored. The cleaner approach would be to also change `init_group`'s return type from `bool` to `void` in the same patch to make the contract explicit. Otherwise a future developer seeing `init_group` returns `bool` might add a failure path without realizing callers now ignore it.

**Nit — `group_count` assignment before `init_group` loop:**

```c
+	mgr->group_count = max_group_count;
```

This is set before the `init_group` loop. In the old code, `mgr->group_count` was incremented one-by-one as each group was initialized, so `destroy_mgr` would only clean up successfully initialized groups. With the new code, if `init_group` were ever changed to fail, `destroy_mgr` would attempt to clean up uninitialized groups.

This is fine given `init_group` can't fail, but it's another reason to **change `init_group` to return `void`** in the same patch to make the invariant clear.

**Struct field reordering:**

```c
-	int group_count;
-	struct drm_dp_tunnel_group *groups;
 	wait_queue_head_t bw_req_queue;
 ...
+	int group_count;
+	struct drm_dp_tunnel_group groups[] __counted_by(group_count);
```

Moving `group_count` and `groups` to the end of the struct is required since flex arrays must be the last member. The `group_count` field is also moved to be adjacent to `groups[]` which is good for `__counted_by`. The reordering may introduce a small hole for padding between `wait_queue_head_t bw_req_queue` and the `#ifdef` block, but this is unlikely to matter in practice.

**Recommendation:** The patch should also change `init_group` from `static bool` to `static void` to match the new calling convention. Without that, the patch is technically correct but leaves a misleading function signature.

---
Generated by Claude Code Patch Reviewer