[PATCH v5] drm/bridge: imx8qxp-pxl2dpi: avoid ERR_PTR with device_node cleanup

[PATCH v5] drm/bridge: imx8qxp-pxl2dpi: avoid ERR_PTR with device_node cleanup

[Guangshuo Li]

imx8qxp_pxl2dpi_get_available_ep_from_port() returns ERR_PTR()
on errors. imx8qxp_pxl2dpi_find_next_bridge() stores its return
value in a __free(device_node) variable before checking IS_ERR().
When the function returns on the error path, the cleanup action calls
of_node_put() on the ERR_PTR() value.

Do not store the endpoint node in a cleanup variable before checking
whether it is an error pointer. Use a regular device_node pointer for
the endpoint node, check it with IS_ERR() first, and release it
explicitly with of_node_put() after getting the remote port parent.

This keeps the fix minimal and avoids changing
imx8qxp_pxl2dpi_get_available_ep_from_port().

Fixes: ceea3f7806a10 ("drm/bridge: imx8qxp-pxl2dpi: simplify put of device_node pointers")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v5:
  - Make the fix minimal for stable by avoiding __free(device_node)
    for the endpoint node in imx8qxp_pxl2dpi_find_next_bridge().
  - Keep imx8qxp_pxl2dpi_get_available_ep_from_port() unchanged.
  - Do not change imx8qxp_pxl2dpi_set_pixel_link_sel().
  - Drop Frank's Reviewed-by tag due to the implementation change.

v4:
  - Drop the sentence mentioning the custom static analysis tool.
  - Add Frank's Reviewed-by tag.
  - No functional code changes.

v3:
  - Do not change DEFINE_FREE(device_node, ...).
  - Fix the driver pattern by making
    imx8qxp_pxl2dpi_get_available_ep_from_port() return an int and
    pass the endpoint via an output argument.
  - Update both callers so __free(device_node) never holds ERR_PTR().

v2:
  - Fix DEFINE_FREE(device_node, ...) directly.

 drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
index 441fd32dc91c..f64f57a33c62 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
@@ -264,12 +264,13 @@ imx8qxp_pxl2dpi_get_available_ep_from_port(struct imx8qxp_pxl2dpi *p2d,
 
 static int imx8qxp_pxl2dpi_find_next_bridge(struct imx8qxp_pxl2dpi *p2d)
 {
-	struct device_node *ep __free(device_node) =
+	struct device_node *ep =
 		imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1);
 	if (IS_ERR(ep))
 		return PTR_ERR(ep);
 
 	struct device_node *remote __free(device_node) = of_graph_get_remote_port_parent(ep);
+	of_node_put(ep);
 	if (!remote || !of_device_is_available(remote)) {
 		DRM_DEV_ERROR(p2d->dev, "no available remote\n");
 		return -ENODEV;
-- 
2.43.0

Re: [PATCH v5] drm/bridge: imx8qxp-pxl2dpi: avoid ERR_PTR with device_node cleanup

[Liu Ying]

On Wed, May 06, 2026 at 05:23:24PM +0800, Guangshuo Li wrote:
> imx8qxp_pxl2dpi_get_available_ep_from_port() returns ERR_PTR()
> on errors. imx8qxp_pxl2dpi_find_next_bridge() stores its return
> value in a __free(device_node) variable before checking IS_ERR().
> When the function returns on the error path, the cleanup action calls
> of_node_put() on the ERR_PTR() value.
> 
> Do not store the endpoint node in a cleanup variable before checking
> whether it is an error pointer. Use a regular device_node pointer for
> the endpoint node, check it with IS_ERR() first, and release it
> explicitly with of_node_put() after getting the remote port parent.
> 
> This keeps the fix minimal and avoids changing
> imx8qxp_pxl2dpi_get_available_ep_from_port().
> 
> Fixes: ceea3f7806a10 ("drm/bridge: imx8qxp-pxl2dpi: simplify put of device_node pointers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v5:
>   - Make the fix minimal for stable by avoiding __free(device_node)
>     for the endpoint node in imx8qxp_pxl2dpi_find_next_bridge().

By "minimal" in v4 comment, I meant not to use __free(device_node)
in imx8qxp_pxl2dpi_get_available_ep_from_port() and
imx8qxp_pxl2dpi_set_pixel_link_sel() - please keep using
__free(device_node) in imx8qxp_pxl2dpi_find_next_bridge().


>   - Keep imx8qxp_pxl2dpi_get_available_ep_from_port() unchanged.

No, please fix imx8qxp_pxl2dpi_get_available_ep_from_port() to make it
return int.

>   - Do not change imx8qxp_pxl2dpi_set_pixel_link_sel().

No, you need to change it.

-- 
Regards,
Liu Ying

Re: [PATCH v5] drm/bridge: imx8qxp-pxl2dpi: avoid ERR_PTR with device_node cleanup

[Guangshuo Li]

Hi Liu,

Thanks for the clarification.

On Wed, 6 May 2026 at 17:49, Liu Ying <victor.liu@nxp.com> wrote:
>

>
> By "minimal" in v4 comment, I meant not to use __free(device_node)
> in imx8qxp_pxl2dpi_get_available_ep_from_port() and
> imx8qxp_pxl2dpi_set_pixel_link_sel() - please keep using
> __free(device_node) in imx8qxp_pxl2dpi_find_next_bridge().
>
>
> >   - Keep imx8qxp_pxl2dpi_get_available_ep_from_port() unchanged.
>
> No, please fix imx8qxp_pxl2dpi_get_available_ep_from_port() to make it
> return int.
>
> >   - Do not change imx8qxp_pxl2dpi_set_pixel_link_sel().
>
> No, you need to change it.
>
> --
> Regards,
> Liu Ying

I misunderstood your previous comment. I will update the patch to keep
using __free(device_node) in imx8qxp_pxl2dpi_find_next_bridge(),
change imx8qxp_pxl2dpi_get_available_ep_from_port() to return int and
pass the endpoint through an output argument, but avoid adding cleanup
action usage in imx8qxp_pxl2dpi_get_available_ep_from_port() and
imx8qxp_pxl2dpi_set_pixel_link_sel().

I will also drop the unnecessary local NULL initialization for ep,
since the helper initializes the output argument to NULL.

I will send a v6.

Best regards,
Guangshuo

Re: [PATCH v5] drm/bridge: imx8qxp-pxl2dpi: avoid ERR_PTR with device_node cleanup

[Guangshuo Li]

Hi Liu,

On Wed, 6 May 2026 at 21:58, Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> I will also drop the unnecessary local NULL initialization for ep,
> since the helper initializes the output argument to NULL.
>
> I will send a v6.
>
> Best regards,
> Guangshuo

One correction to my previous reply: I will keep the local ep variable
in imx8qxp_pxl2dpi_find_next_bridge() initialized to NULL, since
checkpatch requires pointers with the __free attribute to be
initialized.

Best regards,
Guangshuo