Re: [PATCH v2] drm/tegra: dc: rgb: Fix refcount leak in tegra_dc_rgb_probe

On 6/3/22 16:27, Miaoqian Lin wrote:

of_get_child_by_name() returns a node pointer with refcount incremented, we should use of_node_put() on it when not need anymore. So add of_node_put() in error paths.

Fixes: d8f4a9eda006 ("drm: Add NVIDIA Tegra20 support") Signed-off-by: Miaoqian Lin linmq006@gmail.com

---

changes in v2:

• update Fixes tag.

v1 Link: https://lore.kernel.org/r/20220602155615.43277-1-linmq006@gmail.com

drivers/gpu/drm/tegra/rgb.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c index ff8fce36d2aa..cef2b1b72385 100644 --- a/drivers/gpu/drm/tegra/rgb.c +++ b/drivers/gpu/drm/tegra/rgb.c @@ -196,12 +196,16 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) int err;

np = of_get_child_by_name(dc->dev->of_node, "rgb");

• if (!np || !of_device_is_available(np))

• return -ENODEV;
  

• if (!np || !of_device_is_available(np)) {

• err = -ENODEV;
  

• goto err_put_node;
  

• }

  rgb = devm_kzalloc(dc->dev, sizeof(*rgb), GFP_KERNEL);

• if (!rgb)

• return -ENOMEM;
  

• if (!rgb) {

• err = -ENOMEM;
  

• goto err_put_node;
  

• }

  rgb->output.dev = dc->dev; rgb->output.of_node = np;

@@ -209,31 +213,34 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc)

err = tegra_output_probe(&rgb->output); if (err < 0)

• return err;
  

• goto err_put_node;
  

• rgb->clk = devm_clk_get(dc->dev, NULL); if (IS_ERR(rgb->clk)) { dev_err(dc->dev, "failed to get clock\n");

• return PTR_ERR(rgb->clk);
  

• err =  PTR_ERR(rgb->clk);
  

• goto err_put_node;
  

  }

  rgb->clk_parent = devm_clk_get(dc->dev, "parent"); if (IS_ERR(rgb->clk_parent)) { dev_err(dc->dev, "failed to get parent clock\n");

• return PTR_ERR(rgb->clk_parent);
  

• err = PTR_ERR(rgb->clk_parent);
  

• goto err_put_node;
  

  }

  err = clk_set_parent(rgb->clk, rgb->clk_parent); if (err < 0) { dev_err(dc->dev, "failed to set parent clock: %d\n", err);

• return err;
  

• goto err_put_node;
  

  }

  rgb->pll_d_out0 = clk_get_sys(NULL, "pll_d_out0"); if (IS_ERR(rgb->pll_d_out0)) { err = PTR_ERR(rgb->pll_d_out0); dev_err(dc->dev, "failed to get pll_d_out0: %d\n", err);

• return err;
  

• goto err_put_node;
  

  }

  if (dc->soc->has_pll_d2_out0) {

@@ -241,13 +248,17 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) if (IS_ERR(rgb->pll_d2_out0)) { err = PTR_ERR(rgb->pll_d2_out0); dev_err(dc->dev, "failed to get pll_d2_out0: %d\n", err);

• 	return err;
  

• 	goto err_put_node;
  

  } }

  dc->rgb = &rgb->output;

  return 0;

+err_put_node:

• of_node_put(np);
• return err;

}

int tegra_dc_rgb_remove(struct tegra_dc *dc)

Doesn't look like the node is put on driver removal either. Hence you should make it resource-managed.