* [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw()
2024-05-03 17:52 [PATCH 0/2] cpufreq: sun50i: fix memory leak and remove of_node_put() Javier Carrasco
@ 2024-05-03 17:52 ` Javier Carrasco
2024-05-10 16:49 ` Andre Przywara
2024-05-03 17:52 ` [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler Javier Carrasco
2024-05-20 9:32 ` [PATCH 0/2] cpufreq: sun50i: fix memory leak and remove of_node_put() Viresh Kumar
2 siblings, 1 reply; 12+ messages in thread
From: Javier Carrasco @ 2024-05-03 17:52 UTC (permalink / raw)
To: Yangtao Li, Rafael J. Wysocki, Viresh Kumar, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Andre Przywara, Jonathan Cameron
Cc: linux-pm, linux-arm-kernel, linux-sunxi, linux-kernel,
Javier Carrasco
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 <javier.carrasco.cruz@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw()
2024-05-03 17:52 ` [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw() Javier Carrasco
@ 2024-05-10 16:49 ` Andre Przywara
2024-05-20 7:33 ` Viresh Kumar
0 siblings, 1 reply; 12+ messages in thread
From: Andre Przywara @ 2024-05-10 16:49 UTC (permalink / raw)
To: Javier Carrasco
Cc: Yangtao Li, Rafael J. Wysocki, Viresh Kumar, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Jonathan Cameron, linux-pm,
linux-arm-kernel, linux-sunxi, linux-kernel
On Fri, 03 May 2024 19:52:32 +0200
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
Hi Javier,
> 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.
Ah, thanks for spotting this, there is indeed a leak. Sorry for the
blunder!
> This can be avoided with the new for_each_child_of_node_scoped() macro
> that removes the need for any of_node_put().
Wow, that's the typical convoluted Linux macro, but it looks correct to me.
It would call the put even if the loop ends naturally, but there is a NULL
test in there, so that's fine.
> Fixes: fa5aec9561cf ("cpufreq: sun50i: Add support for opp_supported_hw")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Thanks!
Andre
> ---
> 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;
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw()
2024-05-10 16:49 ` Andre Przywara
@ 2024-05-20 7:33 ` Viresh Kumar
2024-05-20 8:26 ` Andre Przywara
0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2024-05-20 7:33 UTC (permalink / raw)
To: Andre Przywara
Cc: Javier Carrasco, Yangtao Li, Rafael J. Wysocki, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Jonathan Cameron, linux-pm,
linux-arm-kernel, linux-sunxi, linux-kernel
On 10-05-24, 17:49, Andre Przywara wrote:
> On Fri, 03 May 2024 19:52:32 +0200
> Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> > 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;
Why is the opp pointer removed ?
> > 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;
> >
--
viresh
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw()
2024-05-20 7:33 ` Viresh Kumar
@ 2024-05-20 8:26 ` Andre Przywara
2024-05-20 8:34 ` Viresh Kumar
0 siblings, 1 reply; 12+ messages in thread
From: Andre Przywara @ 2024-05-20 8:26 UTC (permalink / raw)
To: Viresh Kumar
Cc: Javier Carrasco, Yangtao Li, Rafael J. Wysocki, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Jonathan Cameron, linux-pm,
linux-arm-kernel, linux-sunxi, linux-kernel
On Mon, 20 May 2024 13:03:39 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:
Hi,
> On 10-05-24, 17:49, Andre Przywara wrote:
> > On Fri, 03 May 2024 19:52:32 +0200
> > Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> > > 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;
>
> Why is the opp pointer removed ?
Because it's now declared *inside* the for_each_child_of_node_scoped loop
below, courtesy of this new macro. The idea is that by doing so, any
"break;" will exit the scope, triggering the cleanup routine. The loop
running till "the end" will also make "opp" exit its scope, triggering the
same routine.
Cheers,
Andre
>
> > > 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;
> > >
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw()
2024-05-20 8:26 ` Andre Przywara
@ 2024-05-20 8:34 ` Viresh Kumar
0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2024-05-20 8:34 UTC (permalink / raw)
To: Andre Przywara
Cc: Javier Carrasco, Yangtao Li, Rafael J. Wysocki, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Jonathan Cameron, linux-pm,
linux-arm-kernel, linux-sunxi, linux-kernel
On 20-05-24, 09:26, Andre Przywara wrote:
> On Mon, 20 May 2024 13:03:39 +0530
> Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Hi,
>
> > On 10-05-24, 17:49, Andre Przywara wrote:
> > > On Fri, 03 May 2024 19:52:32 +0200
> > > Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> > > > 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;
> >
> > Why is the opp pointer removed ?
>
> Because it's now declared *inside* the for_each_child_of_node_scoped loop
> below, courtesy of this new macro. The idea is that by doing so, any
> "break;" will exit the scope, triggering the cleanup routine. The loop
> running till "the end" will also make "opp" exit its scope, triggering the
> same routine.
Applied. Thanks.
--
viresh
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler
2024-05-03 17:52 [PATCH 0/2] cpufreq: sun50i: fix memory leak and remove of_node_put() Javier Carrasco
2024-05-03 17:52 ` [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw() Javier Carrasco
@ 2024-05-03 17:52 ` Javier Carrasco
2024-05-10 17:42 ` Andre Przywara
2024-05-20 9:32 ` [PATCH 0/2] cpufreq: sun50i: fix memory leak and remove of_node_put() Viresh Kumar
2 siblings, 1 reply; 12+ messages in thread
From: Javier Carrasco @ 2024-05-03 17:52 UTC (permalink / raw)
To: Yangtao Li, Rafael J. Wysocki, Viresh Kumar, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Andre Przywara, Jonathan Cameron
Cc: linux-pm, linux-arm-kernel, linux-sunxi, linux-kernel,
Javier Carrasco
Make use of the __free() cleanup handler to automatically free nodes
when they get out of scope.
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler
2024-05-03 17:52 ` [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler Javier Carrasco
@ 2024-05-10 17:42 ` Andre Przywara
2024-05-20 8:35 ` Viresh Kumar
0 siblings, 1 reply; 12+ messages in thread
From: Andre Przywara @ 2024-05-10 17:42 UTC (permalink / raw)
To: Javier Carrasco
Cc: Yangtao Li, Rafael J. Wysocki, Viresh Kumar, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Jonathan Cameron, linux-pm,
linux-arm-kernel, linux-sunxi, linux-kernel
On Fri, 03 May 2024 19:52:33 +0200
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
Hi,
> Make use of the __free() cleanup handler to automatically free nodes
> when they get out of scope.
Looks alright, the last function is now particularly neat.
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
I haven't tested the error paths yet, but it certainly boots fine on an
OrangePi Zero3.
Cheers,
Andre
> ---
> 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);
> }
>
> /*
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler
2024-05-10 17:42 ` Andre Przywara
@ 2024-05-20 8:35 ` Viresh Kumar
2024-05-20 9:28 ` Markus Elfring
0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2024-05-20 8:35 UTC (permalink / raw)
To: Andre Przywara
Cc: Javier Carrasco, Yangtao Li, Rafael J. Wysocki, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Jonathan Cameron, linux-pm,
linux-arm-kernel, linux-sunxi, linux-kernel
On 10-05-24, 18:42, Andre Przywara wrote:
> On Fri, 03 May 2024 19:52:33 +0200
> Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>
> I haven't tested the error paths yet, but it certainly boots fine on an
> OrangePi Zero3.
>
> > ---
> > 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);
Won't that result in build warning, mixed code and definitions now ?
--
viresh
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler
2024-05-20 8:35 ` Viresh Kumar
@ 2024-05-20 9:28 ` Markus Elfring
2024-05-20 9:31 ` Viresh Kumar
0 siblings, 1 reply; 12+ messages in thread
From: Markus Elfring @ 2024-05-20 9:28 UTC (permalink / raw)
To: Viresh Kumar, Javier Carrasco, linux-sunxi, linux-arm-kernel,
linux-pm, kernel-janitors
Cc: LKML, Andre Przywara, Chen-Yu Tsai, Jernej Skrabec,
Jonathan Cameron, Peter Zijlstra, Rafael J. Wysocki,
Samuel Holland, Yangtao Li
…
> > > +++ 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);
>
> Won't that result in build warning, mixed code and definitions now ?
I suggest to take another look at a corresponding information source.
[PATCH v3 04/57] kbuild: Drop -Wdeclaration-after-statement
https://lore.kernel.org/all/20230612093537.693926033@infradead.org/
See also:
https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Warning-Options.html#index-Wdeclaration-after-statement
Would you like to stress a scope reduction for the affected local variable
by adding any curly brackets?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler
2024-05-20 9:28 ` Markus Elfring
@ 2024-05-20 9:31 ` Viresh Kumar
0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2024-05-20 9:31 UTC (permalink / raw)
To: Markus Elfring
Cc: Javier Carrasco, linux-sunxi, linux-arm-kernel, linux-pm,
kernel-janitors, LKML, Andre Przywara, Chen-Yu Tsai,
Jernej Skrabec, Jonathan Cameron, Peter Zijlstra,
Rafael J. Wysocki, Samuel Holland, Yangtao Li
On 20-05-24, 11:28, Markus Elfring wrote:
> …
> > > > +++ 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);
> >
> > Won't that result in build warning, mixed code and definitions now ?
>
> I suggest to take another look at a corresponding information source.
>
> [PATCH v3 04/57] kbuild: Drop -Wdeclaration-after-statement
> https://lore.kernel.org/all/20230612093537.693926033@infradead.org/
Ah, I wasn't aware of this one.
> See also:
> https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Warning-Options.html#index-Wdeclaration-after-statement
>
>
> Would you like to stress a scope reduction for the affected local variable
> by adding any curly brackets?
No, it looks fine.
--
viresh
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] cpufreq: sun50i: fix memory leak and remove of_node_put()
2024-05-03 17:52 [PATCH 0/2] cpufreq: sun50i: fix memory leak and remove of_node_put() Javier Carrasco
2024-05-03 17:52 ` [PATCH 1/2] cpufreq: sun50i: fix memory leak in dt_has_supported_hw() Javier Carrasco
2024-05-03 17:52 ` [PATCH 2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler Javier Carrasco
@ 2024-05-20 9:32 ` Viresh Kumar
2 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2024-05-20 9:32 UTC (permalink / raw)
To: Javier Carrasco
Cc: Yangtao Li, Rafael J. Wysocki, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Andre Przywara, Jonathan Cameron, linux-pm,
linux-arm-kernel, linux-sunxi, linux-kernel
On 03-05-24, 19:52, Javier Carrasco wrote:
> 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 <javier.carrasco.cruz@gmail.com>
> ---
> Javier Carrasco (2):
> cpufreq: sun50i: fix memory leak in dt_has_supported_hw()
> cpufreq: sun50i: replace of_node_put() with automatic cleanup handler
Applied. Thanks.
--
viresh
^ permalink raw reply [flat|nested] 12+ messages in thread