diff mbox series

[2/2] cpufreq: sun50i: replace of_node_put() with automatic cleanup handler

Message ID 20240503-sun50i-cpufreq-nvmem-cleanup-v1-2-0a2352cac46b@gmail.com
State Accepted
Commit fa8036e5fed21d871aef23092990bfbaddc521d5
Headers show
Series cpufreq: sun50i: fix memory leak and remove of_node_put() | expand

Commit Message

Javier Carrasco May 3, 2024, 5:52 p.m. UTC
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(-)

Comments

Andre Przywara May 10, 2024, 5:42 p.m. UTC | #1
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);
>  }
>  
>  /*
>
Viresh Kumar May 20, 2024, 8:35 a.m. UTC | #2
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 ?
diff mbox series

Patch

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);
 }
 
 /*