* [PATCH v2] bus: (mvebu-mbus) Add missing of_node_put()
@ 2022-06-16 2:01 Liang He
2022-06-16 13:14 ` Pali Rohár
2022-06-16 19:08 ` Christophe JAILLET
0 siblings, 2 replies; 3+ messages in thread
From: Liang He @ 2022-06-16 2:01 UTC (permalink / raw)
To: pali, lpieralisi; +Cc: windhl, linux-kernel
In mvebu_mbus_dt_init(), of_find_matching_node_and_match() and
of_find_node_by_phandle() will both return node pointers with
refcount incremented. We should use of_node_put() in fail path
or when it is not used anymore.
Signed-off-by: Liang He <windhl@126.com>
---
changelog:
v2: (1) use real name (2) add of_node_put when not used anymore
v1: add of_node_put in fail path
drivers/bus/mvebu-mbus.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index db612045616f..e168c8de2ae8 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -1327,22 +1327,28 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
prop = of_get_property(np, "controller", NULL);
if (!prop) {
+ of_node_put(np);
pr_err("required 'controller' property missing\n");
return -EINVAL;
}
controller = of_find_node_by_phandle(be32_to_cpup(prop));
if (!controller) {
+ of_node_put(np);
pr_err("could not find an 'mbus-controller' node\n");
return -ENODEV;
}
if (of_address_to_resource(controller, 0, &mbuswins_res)) {
+ of_node_put(np);
+ of_node_put(controller);
pr_err("cannot get MBUS register address\n");
return -EINVAL;
}
if (of_address_to_resource(controller, 1, &sdramwins_res)) {
+ of_node_put(np);
+ of_node_put(controller);
pr_err("cannot get SDRAM register address\n");
return -EINVAL;
}
@@ -1360,6 +1366,8 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
pr_warn(FW_WARN "deprecated mbus-mvebu Device Tree, suspend/resume will not work\n");
}
+ of_node_put(controller);
+
mbus_state.hw_io_coherency = is_coherent;
/* Get optional pcie-{mem,io}-aperture properties */
@@ -1378,6 +1386,11 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
return ret;
/* Setup statically declared windows in the DT */
- return mbus_dt_setup(&mbus_state, np);
+ ret = mbus_dt_setup(&mbus_state, np);
+
+ of_node_put(np);
+
+ return ret;
+
}
#endif
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] bus: (mvebu-mbus) Add missing of_node_put()
2022-06-16 2:01 [PATCH v2] bus: (mvebu-mbus) Add missing of_node_put() Liang He
@ 2022-06-16 13:14 ` Pali Rohár
2022-06-16 19:08 ` Christophe JAILLET
1 sibling, 0 replies; 3+ messages in thread
From: Pali Rohár @ 2022-06-16 13:14 UTC (permalink / raw)
To: Liang He; +Cc: lpieralisi, linux-kernel
On Thursday 16 June 2022 10:01:35 Liang He wrote:
> In mvebu_mbus_dt_init(), of_find_matching_node_and_match() and
> of_find_node_by_phandle() will both return node pointers with
> refcount incremented. We should use of_node_put() in fail path
> or when it is not used anymore.
>
> Signed-off-by: Liang He <windhl@126.com>
Acked-by: Pali Rohár <pali@kernel.org>
> ---
> changelog:
>
> v2: (1) use real name (2) add of_node_put when not used anymore
>
> v1: add of_node_put in fail path
>
>
> drivers/bus/mvebu-mbus.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> index db612045616f..e168c8de2ae8 100644
> --- a/drivers/bus/mvebu-mbus.c
> +++ b/drivers/bus/mvebu-mbus.c
> @@ -1327,22 +1327,28 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
>
> prop = of_get_property(np, "controller", NULL);
> if (!prop) {
> + of_node_put(np);
> pr_err("required 'controller' property missing\n");
> return -EINVAL;
> }
>
> controller = of_find_node_by_phandle(be32_to_cpup(prop));
> if (!controller) {
> + of_node_put(np);
> pr_err("could not find an 'mbus-controller' node\n");
> return -ENODEV;
> }
>
> if (of_address_to_resource(controller, 0, &mbuswins_res)) {
> + of_node_put(np);
> + of_node_put(controller);
> pr_err("cannot get MBUS register address\n");
> return -EINVAL;
> }
>
> if (of_address_to_resource(controller, 1, &sdramwins_res)) {
> + of_node_put(np);
> + of_node_put(controller);
> pr_err("cannot get SDRAM register address\n");
> return -EINVAL;
> }
> @@ -1360,6 +1366,8 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
> pr_warn(FW_WARN "deprecated mbus-mvebu Device Tree, suspend/resume will not work\n");
> }
>
> + of_node_put(controller);
> +
> mbus_state.hw_io_coherency = is_coherent;
>
> /* Get optional pcie-{mem,io}-aperture properties */
> @@ -1378,6 +1386,11 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
> return ret;
>
> /* Setup statically declared windows in the DT */
> - return mbus_dt_setup(&mbus_state, np);
> + ret = mbus_dt_setup(&mbus_state, np);
> +
> + of_node_put(np);
> +
> + return ret;
> +
> }
> #endif
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] bus: (mvebu-mbus) Add missing of_node_put()
2022-06-16 2:01 [PATCH v2] bus: (mvebu-mbus) Add missing of_node_put() Liang He
2022-06-16 13:14 ` Pali Rohár
@ 2022-06-16 19:08 ` Christophe JAILLET
1 sibling, 0 replies; 3+ messages in thread
From: Christophe JAILLET @ 2022-06-16 19:08 UTC (permalink / raw)
To: Liang He, pali, lpieralisi; +Cc: linux-kernel
Le 16/06/2022 à 04:01, Liang He a écrit :
> In mvebu_mbus_dt_init(), of_find_matching_node_and_match() and
> of_find_node_by_phandle() will both return node pointers with
> refcount incremented. We should use of_node_put() in fail path
> or when it is not used anymore.
>
> Signed-off-by: Liang He <windhl@126.com>
> ---
> changelog:
>
> v2: (1) use real name (2) add of_node_put when not used anymore
>
> v1: add of_node_put in fail path
>
>
> drivers/bus/mvebu-mbus.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> index db612045616f..e168c8de2ae8 100644
> --- a/drivers/bus/mvebu-mbus.c
> +++ b/drivers/bus/mvebu-mbus.c
> @@ -1327,22 +1327,28 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
>
> prop = of_get_property(np, "controller", NULL);
> if (!prop) {
> + of_node_put(np);
> pr_err("required 'controller' property missing\n");
> return -EINVAL;
> }
>
> controller = of_find_node_by_phandle(be32_to_cpup(prop));
> if (!controller) {
> + of_node_put(np);
> pr_err("could not find an 'mbus-controller' node\n");
> return -ENODEV;
> }
>
> if (of_address_to_resource(controller, 0, &mbuswins_res)) {
> + of_node_put(np);
> + of_node_put(controller);
> pr_err("cannot get MBUS register address\n");
> return -EINVAL;
> }
>
> if (of_address_to_resource(controller, 1, &sdramwins_res)) {
> + of_node_put(np);
> + of_node_put(controller);
> pr_err("cannot get SDRAM register address\n");
> return -EINVAL;
> }
> @@ -1360,6 +1366,8 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
> pr_warn(FW_WARN "deprecated mbus-mvebu Device Tree, suspend/resume will not work\n");
> }
>
> + of_node_put(controller);
> +
> mbus_state.hw_io_coherency = is_coherent;
>
> /* Get optional pcie-{mem,io}-aperture properties */
> @@ -1378,6 +1386,11 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
> return ret;
I guess that a "of_node_put(np);" is missing before this return.
This really looks like an error handling path and it has not been
updated as all the other path, including the normal one below.
Moreover having an error handling path and some gotos is a more usual
solution. It avoids code duplication. (but it is also a matter of taste...)
CJ
>
> /* Setup statically declared windows in the DT */
> - return mbus_dt_setup(&mbus_state, np);
> + ret = mbus_dt_setup(&mbus_state, np);
> +
> + of_node_put(np);
> +
> + return ret;
> +
Nitpick : This extra new line is not needed.
> }
> #endif
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-16 19:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16 2:01 [PATCH v2] bus: (mvebu-mbus) Add missing of_node_put() Liang He
2022-06-16 13:14 ` Pali Rohár
2022-06-16 19:08 ` Christophe JAILLET
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®