Drop OF node reference immediately after using it in
syscon_node_to_regmap(), which is both simpler and typical/expected
code pattern.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/pmdomain/rockchip/pm-domains.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 64b4d7120d83..5ee7efbd2ef8 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -716,12 +716,11 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 				goto err_unprepare_clocks;
 			}
 			pd->qos_regmap[j] = syscon_node_to_regmap(qos_node);
+			of_node_put(qos_node);
 			if (IS_ERR(pd->qos_regmap[j])) {
 				error = -ENODEV;
-				of_node_put(qos_node);
 				goto err_unprepare_clocks;
 			}
-			of_node_put(qos_node);
 		}
 	}
 
-- 
2.43.0

> Drop OF node reference immediately after using it in
> syscon_node_to_regmap(), which is both simpler and typical/expected
> code pattern.

Dear Krzysztof,

I noticed also this contribution.
I found it easy to convert it also into the following small script variant
for the semantic patch language (Coccinelle software).


@adjustment@
expression e, x;
@@
+of_node_put(e);
 if (...)
 {
 <+... when != e = x
-   of_node_put(e);
 ...+>
 }
-of_node_put(e);


58 patches were accordingly generated for source files of the software “Linux next-20240913”.
How would we like to tackle remaining update candidates according to similar transformation patterns?

Regards,
Markus

…
> I found it easy to convert it also into the following small script variant
> for the semantic patch language (Coccinelle software).
>
>
> @adjustment@
> expression e, x;
> @@
> +of_node_put(e);
>  if (...)
>  {
>  <+... when != e = x
> -   of_node_put(e);
>  ...+>
>  }
> -of_node_put(e);
>
>
> 58 patches were accordingly generated for source files of the software “Linux next-20240913”.
…

This SmPL script references a known function name.
https://elixir.bootlin.com/linux/v6.11-rc7/source/drivers/of/dynamic.c#L43

I imagine that it can become helpful to avoid duplicate function calls
also in combination with other functions.
But there are development challenges to consider for functions like mutex_unlock()
which get addresses passed for data structure members.
Thus I hoped that something can be achieved with the following script variant.


@extended_adjustment@
expression e, f, x, y;
@@
+f(e);
 if (...)
 {
 <+... when != \( e = x \| y(..., &e, ...) \)
(   f(&e);
|
-   f(e);
)
 ...+>
 }
(f(&e);
|
-f(e);
)


But the software combination “Coccinelle 1.2-00038-g2297fc0f” points details out
for further development considerations.

Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses> spatch drivers/pmdomain/rockchip/pm-domains.c …/Projekte/Coccinelle/janitor/avoid_duplicate_function_call.cocci
…
previous modification:

  <<< f(e);
CONTEXT

According to environment 2:
   extended_adjustment.e -> pmu->mutex

   extended_adjustment.f -> mutex_unlock


current modification:

  <<< f(e);
CONTEXT

According to environment 2:
   extended_adjustment.e -> &pmu->mutex

   extended_adjustment.f -> mutex_unlock


exn while in timeout_function
extended_adjustment: already tagged token:
C code context
File "drivers/pmdomain/rockchip/pm-domains.c", line 538, column 1, charpos = 14292
  around = 'if',
  whole content =       if (rockchip_pmu_domain_is_on(pd) != power_on) {



How can such a software situation be improved?

Regards,
Markus

On Sun, 25 Aug 2024 at 20:31, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> Drop OF node reference immediately after using it in
> syscon_node_to_regmap(), which is both simpler and typical/expected
> code pattern.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/pmdomain/rockchip/pm-domains.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 64b4d7120d83..5ee7efbd2ef8 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -716,12 +716,11 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>                                 goto err_unprepare_clocks;
>                         }
>                         pd->qos_regmap[j] = syscon_node_to_regmap(qos_node);
> +                       of_node_put(qos_node);
>                         if (IS_ERR(pd->qos_regmap[j])) {
>                                 error = -ENODEV;
> -                               of_node_put(qos_node);
>                                 goto err_unprepare_clocks;
>                         }
> -                       of_node_put(qos_node);
>                 }
>         }
>
> --
> 2.43.0
>

Am Sonntag, 25. August 2024, 20:31:16 CEST schrieb Krzysztof Kozlowski:
> Drop OF node reference immediately after using it in
> syscon_node_to_regmap(), which is both simpler and typical/expected
> code pattern.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>