When return errors, the clock driver does not unmap
the mapped memory, so fix this issue.
Signed-off-by: Dongjiu Geng <[email protected]>
---
drivers/clk/hisilicon/clk-hi3620.c | 8 ++++++--
drivers/clk/hisilicon/clk.c | 5 ++++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/hisilicon/clk-hi3620.c b/drivers/clk/hisilicon/clk-hi3620.c
index a3d04c7c3da8..d5f1a8df4b1c 100644
--- a/drivers/clk/hisilicon/clk-hi3620.c
+++ b/drivers/clk/hisilicon/clk-hi3620.c
@@ -463,12 +463,16 @@ static void __init hi3620_mmc_clk_init(struct device_node *node)
}
clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
- if (WARN_ON(!clk_data))
+ if (WARN_ON(!clk_data)) {
+ iounmap(base);
return;
+ }
clk_data->clks = kcalloc(num, sizeof(*clk_data->clks), GFP_KERNEL);
- if (!clk_data->clks)
+ if (!clk_data->clks) {
+ iounmap(base);
return;
+ }
for (i = 0; i < num; i++) {
struct hisi_mmc_clock *mmc_clk = &hi3620_mmc_clks[i];
diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c
index 54d9fdc93599..53acaff32934 100644
--- a/drivers/clk/hisilicon/clk.c
+++ b/drivers/clk/hisilicon/clk.c
@@ -69,8 +69,10 @@ struct hisi_clock_data *hisi_clk_init(struct device_node *np,
}
clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
- if (!clk_data)
+ if (!clk_data) {
+ iounmap(base);
goto err;
+ }
clk_data->base = base;
clk_table = kcalloc(nr_clks, sizeof(*clk_table), GFP_KERNEL);
@@ -82,6 +84,7 @@ struct hisi_clock_data *hisi_clk_init(struct device_node *np,
of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data->clk_data);
return clk_data;
err_data:
+ iounmap(base);
kfree(clk_data);
err:
return NULL;
--
2.17.1
On 2020/11/8 21:55, Markus Elfring wrote:
>> When return errors, …
>
> I would find an other wording more appropriate for this change description.
>
>
>> …, so fix this issue.
>
> I suggest to replace this information by an other imperative wording
> and the tag “Fixes”.
OK, done， I have submitted the version 2 patch
>
>
> …
>> +++ b/drivers/clk/hisilicon/clk-hi3620.c
>> @@ -463,12 +463,16 @@ static void __init hi3620_mmc_clk_init(struct device_node *node)
>> }
>>
>> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
>> - if (WARN_ON(!clk_data))
>> + if (WARN_ON(!clk_data)) {
>> + iounmap(base);
>
> Can a statement like “goto unmap_io;” make sense here?
OK， I have changed it.
>
>
>> return;
>> + }
>>
>> clk_data->clks = kcalloc(num, sizeof(*clk_data->clks), GFP_KERNEL);
>> - if (!clk_data->clks)
>> + if (!clk_data->clks) {
>
> How do you think about to add the function call “kfree(clk_data)” in this if branch?
OK, I have changed it.
>
>
> …
>> +++ b/drivers/clk/hisilicon/clk.c
> …
> if (!base) {
> pr_err("%s: failed to map clock registers\n", __func__);
> - goto err;
> + return NULL;
>
>
>> @@ -69,8 +69,10 @@ struct hisi_clock_data *hisi_clk_init(struct device_node *np,
>> }
>>
>> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
>> - if (!clk_data)
>> + if (!clk_data) {
>> + iounmap(base);
>> goto err;
>
> Please use another jump target.
OK, thanks, I have changed it.
>
>
>> @@ -82,6 +84,7 @@ struct hisi_clock_data *hisi_clk_init(struct device_node *np,
>> of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data->clk_data);
>> return clk_data;
>> err_data:
>> + iounmap(base);
>> kfree(clk_data);
>> err:
>> return NULL;
>
> I propose to apply the following code variant.
OK, have modified.
>
> return clk_data;
>
> free_clk_data:
> kfree(clk_data);
> unmap_io:
> iounmap(base);
> return NULL;
>
>
> Regards,
> Markus
> .
>