Messages in this thread |  | | Date | Mon, 4 May 2026 15:09:13 -0400 | | From | Frank Li <> | | Subject | Re: [PATCH v3] drm/bridge: imx8qxp-pxl2dpi: avoid ERR_PTR with device_node cleanup |
| |
On Sat, May 02, 2026 at 07:55:28PM +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 let a device_node cleanup variable hold error pointers. Return > the error code from imx8qxp_pxl2dpi_get_available_ep_from_port() > directly and pass the endpoint node through an output argument. This > keeps the cleanup action operating only on NULL or a valid device_node, > while preserving the existing error codes. > > This issue was found by a custom static analysis tool.
Nit: needn't mention this.
Reviewed-by: Frank Li <Frank.Li@nxp.com> > > 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> > --- > 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 | 55 +++++++++----------- > 1 file changed, 26 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c > index 441fd32dc91c..a82f10218707 100644 > --- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c > +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c > @@ -222,52 +222,52 @@ static const struct drm_bridge_funcs imx8qxp_pxl2dpi_bridge_funcs = { > imx8qxp_pxl2dpi_bridge_atomic_get_output_bus_fmts, > }; > > -static struct device_node * > +static int > imx8qxp_pxl2dpi_get_available_ep_from_port(struct imx8qxp_pxl2dpi *p2d, > - u32 port_id) > + u32 port_id, > + struct device_node **ep) > { > - struct device_node *port, *ep; > + struct device_node *port __free(device_node) = > + of_graph_get_port_by_id(p2d->dev->of_node, port_id); > int ep_cnt; > > - port = of_graph_get_port_by_id(p2d->dev->of_node, port_id); > + *ep = NULL; > if (!port) { > DRM_DEV_ERROR(p2d->dev, "failed to get port@%u\n", port_id); > - return ERR_PTR(-ENODEV); > + return -ENODEV; > } > > ep_cnt = of_get_available_child_count(port); > if (ep_cnt == 0) { > DRM_DEV_ERROR(p2d->dev, "no available endpoints of port@%u\n", > port_id); > - ep = ERR_PTR(-ENODEV); > - goto out; > + return -ENODEV; > } else if (ep_cnt > 1) { > DRM_DEV_ERROR(p2d->dev, > "invalid available endpoints of port@%u\n", > port_id); > - ep = ERR_PTR(-EINVAL); > - goto out; > + return -EINVAL; > } > > - ep = of_get_next_available_child(port, NULL); > - if (!ep) { > + *ep = of_get_next_available_child(port, NULL); > + if (!*ep) { > DRM_DEV_ERROR(p2d->dev, > "failed to get available endpoint of port@%u\n", > port_id); > - ep = ERR_PTR(-ENODEV); > - goto out; > + return -ENODEV; > } > -out: > - of_node_put(port); > - return ep; > + > + return 0; > } > > static int imx8qxp_pxl2dpi_find_next_bridge(struct imx8qxp_pxl2dpi *p2d) > { > - struct device_node *ep __free(device_node) = > - imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1); > - if (IS_ERR(ep)) > - return PTR_ERR(ep); > + struct device_node *ep __free(device_node) = NULL; > + int ret; > + > + ret = imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1, &ep); > + if (ret) > + return ret; > > struct device_node *remote __free(device_node) = of_graph_get_remote_port_parent(ep); > if (!remote || !of_device_is_available(remote)) { > @@ -287,26 +287,23 @@ static int imx8qxp_pxl2dpi_find_next_bridge(struct imx8qxp_pxl2dpi *p2d) > > static int imx8qxp_pxl2dpi_set_pixel_link_sel(struct imx8qxp_pxl2dpi *p2d) > { > - struct device_node *ep; > + struct device_node *ep __free(device_node) = NULL; > struct of_endpoint endpoint; > int ret; > > - ep = imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 0); > - if (IS_ERR(ep)) > - return PTR_ERR(ep); > + ret = imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 0, &ep); > + if (ret) > + return ret; > > ret = of_graph_parse_endpoint(ep, &endpoint); > if (ret) { > DRM_DEV_ERROR(p2d->dev, > "failed to parse endpoint of port@0: %d\n", ret); > - goto out; > + return ret; > } > > p2d->pl_sel = endpoint.id; > -out: > - of_node_put(ep); > - > - return ret; > + return 0; > } > > static int imx8qxp_pxl2dpi_parse_dt_companion(struct imx8qxp_pxl2dpi *p2d) > -- > 2.43.0 >
|  |