[PATCH v2 0/3] clk: fsl-sai: fix memory leak

[PATCH v2 0/3] clk: fsl-sai: fix memory leak

[Michael Walle]

Patch 1 will fix the memory leak and can be picked up by the stable queues.
Patch 2-3 will add and use the devm_ variant to simplify the handling.

Please note that the new devm_ isn't exported as the original one isn't
exported either. I'm planning to post another patch which will add module
support to the clk-fsl-sai driver and this will then export the function.

Michael Walle (3):
  clk: fsl-sai: fix memory leak
  clk: composite: add devm_clk_hw_register_composite_pdata()
  clk: fsl-sai: use devm_clk_hw_register_composite_pdata()

 drivers/clk/clk-composite.c  | 50 ++++++++++++++++++++++++++++++++++++
 drivers/clk/clk-fsl-sai.c    | 14 +++++-----
 include/linux/clk-provider.h |  7 +++++
 3 files changed, 64 insertions(+), 7 deletions(-)

-- 
2.20.1

[PATCH v2 1/3] clk: fsl-sai: fix memory leak

[Michael Walle]

If the device is removed we don't unregister the composite clock. Fix
that.

Fixes: 9cd10205227c ("clk: fsl-sai: new driver")
Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v1:
 - none

 drivers/clk/clk-fsl-sai.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
index 0221180a4dd7..1e81c8d8a6fd 100644
--- a/drivers/clk/clk-fsl-sai.c
+++ b/drivers/clk/clk-fsl-sai.c
@@ -68,9 +68,20 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 
+	platform_set_drvdata(pdev, hw);
+
 	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
 }
 
+static int fsl_sai_clk_remove(struct platform_device *pdev)
+{
+	struct clk_hw *hw = platform_get_drvdata(pdev);
+
+	clk_hw_unregister_composite(hw);
+
+	return 0;
+}
+
 static const struct of_device_id of_fsl_sai_clk_ids[] = {
 	{ .compatible = "fsl,vf610-sai-clock" },
 	{ }
@@ -79,6 +90,7 @@ MODULE_DEVICE_TABLE(of, of_fsl_sai_clk_ids);
 
 static struct platform_driver fsl_sai_clk_driver = {
 	.probe = fsl_sai_clk_probe,
+	.remove = fsl_sai_clk_remove,
 	.driver		= {
 		.name	= "fsl-sai-clk",
 		.of_match_table = of_fsl_sai_clk_ids,
-- 
2.20.1

Re: [PATCH v2 1/3] clk: fsl-sai: fix memory leak

[Stephen Boyd]

Quoting Michael Walle (2020-11-05 11:27:44)
> If the device is removed we don't unregister the composite clock. Fix
> that.
> 
> Fixes: 9cd10205227c ("clk: fsl-sai: new driver")
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

Applied to clk-next

[PATCH v2 2/3] clk: composite: add devm_clk_hw_register_composite_pdata()

[Michael Walle]

This will simplify drivers which would only unregister the clk in their
remove() op.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v1:
 - new patch

 drivers/clk/clk-composite.c  | 50 ++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |  7 +++++
 2 files changed, 57 insertions(+)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 2ddb54f7d3ab..0506046a5f4b 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/device.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 
@@ -405,3 +406,52 @@ void clk_hw_unregister_composite(struct clk_hw *hw)
 	kfree(composite);
 }
 EXPORT_SYMBOL_GPL(clk_hw_unregister_composite);
+
+static void devm_clk_hw_release_composite(struct device *dev, void *res)
+{
+	clk_hw_unregister_composite(*(struct clk_hw **)res);
+}
+
+static struct clk_hw *__devm_clk_hw_register_composite(struct device *dev,
+			const char *name, const char * const *parent_names,
+			const struct clk_parent_data *pdata, int num_parents,
+			struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+			struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+			struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+			unsigned long flags)
+{
+	struct clk_hw **ptr, *hw;
+
+	ptr = devres_alloc(devm_clk_hw_release_composite, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	hw = __clk_hw_register_composite(dev, name, parent_names, pdata,
+					 num_parents, mux_hw, mux_ops, rate_hw,
+					 rate_ops, gate_hw, gate_ops, flags);
+
+	if (!IS_ERR(hw)) {
+		*ptr = hw;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return hw;
+}
+
+struct clk_hw *devm_clk_hw_register_composite_pdata(struct device *dev,
+			const char *name,
+			const struct clk_parent_data *parent_data,
+			int num_parents,
+			struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+			struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+			struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+			unsigned long flags)
+{
+	return __devm_clk_hw_register_composite(dev, name, NULL, parent_data,
+						num_parents, mux_hw, mux_ops,
+						rate_hw, rate_ops, gate_hw,
+						gate_ops, flags);
+}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 9ead4633c8c8..5f896df01f83 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -1089,6 +1089,13 @@ struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
 		struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
 		struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
 		unsigned long flags);
+struct clk_hw *devm_clk_hw_register_composite_pdata(struct device *dev,
+		const char *name, const struct clk_parent_data *parent_data,
+		int num_parents,
+		struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+		struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+		struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+		unsigned long flags);
 void clk_hw_unregister_composite(struct clk_hw *hw);
 
 struct clk *clk_register(struct device *dev, struct clk_hw *hw);
-- 
2.20.1

Re: [PATCH v2 2/3] clk: composite: add devm_clk_hw_register_composite_pdata()

[Stephen Boyd]

Quoting Michael Walle (2020-11-05 11:27:45)
> This will simplify drivers which would only unregister the clk in their
> remove() op.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

Applied to clk-next

[PATCH v2 3/3] clk: fsl-sai: use devm_clk_hw_register_composite_pdata()

[Michael Walle]

Simplify the driver by using that helper and drop the remove() function.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Changes since v1:
 - new patch

 drivers/clk/clk-fsl-sai.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
index 1e81c8d8a6fd..6238fcea0467 100644
--- a/drivers/clk/clk-fsl-sai.c
+++ b/drivers/clk/clk-fsl-sai.c
@@ -58,30 +58,19 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
 	/* set clock direction, we are the BCLK master */
 	writel(CR2_BCD, base + I2S_CR2);
 
-	hw = clk_hw_register_composite_pdata(dev, dev->of_node->name,
-					     &pdata, 1, NULL, NULL,
-					     &sai_clk->div.hw,
-					     &clk_divider_ops,
-					     &sai_clk->gate.hw,
-					     &clk_gate_ops,
-					     CLK_SET_RATE_GATE);
+	hw = devm_clk_hw_register_composite_pdata(dev, dev->of_node->name,
+						  &pdata, 1, NULL, NULL,
+						  &sai_clk->div.hw,
+						  &clk_divider_ops,
+						  &sai_clk->gate.hw,
+						  &clk_gate_ops,
+						  CLK_SET_RATE_GATE);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 
-	platform_set_drvdata(pdev, hw);
-
 	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
 }
 
-static int fsl_sai_clk_remove(struct platform_device *pdev)
-{
-	struct clk_hw *hw = platform_get_drvdata(pdev);
-
-	clk_hw_unregister_composite(hw);
-
-	return 0;
-}
-
 static const struct of_device_id of_fsl_sai_clk_ids[] = {
 	{ .compatible = "fsl,vf610-sai-clock" },
 	{ }
@@ -90,7 +79,6 @@ MODULE_DEVICE_TABLE(of, of_fsl_sai_clk_ids);
 
 static struct platform_driver fsl_sai_clk_driver = {
 	.probe = fsl_sai_clk_probe,
-	.remove = fsl_sai_clk_remove,
 	.driver		= {
 		.name	= "fsl-sai-clk",
 		.of_match_table = of_fsl_sai_clk_ids,
-- 
2.20.1

Re: [PATCH v2 3/3] clk: fsl-sai: use devm_clk_hw_register_composite_pdata()

[Stephen Boyd]

Quoting Michael Walle (2020-11-05 11:27:46)
> Simplify the driver by using that helper and drop the remove() function.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

Applied to clk-next