Hi Laurent,
On 25-05-15, Laurent Pinchart wrote:
> Hi Marco,
>
> On Thu, May 15, 2025 at 12:24:53AM +0200, Marco Felsch wrote:
> > Make use of __free(device_node) to simplify the of_node_put() error
> > handling paths. No functional changes.
> >
> > Signed-off-by: Marco Felsch <m.fel...@pengutronix.de>
> > ---
> > drivers/gpu/drm/bridge/fsl-ldb.c | 24 +++++++++---------------
> > 1 file changed, 9 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c
> > b/drivers/gpu/drm/bridge/fsl-ldb.c
> > index e0a229c91953..cea9ddaa5e01 100644
> > --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> > +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> > @@ -287,8 +287,9 @@ static const struct drm_bridge_funcs funcs = {
> > static int fsl_ldb_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > - struct device_node *panel_node;
> > - struct device_node *remote1, *remote2;
> > + struct device_node *panel_node __free(device_node) = NULL;
> > + struct device_node *remote1 __free(device_node) = NULL;
> > + struct device_node *remote2 __free(device_node) = NULL;
> > struct drm_panel *panel;
> > struct fsl_ldb *fsl_ldb;
> > int dual_link;
> > @@ -321,21 +322,16 @@ static int fsl_ldb_probe(struct platform_device *pdev)
> > remote2 = of_graph_get_remote_node(dev->of_node, 2, 0);
> > fsl_ldb->ch0_enabled = (remote1 != NULL);
> > fsl_ldb->ch1_enabled = (remote2 != NULL);
> > - panel_node = of_node_get(remote1 ? remote1 : remote2);
> > - of_node_put(remote1);
> > - of_node_put(remote2);
> > + panel_node = remote1 ? remote1 : remote2;
>
> This will cause a double put of panel_node, once due to __free() on
> remote1 or remote2, and the second time due to __free() on panel_node.
Argh.. you're right. I drop the __free() from the panel_node.
Thanks,
Marco
>
> >
> > - if (!fsl_ldb->ch0_enabled && !fsl_ldb->ch1_enabled) {
> > - of_node_put(panel_node);
> > + if (!fsl_ldb->ch0_enabled && !fsl_ldb->ch1_enabled)
> > return dev_err_probe(dev, -ENXIO, "No panel node found");
> > - }
> >
> > dev_dbg(dev, "Using %s\n",
> > fsl_ldb_is_dual(fsl_ldb) ? "dual-link mode" :
> > fsl_ldb->ch0_enabled ? "channel 0" : "channel 1");
> >
> > panel = of_drm_find_panel(panel_node);
> > - of_node_put(panel_node);
> > if (IS_ERR(panel))
> > return dev_err_probe(dev, PTR_ERR(panel), "drm panel not
> > found\n");
> >
> > @@ -345,14 +341,12 @@ static int fsl_ldb_probe(struct platform_device *pdev)
> > "drm panel-bridge add failed\n");
> >
> > if (fsl_ldb_is_dual(fsl_ldb)) {
> > - struct device_node *port1, *port2;
> > + struct device_node *port1 __free(device_node) =
> > + of_graph_get_port_by_id(dev->of_node, 1);
> > + struct device_node *port2 __free(device_node) =
> > + of_graph_get_port_by_id(dev->of_node, 2);
> >
> > - port1 = of_graph_get_port_by_id(dev->of_node, 1);
> > - port2 = of_graph_get_port_by_id(dev->of_node, 2);
> > dual_link = drm_of_lvds_get_dual_link_pixel_order(port1, port2);
> > - of_node_put(port1);
> > - of_node_put(port2);
> > -
> > if (dual_link < 0)
> > return dev_err_probe(dev, dual_link,
> > "Error getting dual link
> > configuration\n");
>
> --
> Regards,
>
> Laurent Pinchart
>
