From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/bridge: imx8qxp-pxl2dpi: avoid ERR_PTR with device_node cleanup
Date: Thu, 07 May 2026 13:48:12 +1000
Message-ID: <review-patch1-20260506092324.635014-1-lgs201920130244@gmail.com>
In-Reply-To: <20260506092324.635014-1-lgs201920130244@gmail.com>
References: <20260506092324.635014-1-lgs201920130244@gmail.com> <20260506092324.635014-1-lgs201920130244@gmail.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Bug analysis:** Correct. The `DEFINE_FREE` for `device_node` at `include/=
linux/of.h:138`:
```c
DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T))
```
only guards against NULL, not ERR_PTR. The function `imx8qxp_pxl2dpi_get_av=
ailable_ep_from_port()` returns ERR_PTR on multiple error paths. So the ori=
ginal code:
```c
struct device_node *ep __free(device_node) =3D
    imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1);
if (IS_ERR(ep))
    return PTR_ERR(ep);
```
would trigger `of_node_put()` on an ERR_PTR value when the function returns=
 early on the error path, since `__free` runs at scope exit.

**Fix correctness:** The patch correctly:
1. Removes `__free(device_node)` from `ep`, making it a plain pointer.
2. Keeps the existing `IS_ERR(ep)` check =E2=80=94 on this path, no cleanup=
 is needed (ERR_PTR is not a real node).
3. Adds an explicit `of_node_put(ep)` after `of_graph_get_remote_port_paren=
t(ep)` consumes the node.
4. Leaves `remote` with `__free(device_node)` =E2=80=94 this is safe becaus=
e `of_graph_get_remote_port_parent()` returns NULL on failure, never ERR_PT=
R.

```c
struct device_node *remote __free(device_node) =3D of_graph_get_remote_port=
_parent(ep);
of_node_put(ep);
```

The placement of `of_node_put(ep)` is correct: it's called after `ep` is co=
nsumed by `of_graph_get_remote_port_parent()` and before the function could=
 return without releasing it.

**Minor observations:**
- The commit message is well-written and clearly explains the bug, the fix =
strategy, and why the approach was chosen (minimal for stable backport).
- The other caller, `imx8qxp_pxl2dpi_set_pixel_link_sel()`, already uses ma=
nual `of_node_put()` and is not affected by this bug.
- The Fixes tag correctly points to `ceea3f7806a10` which introduced the `_=
_free(device_node)` annotation.
- The v5 changelog shows good iteration, with the author converging on the =
most minimal fix after exploring more invasive approaches in earlier versio=
ns.

**No issues found.** The patch is correct, minimal, and appropriate for sta=
ble.

---
Generated by Claude Code Patch Reviewer