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