diff mbox series

cpufreq: mediatek-hw: Fix resource management and error handling in mtk_cpu_resources_init()

Message ID 20240906084057.67680-1-riyandhiman14@gmail.com
State New
Headers show
Series cpufreq: mediatek-hw: Fix resource management and error handling in mtk_cpu_resources_init() | expand

Commit Message

Riyan Dhiman Sept. 6, 2024, 8:40 a.m. UTC
Memory region and IO memory were not released if mtk_cpu_create_freq_table() failed.
Added error handling to ensure that IO memory is unmapped and the memory region is
released properly to prevent resource leaks and ensure all resources are cleaned up on error.

Signed-off-by: Riyan Dhiman <riyandhiman14@gmail.com>
---
 drivers/cpufreq/mediatek-cpufreq-hw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Markus Elfring Sept. 7, 2024, 11:52 a.m. UTC | #1
> Memory region and IO memory were not released if mtk_cpu_create_freq_table() failed.
> Added error handling to ensure that IO memory is unmapped and the memory region is
> released properly to prevent resource leaks and ensure all resources are cleaned up on error.

* Would you like to improve such a change description another bit
  also according to other line length preferences?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.11-rc6#n94

* How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?


Regards,
Markus
Riyan Dhiman Sept. 7, 2024, 3:42 p.m. UTC | #2
>> Memory region and IO memory were not released if mtk_cpu_create_freq_table() failed.
>> Added error handling to ensure that IO memory is unmapped and the memory region is
>> released properly to prevent resource leaks and ensure all resources are cleaned up on error.
>
> * Would you like to improve such a change description another bit

Should I elaborate a little more on the issue and explain the fix I have created?

>  also according to other line length preferences?

Can you please help me a little bit about this? Is it the character lenght of subject?
should I change it to: cpufreq: mediatek: Fix resource leaks on mtk_cpu_create_freq_table() 
failure.

> * How do you think about to add any tags (like "Fixes" and "Cc") accordingly?

Should I change "patch" in subject to "Fixes" or add commit id in the patch message with 
fixes tag? Also, whom should add I in cc since I can't find any other email from get_maintainer 
script.

Regards,
Riyan Dhiman
Markus Elfring Sept. 7, 2024, 4:10 p.m. UTC | #3
>>> Memory region and IO memory were not released if mtk_cpu_create_freq_table() failed.
>>> Added error handling to ensure that IO memory is unmapped and the memory region is
>>> released properly to prevent resource leaks and ensure all resources are cleaned up on error.
>>
>> * Would you like to improve such a change description another bit
>
> Should I elaborate a little more on the issue and explain the fix I have created?

Please take another look at guidance from linked information sources.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.11-rc6#n94


>>  also according to other line length preferences?
>
> Can you please help me a little bit about this?

Yes, of course.


> Is it the character lenght of subject?

Not directly.

The summary phrase can eventually become more succinct.


>> * How do you think about to add any tags (like "Fixes" and "Cc") accordingly?
>
> Should I change "patch" in subject to "Fixes" or …

Obviously not.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.11-rc6#n145

Regards,
Markus
diff mbox series

Patch

diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
index 8925e096d5b9..3b1303f350ec 100644
--- a/drivers/cpufreq/mediatek-cpufreq-hw.c
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -207,13 +207,15 @@  static int mtk_cpu_resources_init(struct platform_device *pdev,
 	ret = mtk_cpu_create_freq_table(pdev, data);
 	if (ret) {
 		dev_info(dev, "Domain-%d failed to create freq table\n", index);
-		return ret;
+		goto unmap_region;
 	}
 
 	policy->freq_table = data->table;
 	policy->driver_data = data;
 
 	return 0;
+unmap_region:
+	iounmap(base);
 release_region:
 	release_mem_region(res->start, resource_size(res));
 	return ret;