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
> ---
> changes in v2:
> - update Fixes tag.
> v1 Link: https://lore.kernel.org/r/20220602155615.43277-1-linmq...@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(>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 = >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.
--
Best regards,
Dmitry
