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. Change
imx8qxp_pxl2dpi_get_available_ep_from_port() to return an int and pass
the endpoint node through an output argument. Initialize the output
argument to NULL so callers hold either NULL on error paths or a valid
device_node pointer on successful path.
Fixes: ceea3f7806a10 ("drm/bridge: imx8qxp-pxl2dpi: simplify put of device_node
pointers")
Cc: [email protected]
Reviewed-by: Liu Ying <[email protected]>
Signed-off-by: Guangshuo Li <[email protected]>
---
v7:
- Rephrase the commit message sentence about output argument
initialization as suggested by Liu Ying.
- Drop the unnecessary sentence about keeping explicit of_node_put()
usage.
- Add Liu Ying's Reviewed-by tag.
- No code changes.
v6:
- Change imx8qxp_pxl2dpi_get_available_ep_from_port() to return int
and pass the endpoint through an output argument.
- Keep using __free(device_node) in imx8qxp_pxl2dpi_find_next_bridge().
- Keep ep initialized to NULL in imx8qxp_pxl2dpi_find_next_bridge()
to satisfy the __free pointer initialization requirement.
- Do not add cleanup action usage in
imx8qxp_pxl2dpi_get_available_ep_from_port() or
imx8qxp_pxl2dpi_set_pixel_link_sel().
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 | 40 +++++++++++---------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
index 441fd32dc91c..d64e328bf542 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
@@ -222,52 +222,58 @@ 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;
+ int ret = 0;
int ep_cnt;
+ *ep = NULL;
+
port = of_graph_get_port_by_id(p2d->dev->of_node, port_id);
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);
+ ret = -ENODEV;
goto out;
} else if (ep_cnt > 1) {
DRM_DEV_ERROR(p2d->dev,
"invalid available endpoints of port@%u\n",
port_id);
- ep = ERR_PTR(-EINVAL);
+ ret = -EINVAL;
goto out;
}
- 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);
+ ret = -ENODEV;
goto out;
}
out:
of_node_put(port);
- return ep;
+ return ret;
}
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)) {
@@ -291,9 +297,9 @@ static int imx8qxp_pxl2dpi_set_pixel_link_sel(struct
imx8qxp_pxl2dpi *p2d)
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) {
--
2.43.0
