This series fixes a memory leak by means of the _scoped version of the
for_each_child_of_node() loop, which was recently introduced with
34af4554fb0c ("of: Introduce for_each_*_child_of_node_scoped() to
automate of_node_put() handling").
The new approach is still not widely used, but this might be a good
occasion to use it in a driver because it actually fixes a bug, and
the loop is rather simple.
The creator of the new macro was added to the discussion in case the
new approach is still not mature enough, even for such simple case.
Additionally, the existing uses of of_node_put() have been removed to
favour the _free() cleanup handler, which reduces the chances of having
any other memory leak because some of_node_put() is missing as well as
simplifies the current code.
I don't have the real hardware to test the series, so I "faked" the node
in a device tree for an arm64 device (Rockchip) and hacked the driver
to get to run dt_has_supported_hw(). The new implementation works as
expected, but if someone wants to test it with the proper SoC,
additional tests are always welcome. The same applies for the removals
of of_node_put().
Signed-off-by: Javier Carrasco <[email protected]>
---
Javier Carrasco (2):
cpufreq: sun50i: fix memory leak in dt_has_supported_hw()
cpufreq: sun50i: replace of_node_put() with automatic cleanup handler
drivers/cpufreq/sun50i-cpufreq-nvmem.c | 27 +++++++++------------------
1 file changed, 9 insertions(+), 18 deletions(-)
---
base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
change-id: 20240503-sun50i-cpufreq-nvmem-cleanup-40a4cf8fa56d
Best regards,
--
Javier Carrasco <[email protected]>
The for_each_child_of_node() loop does not decrement the child node
refcount before the break instruction, even though the node is no
longer required.
This can be avoided with the new for_each_child_of_node_scoped() macro
that removes the need for any of_node_put().
Fixes: fa5aec9561cf ("cpufreq: sun50i: Add support for opp_supported_hw")
Signed-off-by: Javier Carrasco <[email protected]>
---
drivers/cpufreq/sun50i-cpufreq-nvmem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
index 0b882765cd66..ef83e4bf2639 100644
--- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
+++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
@@ -131,7 +131,7 @@ static const struct of_device_id cpu_opp_match_list[] = {
static bool dt_has_supported_hw(void)
{
bool has_opp_supported_hw = false;
- struct device_node *np, *opp;
+ struct device_node *np;
struct device *cpu_dev;
cpu_dev = get_cpu_device(0);
@@ -142,7 +142,7 @@ static bool dt_has_supported_hw(void)
if (!np)
return false;
- for_each_child_of_node(np, opp) {
+ for_each_child_of_node_scoped(np, opp) {
if (of_find_property(opp, "opp-supported-hw", NULL)) {
has_opp_supported_hw = true;
break;
--
2.40.1
Make use of the __free() cleanup handler to automatically free nodes
when they get out of scope.
Signed-off-by: Javier Carrasco <[email protected]>
---
drivers/cpufreq/sun50i-cpufreq-nvmem.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
index ef83e4bf2639..eb47c193269c 100644
--- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
+++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
@@ -131,14 +131,14 @@ static const struct of_device_id cpu_opp_match_list[] = {
static bool dt_has_supported_hw(void)
{
bool has_opp_supported_hw = false;
- struct device_node *np;
struct device *cpu_dev;
cpu_dev = get_cpu_device(0);
if (!cpu_dev)
return false;
- np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+ struct device_node *np __free(device_node) =
+ dev_pm_opp_of_get_opp_desc_node(cpu_dev);
if (!np)
return false;
@@ -149,8 +149,6 @@ static bool dt_has_supported_hw(void)
}
}
- of_node_put(np);
-
return has_opp_supported_hw;
}
@@ -165,7 +163,6 @@ static int sun50i_cpufreq_get_efuse(void)
const struct sunxi_cpufreq_data *opp_data;
struct nvmem_cell *speedbin_nvmem;
const struct of_device_id *match;
- struct device_node *np;
struct device *cpu_dev;
u32 *speedbin;
int ret;
@@ -174,19 +171,18 @@ static int sun50i_cpufreq_get_efuse(void)
if (!cpu_dev)
return -ENODEV;
- np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+ struct device_node *np __free(device_node) =
+ dev_pm_opp_of_get_opp_desc_node(cpu_dev);
if (!np)
return -ENOENT;
match = of_match_node(cpu_opp_match_list, np);
- if (!match) {
- of_node_put(np);
+ if (!match)
return -ENOENT;
- }
+
opp_data = match->data;
speedbin_nvmem = of_nvmem_cell_get(np, NULL);
- of_node_put(np);
if (IS_ERR(speedbin_nvmem))
return dev_err_probe(cpu_dev, PTR_ERR(speedbin_nvmem),
"Could not get nvmem cell\n");
@@ -301,14 +297,9 @@ MODULE_DEVICE_TABLE(of, sun50i_cpufreq_match_list);
static const struct of_device_id *sun50i_cpufreq_match_node(void)
{
- const struct of_device_id *match;
- struct device_node *np;
-
- np = of_find_node_by_path("/");
- match = of_match_node(sun50i_cpufreq_match_list, np);
- of_node_put(np);
+ struct device_node *np __free(device_node) = of_find_node_by_path("/");
- return match;
+ return of_match_node(sun50i_cpufreq_match_list, np);
}
/*
--
2.40.1