clkdev: prevent potential memory leak when used in modules

PATCH Superseded Andy Shevchenko 3 participants 5 messages Feb 3, 2015 – Feb 4, 2015

Subsystems (10)

Arm (32-bit) Arm sub-architectures MIPS Common clk +6 more subsystems

lore

Share & tools

lore

Andy Shevchenko

author

2015-02-03 18:23 UTC #0 lore raw

PATCH 1/1 clkdev: prevent potential memory leak when used in modules

Since clk_register_clkdev() is exported for modules the caller should get a pointer to the allocated resources. Otherwise the memory leak is guaranteed on the ->remove() stage.

The patch changes the prototype of the clk_register_clkdev() to return pointer to the allocated resources. The caller should take care of the returned variable and free allocated resources when needed. Together with the main change the users are updated too.

The patch compile tested on x86, thus requires to be tested on touched architectures.

Andy Shevchenko (1):
clkdev: change prototype of clk_register_clkdev()

 arch/arm/mach-msm/clock-pcom.c         |  9 +++++----
 arch/arm/mach-vexpress/spc.c           |  5 ++++-
 arch/mips/ath79/clock.c                |  6 +++---
 drivers/clk/clk-bcm2835.c              | 12 +++++++-----
 drivers/clk/clk-max-gen.c              |  9 ++++-----
 drivers/clk/clk-xgene.c                |  6 +++---
 drivers/clk/clkdev.c                   | 14 +++++++++-----
 drivers/clk/samsung/clk-pll.c          | 13 ++++++++-----
 drivers/clk/samsung/clk-s3c2410-dclk.c | 19 +++++++++---------
 drivers/clk/samsung/clk.c              | 35 +++++++++++++++++++---------------
 include/linux/clkdev.h                 |  2 +-
 11 files changed, 74 insertions(+), 56 deletions(-)

Signature · 1 line

2.1.4

Andy Shevchenko

author

2015-02-03 18:18 UTC #1 lore raw

PATCH 1/1 clkdev: change prototype of clk_register_clkdev()

Since clk_register_clkdev() is exported for modules the caller should get a pointer to the allocated resources. Otherwise the memory leak is guaranteed on the ->remove() stage.

Cc Tomeu Vizoso <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 arch/arm/mach-msm/clock-pcom.c         |  9 +++++----
 arch/arm/mach-vexpress/spc.c           |  5 ++++-
 arch/mips/ath79/clock.c                |  6 +++---
 drivers/clk/clk-bcm2835.c              | 12 +++++++-----
 drivers/clk/clk-max-gen.c              |  9 ++++-----
 drivers/clk/clk-xgene.c                |  6 +++---
 drivers/clk/clkdev.c                   | 14 +++++++++-----
 drivers/clk/samsung/clk-pll.c          | 13 ++++++++-----
 drivers/clk/samsung/clk-s3c2410-dclk.c | 19 +++++++++---------
 drivers/clk/samsung/clk.c              | 35 +++++++++++++++++++---------------
 include/linux/clkdev.h                 |  2 +-
 11 files changed, 74 insertions(+), 56 deletions(-)

arch/arm/mach-msm/clock-pcom.c +5 -4

index f5b69d7..d1ebbfe 100644
@@ -128,7 +128,7 @@ static struct clk_ops clk_ops_pcom = {
 static int msm_clock_pcom_probe(struct platform_device *pdev)
 {
 	const struct pcom_clk_pdata *pdata = pdev->dev.platform_data;
-	int i, ret;
+	int i;
 
 	for (i = 0; i < pdata->num_lookups; i++) {
 		const struct clk_pcom_desc *desc = &pdata->lookup[i];

@@ -136,6 +136,7 @@ static int msm_clock_pcom_probe(struct platform_device *pdev)
 		struct clk_pcom *p;
 		struct clk_hw *hw;
 		struct clk_init_data init;
+		struct clk_lookup *cl;
 
 		p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
 		if (!p)

@@ -157,9 +158,9 @@ static int msm_clock_pcom_probe(struct platform_device *pdev)
 			init.flags |= CLK_IGNORE_UNUSED;
 
 		c = devm_clk_register(&pdev->dev, hw);
-		ret = clk_register_clkdev(c, desc->con, desc->dev);
-		if (ret)
-			return ret;
+		cl = clk_register_clkdev(c, desc->con, desc->dev);
+		if (IS_ERR(cl))
+			return PTR_ERR(cl);
 	}
 
 	return 0;

arch/arm/mach-vexpress/spc.c +4 -1

index f61158c..0477007 100644
@@ -568,6 +568,8 @@ static int __init ve_spc_clk_init(void)
 
 	for_each_possible_cpu(cpu) {
 		struct device *cpu_dev = get_cpu_device(cpu);
+		struct clk_lookup *cl;
+
 		if (!cpu_dev) {
 			pr_warn("failed to get cpu%d device\n", cpu);
 			continue;

@@ -577,7 +579,8 @@ static int __init ve_spc_clk_init(void)
 			pr_warn("failed to register cpu%d clock\n", cpu);
 			continue;
 		}
-		if (clk_register_clkdev(clk, NULL, dev_name(cpu_dev))) {
+		cl = clk_register_clkdev(clk, NULL, dev_name(cpu_dev));
+		if (IS_ERR(cl)) {
 			pr_warn("failed to register cpu%d clock lookup\n", cpu);
 			continue;
 		}

arch/mips/ath79/clock.c +3 -3

index 26479f4..db5ed95 100644
@@ -35,7 +35,7 @@ struct clk {
 static void __init ath79_add_sys_clkdev(const char *id, unsigned long rate)
 {
 	struct clk *clk;
-	int err;
+	struct clk_lookup *cl;
 
 	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
 	if (!clk)

@@ -43,8 +43,8 @@ static void __init ath79_add_sys_clkdev(const char *id, unsigned long rate)
 
 	clk->rate = rate;
 
-	err = clk_register_clkdev(clk, id, NULL);
-	if (err)
+	cl = clk_register_clkdev(clk, id, NULL);
+	if (IS_ERR(cl))
 		panic("unable to register %s clock device", id);
 }

drivers/clk/clk-bcm2835.c +7 -5

index 6b950ca..6243527 100644
@@ -30,7 +30,7 @@
 void __init bcm2835_init_clocks(void)
 {
 	struct clk *clk;
-	int ret;
+	struct clk_lookup *cl;
 
 	clk = clk_register_fixed_rate(NULL, "sys_pclk", NULL, CLK_IS_ROOT,
 					250000000);

@@ -46,15 +46,17 @@ void __init bcm2835_init_clocks(void)
 					3000000);
 	if (IS_ERR(clk))
 		pr_err("uart0_pclk not registered\n");
-	ret = clk_register_clkdev(clk, NULL, "20201000.uart");
-	if (ret)
+
+	cl = clk_register_clkdev(clk, NULL, "20201000.uart");
+	if (IS_ERR(cl))
 		pr_err("uart0_pclk alias not registered\n");
 
 	clk = clk_register_fixed_rate(NULL, "uart1_pclk", NULL, CLK_IS_ROOT,
 					125000000);
 	if (IS_ERR(clk))
 		pr_err("uart1_pclk not registered\n");
-	ret = clk_register_clkdev(clk, NULL, "20215000.uart");
-	if (ret)
+
+	cl = clk_register_clkdev(clk, NULL, "20215000.uart");
+	if (IS_ERR(cl))
 		pr_err("uart1_pclk alias not registered\n");
 }

drivers/clk/clk-max-gen.c +4 -5

index 6505049..cd19136 100644
@@ -92,16 +92,15 @@ static struct clk *max_gen_clk_register(struct device *dev,
 {
 	struct clk *clk;
 	struct clk_hw *hw = &max_gen->hw;
-	int ret;
+	struct clk_lookup *cl;
 
 	clk = devm_clk_register(dev, hw);
 	if (IS_ERR(clk))
 		return clk;
 
-	ret = clk_register_clkdev(clk, hw->init->name, NULL);
-
-	if (ret)
-		return ERR_PTR(ret);
+	cl = clk_register_clkdev(clk, hw->init->name, NULL);
+	if (IS_ERR(cl))
+		return ERR_CAST(cl);
 
 	return clk;
 }

drivers/clk/clk-xgene.c +3 -3

index dd8a62d..ce5bf4d 100644
@@ -401,8 +401,8 @@ static struct clk *xgene_register_clk(struct device *dev,
 {
 	struct xgene_clk *apmclk;
 	struct clk *clk;
+	struct clk_lookup *cl;
 	struct clk_init_data init;
-	int rc;
 
 	/* allocate the APM clock structure */
 	apmclk = kzalloc(sizeof(*apmclk), GFP_KERNEL);

@@ -431,8 +431,8 @@ static struct clk *xgene_register_clk(struct device *dev,
 	}
 
 	/* Register the clock for lookup */
-	rc = clk_register_clkdev(clk, name, NULL);
-	if (rc != 0) {
+	cl = clk_register_clkdev(clk, name, NULL);
+	if (IS_ERR(cl)) {
 		pr_err("%s: could not register lookup clk %s\n",
 			__func__, name);
 	}

drivers/clk/clkdev.c +9 -5

index 6e5c504..e07b1e2 100644
@@ -307,29 +307,33 @@ EXPORT_SYMBOL(clkdev_drop);
  * clkdev.
  *
  * To make things easier for mass registration, we detect error clks
- * from a previous clk_register() call, and return the error code for
+ * from a previous clk_register() call, and return the error pointer for
  * those.  This is to permit this function to be called immediately
  * after clk_register().
+ *
+ * Return:
+ * pointer to the allocated struct clk_lookup on success, or error pointer
+ * otherwise.
  */
-int clk_register_clkdev(struct clk *clk, const char *con_id,
+struct clk_lookup *clk_register_clkdev(struct clk *clk, const char *con_id,
 	const char *dev_fmt, ...)
 {
 	struct clk_lookup *cl;
 	va_list ap;
 
 	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+		return ERR_CAST(clk);
 
 	va_start(ap, dev_fmt);
 	cl = vclkdev_alloc(clk, con_id, dev_fmt, ap);
 	va_end(ap);
 
 	if (!cl)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	clkdev_add(cl);
 
-	return 0;
+	return cl;
 }
 EXPORT_SYMBOL(clk_register_clkdev);

drivers/clk/samsung/clk-pll.c +8 -5

index 9d70e5c..f952e36 100644
@@ -927,6 +927,7 @@ struct clk * __init samsung_clk_register_pll2550x(const char *name,
 {
 	struct samsung_clk_pll2550x *pll;
 	struct clk *clk;
+	struct clk_lookup *cl;
 	struct clk_init_data init;
 
 	pll = kzalloc(sizeof(*pll), GFP_KERNEL);

@@ -952,7 +953,8 @@ struct clk * __init samsung_clk_register_pll2550x(const char *name,
 		kfree(pll);
 	}
 
-	if (clk_register_clkdev(clk, name, NULL))
+	cl = clk_register_clkdev(clk, name, NULL);
+	if (IS_ERR(cl))
 		pr_err("%s: failed to register lookup for %s", __func__, name);
 
 	return clk;

@@ -1161,8 +1163,9 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,
 {
 	struct samsung_clk_pll *pll;
 	struct clk *clk;
+	struct clk_lookup *cl;
 	struct clk_init_data init;
-	int ret, len;
+	int len;
 
 	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
 	if (!pll) {

@@ -1296,10 +1299,10 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,
 	if (!pll_clk->alias)
 		return;
 
-	ret = clk_register_clkdev(clk, pll_clk->alias, pll_clk->dev_name);
-	if (ret)
+	cl = clk_register_clkdev(clk, pll_clk->alias, pll_clk->dev_name);
+	if (IS_ERR(cl))
 		pr_err("%s: failed to register lookup for %s : %d",
-			__func__, pll_clk->name, ret);
+			__func__, pll_clk->name, PTR_ERR(cl));
 }
 
 void __init samsung_clk_register_pll(struct samsung_clk_provider *ctx,

drivers/clk/samsung/clk-s3c2410-dclk.c +10 -9

index f4f29ed6..1f9dd36 100644
@@ -238,6 +238,7 @@ static int s3c24xx_dclk_probe(struct platform_device *pdev)
 	struct s3c24xx_dclk *s3c24xx_dclk;
 	struct resource *mem;
 	struct clk **clk_table;
+	struct clk_lookup *cl;
 	struct s3c24xx_dclk_drv_data *dclk_variant;
 	int ret, i;

@@ -309,17 +310,17 @@ static int s3c24xx_dclk_probe(struct platform_device *pdev)
 			goto err_clk_register;
 		}
 
-	ret = clk_register_clkdev(clk_table[MUX_DCLK0], "dclk0", NULL);
-	if (!ret)
-		ret = clk_register_clkdev(clk_table[MUX_DCLK1], "dclk1", NULL);
-	if (!ret)
-		ret = clk_register_clkdev(clk_table[MUX_CLKOUT0],
+	cl = clk_register_clkdev(clk_table[MUX_DCLK0], "dclk0", NULL);
+	if (!IS_ERR(cl))
+		cl = clk_register_clkdev(clk_table[MUX_DCLK1], "dclk1", NULL);
+	if (!IS_ERR(cl))
+		cl = clk_register_clkdev(clk_table[MUX_CLKOUT0],
 					  "clkout0", NULL);
-	if (!ret)
-		ret = clk_register_clkdev(clk_table[MUX_CLKOUT1],
+	if (!IS_ERR(cl))
+		cl = clk_register_clkdev(clk_table[MUX_CLKOUT1],
 					  "clkout1", NULL);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to register aliases, %d\n", ret);
+	if (IS_ERR(cl)) {
+		dev_err(&pdev->dev, "failed to register aliases, %d\n", PTR_ERR(cl));
 		goto err_clk_register;
 	}

drivers/clk/samsung/clk.c +20 ~~-15~~

index 4bda540..a08e6f0 100644
@@ -102,7 +102,8 @@ void __init samsung_clk_register_alias(struct samsung_clk_provider *ctx,
 				unsigned int nr_clk)
 {
 	struct clk *clk;
-	unsigned int idx, ret;
+	struct clk_lookup *cl;
+	unsigned int idx;
 
 	if (!ctx->clk_data.clks) {
 		pr_err("%s: clock table missing\n", __func__);

@@ -123,8 +124,8 @@ void __init samsung_clk_register_alias(struct samsung_clk_provider *ctx,
 			continue;
 		}
 
-		ret = clk_register_clkdev(clk, list->alias, list->dev_name);
-		if (ret)
+		cl = clk_register_clkdev(clk, list->alias, list->dev_name);
+		if (IS_ERR(cl))
 			pr_err("%s: failed to register lookup %s\n",
 					__func__, list->alias);
 	}

@@ -135,7 +136,8 @@ void __init samsung_clk_register_fixed_rate(struct samsung_clk_provider *ctx,
 		struct samsung_fixed_rate_clock *list, unsigned int nr_clk)
 {
 	struct clk *clk;
-	unsigned int idx, ret;
+	struct clk_lookup *cl;
+	unsigned int idx;
 
 	for (idx = 0; idx < nr_clk; idx++, list++) {
 		clk = clk_register_fixed_rate(NULL, list->name,

@@ -152,8 +154,8 @@ void __init samsung_clk_register_fixed_rate(struct samsung_clk_provider *ctx,
 		 * Unconditionally add a clock lookup for the fixed rate clocks.
 		 * There are not many of these on any of Samsung platforms.
 		 */
-		ret = clk_register_clkdev(clk, list->name, NULL);
-		if (ret)
+		cl = clk_register_clkdev(clk, list->name, NULL);
+		if (IS_ERR(cl))
 			pr_err("%s: failed to register clock lookup for %s",
 				__func__, list->name);
 	}

@@ -185,7 +187,8 @@ void __init samsung_clk_register_mux(struct samsung_clk_provider *ctx,
 				unsigned int nr_clk)
 {
 	struct clk *clk;
-	unsigned int idx, ret;
+	struct clk_lookup *cl;
+	unsigned int idx;
 
 	for (idx = 0; idx < nr_clk; idx++, list++) {
 		clk = clk_register_mux(NULL, list->name, list->parent_names,

@@ -202,9 +205,9 @@ void __init samsung_clk_register_mux(struct samsung_clk_provider *ctx,
 
 		/* register a clock lookup only if a clock alias is specified */
 		if (list->alias) {
-			ret = clk_register_clkdev(clk, list->alias,
+			cl = clk_register_clkdev(clk, list->alias,
 						list->dev_name);
-			if (ret)
+			if (IS_ERR(cl))
 				pr_err("%s: failed to register lookup %s\n",
 						__func__, list->alias);
 		}

@@ -217,7 +220,8 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
 				unsigned int nr_clk)
 {
 	struct clk *clk;
-	unsigned int idx, ret;
+	struct clk_lookup *cl;
+	unsigned int idx;
 
 	for (idx = 0; idx < nr_clk; idx++, list++) {
 		if (list->table)

@@ -241,9 +245,9 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
 
 		/* register a clock lookup only if a clock alias is specified */
 		if (list->alias) {
-			ret = clk_register_clkdev(clk, list->alias,
+			cl = clk_register_clkdev(clk, list->alias,
 						list->dev_name);
-			if (ret)
+			if (IS_ERR(cl))
 				pr_err("%s: failed to register lookup %s\n",
 						__func__, list->alias);
 		}

@@ -256,7 +260,8 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
 				unsigned int nr_clk)
 {
 	struct clk *clk;
-	unsigned int idx, ret;
+	struct clk_lookup *cl;
+	unsigned int idx;
 
 	for (idx = 0; idx < nr_clk; idx++, list++) {
 		clk = clk_register_gate(NULL, list->name, list->parent_name,

@@ -270,9 +275,9 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
 
 		/* register a clock lookup only if a clock alias is specified */
 		if (list->alias) {
-			ret = clk_register_clkdev(clk, list->alias,
+			cl = clk_register_clkdev(clk, list->alias,
 							list->dev_name);
-			if (ret)
+			if (IS_ERR(cl))
 				pr_err("%s: failed to register lookup %s\n",
 					__func__, list->alias);
 		}

include/linux/clkdev.h +1 -1

index 94bad77..87ace2c 100644
@@ -40,7 +40,7 @@ void clkdev_drop(struct clk_lookup *cl);
 void clkdev_add_table(struct clk_lookup *, size_t);
 int clk_add_alias(const char *, const char *, char *, struct device *);
 
-int clk_register_clkdev(struct clk *, const char *, const char *, ...);
+struct clk_lookup *clk_register_clkdev(struct clk *, const char *, const char *, ...);
 int clk_register_clkdevs(struct clk *, struct clk_lookup *, size_t);
 
 #ifdef CONFIG_COMMON_CLK

Signature · 1 line

2.1.4

Mika Westerberg

Reviewed-by on v1 1/1

2015-02-04 15:51 UTC #2 lore raw

Re: [PATCH v1] clkdev: change prototype of clk_register_clkdev()

Quoted 6 lines · Andy Shevchenko

Since clk_register_clkdev() is exported for modules the caller should get a pointer to the allocated resources. Otherwise the memory leak is guaranteed on the ->remove() stage.
Cc Tomeu Vizoso <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>

One comment, see below. Nothing major so feel free to add,

Reviewed-by Mika Westerberg <[email protected]>

Quoted 13 lines

---
 arch/arm/mach-msm/clock-pcom.c         |  9 +++++----
 arch/arm/mach-vexpress/spc.c           |  5 ++++-
 arch/mips/ath79/clock.c                |  6 +++---
 drivers/clk/clk-bcm2835.c              | 12 +++++++-----
 drivers/clk/clk-max-gen.c              |  9 ++++-----
 drivers/clk/clk-xgene.c                |  6 +++---
 drivers/clk/clkdev.c                   | 14 +++++++++-----
 drivers/clk/samsung/clk-pll.c          | 13 ++++++++-----
 drivers/clk/samsung/clk-s3c2410-dclk.c | 19 +++++++++---------
 drivers/clk/samsung/clk.c              | 35 +++++++++++++++++++---------------
 include/linux/clkdev.h                 |  2 +-
 11 files changed, 74 insertions(+), 56 deletions(-)

...

Quoted 5 lines

index 6e5c504..e07b1e2 100644
drivers/clk/clkdev.c +0 -0
@@ -307,29 +307,33 @@ EXPORT_SYMBOL(clkdev_drop);
  * clkdev.

drivers/clk/clkdev.c EXPORT_SYMBOL(clkdev_drop); · line 316

Quoted 10 lines

drivers/clk/clkdev.c +5 -1

  *
  * To make things easier for mass registration, we detect error clks
- * from a previous clk_register() call, and return the error code for
+ * from a previous clk_register() call, and return the error pointer for
  * those.  This is to permit this function to be called immediately
  * after clk_register().
+ *
+ * Return:
+ * pointer to the allocated struct clk_lookup on success, or error pointer
+ * otherwise.

This should probably say how these resources are supposed to be released.

Quoted 26 lines

drivers/clk/clkdev.c +4 -4

  */
-int clk_register_clkdev(struct clk *clk, const char *con_id,
+struct clk_lookup *clk_register_clkdev(struct clk *clk, const char *con_id,
 	const char *dev_fmt, ...)
 {
 	struct clk_lookup *cl;
 	va_list ap;
 
 	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+		return ERR_CAST(clk);
 
 	va_start(ap, dev_fmt);
 	cl = vclkdev_alloc(clk, con_id, dev_fmt, ap);
 	va_end(ap);
 
 	if (!cl)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	clkdev_add(cl);
 
-	return 0;
+	return cl;
 }
 EXPORT_SYMBOL(clk_register_clkdev);

Andy Shevchenko

author

2015-02-04 16:04 UTC #3 lore raw

Re: [PATCH v1] clkdev: change prototype of clk_register_clkdev()

Quoted 11 lines · Mika Westerberg

Quoted 6 lines · Andy Shevchenko
Since clk_register_clkdev() is exported for modules the caller should get a pointer to the allocated resources. Otherwise the memory leak is guaranteed on the ->remove() stage.
Cc Tomeu Vizoso <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
One comment, see below. Nothing major so feel free to add,
Reviewed-by Mika Westerberg <[email protected]>

Thanks for review!

Though still wait for others to comment and test if possible.

By the way, I have to add that we have one unpublished (yet) user of this. So, we are testing this internally on x86. Just to note that the user is a module which might be unloaded. That's why important to release the acquired resources.

Quoted 27 lines

Quoted 14 lines

---
 arch/arm/mach-msm/clock-pcom.c         |  9 +++++----
 arch/arm/mach-vexpress/spc.c           |  5 ++++-
 arch/mips/ath79/clock.c                |  6 +++---
 drivers/clk/clk-bcm2835.c              | 12 +++++++-----
 drivers/clk/clk-max-gen.c              |  9 ++++-----
 drivers/clk/clk-xgene.c                |  6 +++---
 drivers/clk/clkdev.c                   | 14 +++++++++-----
 drivers/clk/samsung/clk-pll.c          | 13 ++++++++-----
 drivers/clk/samsung/clk-s3c2410-dclk.c | 19 +++++++++---------
 drivers/clk/samsung/clk.c              | 35 +++++++++++++++++++---------------
 include/linux/clkdev.h                 |  2 +-
 11 files changed, 74 insertions(+), 56 deletions(-)

...

Quoted 10 lines

index 6e5c504..e07b1e2 100644

drivers/clk/clkdev.c +1 -1

@@ -307,29 +307,33 @@ EXPORT_SYMBOL(clkdev_drop);
  * clkdev.
  *
  * To make things easier for mass registration, we detect error clks
- * from a previous clk_register() call, and return the error code for
+ * from a previous clk_register() call, and return the error pointer for
  * those.  This is to permit this function to be called immediately

Quoted 8 lines

drivers/clk/clkdev.c EXPORT_SYMBOL(clkdev_drop); · line 316
Quoted 5 lines
drivers/clk/clkdev.c +4 -0
  * after clk_register().
+ *
+ * Return:
+ * pointer to the allocated struct clk_lookup on success, or error pointer
+ * otherwise.
This should probably say how these resources are supposed to be released.

Agree. I would add this in v2.

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy

Stephen Boyd

2015-02-04 23:18 UTC #4 lore raw

Re: [PATCH v1] clkdev: change prototype of clk_register_clkdev()

In reply to Andy Shevchenko

Quoted 6 lines · Andy Shevchenko

Since clk_register_clkdev() is exported for modules the caller should get a pointer to the allocated resources. Otherwise the memory leak is guaranteed on the ->remove() stage.
Cc Tomeu Vizoso <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>

The idea seems ok to me. Please Cc Russell on the next version. It would also be good to make a note that this patch doesn't attempt to fix any memory leaks that may exist by adding calls to clkdev_drop().

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project