linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/3] clk: clkdev add managed lookup registrations
@ 2018-12-07 11:09 Matti Vaittinen
  2018-12-07 11:09 ` [PATCH v7 1/3] clkdev: add managed clkdev lookup registration Matti Vaittinen
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Matti Vaittinen @ 2018-12-07 11:09 UTC (permalink / raw)
  To: matti.vaittinen, mazziesaccount, corbet, cw00.choi, krzk,
	b.zolnierkie, mturquette, sboyd, linux, pombredanne, sre, vkoul,
	linux, pavel, sjhuang, andrew.smirnov, djkurtz, akshu.agrawal,
	rafael.j.wysocki, linux-doc, linux-kernel, linux-clk,
	linux-arm-kernel

Series adds managed clkdev lookup interfaces and cleans few drivers

Few clk drivers appear to be leaking clkdev lookup registrations at
driver remove. The patch series adds devm versions of lookup
registrations and cleans up few drivers. Driver clean-up patches have
not been tested as I lack the HW. All testing and comments if
driver/device removal is even possible for changed drivers is highly
appreciated. If removal is not possible I will gladly drop the patches
from series - although leaking lookups may serve as bad example for new
developers =)

Changed drivers are:
clk-max77686 and clk-st

Please note that the patch #2 requires this change to work correctly:
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570

Changelog v7:
- rewmoved already applied of_provider patches and
  now independent bd718x7 patch from the series.
- No functional changes.

Changelog v6
- Drop 'devm_of_clk_add_parent_hw_provider'. Change
  'devm_of_clk_add_hw_provider' to look the parent device node for
  provider information if device's own node does not contain
  #clock-cells - property.
- Add kerneldoc in own patch.
- Remove NULL checks from devres match function for clkdev releasing
- Clean styling issues from clkdev.c

Changelog v5
- Split v4 patch 1. Place clkdev stuff to patch 1 and clk provider to patch 2
- Remove devm_clk_release_clkdev from devres.txt
- Added kerneldoc for managed provider registrations.
- Cleaned styling issues.

Changelog v4
- Add support for ROHM bd718x7 PMIC clock gate. Included in this patch
  series because it depends on managed interfaces added in patch 1.

Changelog v3
Address issues spotted by Krzysztof Kozlowski
- Drop patch 3 for clk-s3c2410-dclk as this device can never be removed
- Fix indentiation for clk-max77686
- Rest  of the patches are unchanged.

Changelog v2
Issue spotted by 0-Day test suite
- Add a stub function 'devm_of_clk_add_parent_hw_provider' for no OF config.
- patches 2-8 are unchanged.

This patch series is based on clk-next

---

Matti Vaittinen (3):
  clkdev: add managed clkdev lookup registration
  clk: clk-max77686: Clean clkdev lookup leak and use devm
  clk: clk-st: avoid clkdev lookup leak at remove

 Documentation/driver-model/devres.txt |   1 +
 drivers/clk/clk-max77686.c            |  28 ++-------
 drivers/clk/clkdev.c                  | 111 +++++++++++++++++++++++++++-------
 drivers/clk/x86/clk-st.c              |   3 +-
 include/linux/clkdev.h                |   4 ++
 5 files changed, 101 insertions(+), 46 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v7 1/3] clkdev: add managed clkdev lookup registration
  2018-12-07 11:09 [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
@ 2018-12-07 11:09 ` Matti Vaittinen
  2019-02-06 19:05   ` Stephen Boyd
  2018-12-07 11:10 ` [PATCH v7 2/3] clk: clk-max77686: Clean clkdev lookup leak and use devm Matti Vaittinen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Matti Vaittinen @ 2018-12-07 11:09 UTC (permalink / raw)
  To: matti.vaittinen, mazziesaccount, corbet, cw00.choi, krzk,
	b.zolnierkie, mturquette, sboyd, linux, pombredanne, sre, vkoul,
	linux, pavel, sjhuang, andrew.smirnov, djkurtz, akshu.agrawal,
	rafael.j.wysocki, linux-doc, linux-kernel, linux-clk,
	linux-arm-kernel

Clkdev registration lacks of managed registration functions and it
seems few drivers do not drop clkdev lookups at exit. Add
devm_clk_hw_register_clkdev and devm_clk_release_clkdev to ease lookup
releasing at exit.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 Documentation/driver-model/devres.txt |   1 +
 drivers/clk/clkdev.c                  | 111 +++++++++++++++++++++++++++-------
 include/linux/clkdev.h                |   4 ++
 3 files changed, 93 insertions(+), 23 deletions(-)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 43681ca0837f..dbf14326243b 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -238,6 +238,7 @@ CLOCK
   devm_clk_put()
   devm_clk_hw_register()
   devm_of_clk_add_hw_provider()
+  devm_clk_hw_register_clkdev()
 
 DMA
   dmaenginem_async_device_register()
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 9ab3db8b3988..4621f8a91fc0 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -401,6 +401,23 @@ static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
 	return cl;
 }
 
+static int do_clk_register_clkdev(struct clk_hw *hw,
+	struct clk_lookup **cl, const char *con_id, const char *dev_id)
+{
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	/*
+	 * Since dev_id can be NULL, and NULL is handled specially, we must
+	 * pass it as either a NULL format string, or with "%s".
+	 */
+	if (dev_id)
+		*cl = __clk_register_clkdev(hw, con_id, "%s", dev_id);
+	else
+		*cl = __clk_register_clkdev(hw, con_id, NULL);
+
+	return *cl ? 0 : -ENOMEM;
+}
+
 /**
  * clk_register_clkdev - register one clock lookup for a struct clk
  * @clk: struct clk to associate with all clk_lookups
@@ -423,17 +440,8 @@ int clk_register_clkdev(struct clk *clk, const char *con_id,
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
-	/*
-	 * Since dev_id can be NULL, and NULL is handled specially, we must
-	 * pass it as either a NULL format string, or with "%s".
-	 */
-	if (dev_id)
-		cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, "%s",
-					   dev_id);
-	else
-		cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, NULL);
-
-	return cl ? 0 : -ENOMEM;
+	return do_clk_register_clkdev(__clk_get_hw(clk), &cl, con_id,
+					      dev_id);
 }
 EXPORT_SYMBOL(clk_register_clkdev);
 
@@ -456,18 +464,75 @@ int clk_hw_register_clkdev(struct clk_hw *hw, const char *con_id,
 {
 	struct clk_lookup *cl;
 
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	return do_clk_register_clkdev(hw, &cl, con_id, dev_id);
+}
+EXPORT_SYMBOL(clk_hw_register_clkdev);
 
-	/*
-	 * Since dev_id can be NULL, and NULL is handled specially, we must
-	 * pass it as either a NULL format string, or with "%s".
-	 */
-	if (dev_id)
-		cl = __clk_register_clkdev(hw, con_id, "%s", dev_id);
-	else
-		cl = __clk_register_clkdev(hw, con_id, NULL);
+static void devm_clkdev_release(struct device *dev, void *res)
+{
+	clkdev_drop(*(struct clk_lookup **)res);
+}
 
-	return cl ? 0 : -ENOMEM;
+static int devm_clk_match_clkdev(struct device *dev, void *res, void *data)
+{
+	struct clk_lookup **l = res;
+
+	return *l == data;
 }
-EXPORT_SYMBOL(clk_hw_register_clkdev);
+
+/**
+ * devm_clk_release_clkdev - Resource managed clkdev lookup release
+ * @dev: device this lookup is bound
+ * @con_id: connection ID string on device
+ * @dev_id: format string describing device name
+ *
+ * Drop the clkdev lookup created with devm_clk_hw_register_clkdev.
+ * Normally this function will not need to be called and the resource
+ * management code will ensure that the resource is freed.
+ */
+void devm_clk_release_clkdev(struct device *dev, const char *con_id,
+			     const char *dev_id)
+{
+	struct clk_lookup *cl;
+	int rval;
+
+	cl = clk_find(dev_id, con_id);
+	WARN_ON(!cl);
+	rval = devres_release(dev, devm_clkdev_release,
+			      devm_clk_match_clkdev, cl);
+	WARN_ON(rval);
+}
+EXPORT_SYMBOL(devm_clk_release_clkdev);
+
+/**
+ * devm_clk_hw_register_clkdev - managed clk lookup registration for clk_hw
+ * @dev: device this lookup is bound
+ * @hw: struct clk_hw to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_id: format string describing device name
+ *
+ * con_id or dev_id may be NULL as a wildcard, just as in the rest of
+ * clkdev.
+ *
+ * To make things easier for mass registration, we detect error clk_hws
+ * from a previous clk_hw_register_*() call, and return the error code for
+ * those.  This is to permit this function to be called immediately
+ * after clk_hw_register_*().
+ */
+int devm_clk_hw_register_clkdev(struct device *dev, struct clk_hw *hw,
+				const char *con_id, const char *dev_id)
+{
+	int rval = -ENOMEM;
+	struct clk_lookup **cl;
+
+	cl = devres_alloc(devm_clkdev_release, sizeof(*cl), GFP_KERNEL);
+	if (cl) {
+		rval = do_clk_register_clkdev(hw, cl, con_id, dev_id);
+		if (!rval)
+			devres_add(dev, cl);
+		else
+			devres_free(cl);
+	}
+	return rval;
+}
+EXPORT_SYMBOL(devm_clk_hw_register_clkdev);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index 4890ff033220..ccb32af5848b 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -52,4 +52,8 @@ int clk_add_alias(const char *, const char *, const char *, struct device *);
 int clk_register_clkdev(struct clk *, const char *, const char *);
 int clk_hw_register_clkdev(struct clk_hw *, const char *, const char *);
 
+int devm_clk_hw_register_clkdev(struct device *dev, struct clk_hw *hw,
+				const char *con_id, const char *dev_id);
+void devm_clk_release_clkdev(struct device *dev, const char *con_id,
+			     const char *dev_id);
 #endif
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 2/3] clk: clk-max77686: Clean clkdev lookup leak and use devm
  2018-12-07 11:09 [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
  2018-12-07 11:09 ` [PATCH v7 1/3] clkdev: add managed clkdev lookup registration Matti Vaittinen
@ 2018-12-07 11:10 ` Matti Vaittinen
  2019-02-06 19:05   ` Stephen Boyd
  2018-12-07 11:10 ` [PATCH v7 3/3] clk: clk-st: avoid clkdev lookup leak at remove Matti Vaittinen
  2019-01-31 13:24 ` [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
  3 siblings, 1 reply; 11+ messages in thread
From: Matti Vaittinen @ 2018-12-07 11:10 UTC (permalink / raw)
  To: matti.vaittinen, mazziesaccount, corbet, cw00.choi, krzk,
	b.zolnierkie, mturquette, sboyd, linux, pombredanne, sre, vkoul,
	linux, pavel, sjhuang, andrew.smirnov, djkurtz, akshu.agrawal,
	rafael.j.wysocki, linux-doc, linux-kernel, linux-clk,
	linux-arm-kernel

clk-max77686 never clean clkdev lookup at remove. This can cause
oops if clk-max77686 is removed and inserted again. Fix leak by
using new devm clkdev lookup registration. Simplify also error
path by using new devm_of_clk_add_hw_provider.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Please note that the patch #2 requires this change to work correctly:
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570

 drivers/clk/clk-max77686.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 22c937644c93..3727d5472450 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -235,8 +235,9 @@ static int max77686_clk_probe(struct platform_device *pdev)
 			return ret;
 		}
 
-		ret = clk_hw_register_clkdev(&max_clk_data->hw,
-					     max_clk_data->clk_idata.name, NULL);
+		ret = devm_clk_hw_register_clkdev(dev, &max_clk_data->hw,
+						  max_clk_data->clk_idata.name,
+						  NULL);
 		if (ret < 0) {
 			dev_err(dev, "Failed to clkdev register: %d\n", ret);
 			return ret;
@@ -244,8 +245,8 @@ static int max77686_clk_probe(struct platform_device *pdev)
 	}
 
 	if (parent->of_node) {
-		ret = of_clk_add_hw_provider(parent->of_node, of_clk_max77686_get,
-					     drv_data);
+		ret = devm_of_clk_add_hw_provider(dev, of_clk_max77686_get,
+						  drv_data);
 
 		if (ret < 0) {
 			dev_err(dev, "Failed to register OF clock provider: %d\n",
@@ -261,27 +262,11 @@ static int max77686_clk_probe(struct platform_device *pdev)
 					 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT);
 		if (ret < 0) {
 			dev_err(dev, "Failed to config low-jitter: %d\n", ret);
-			goto remove_of_clk_provider;
+			return ret;
 		}
 	}
 
 	return 0;
-
-remove_of_clk_provider:
-	if (parent->of_node)
-		of_clk_del_provider(parent->of_node);
-
-	return ret;
-}
-
-static int max77686_clk_remove(struct platform_device *pdev)
-{
-	struct device *parent = pdev->dev.parent;
-
-	if (parent->of_node)
-		of_clk_del_provider(parent->of_node);
-
-	return 0;
 }
 
 static const struct platform_device_id max77686_clk_id[] = {
@@ -297,7 +282,6 @@ static struct platform_driver max77686_clk_driver = {
 		.name  = "max77686-clk",
 	},
 	.probe = max77686_clk_probe,
-	.remove = max77686_clk_remove,
 	.id_table = max77686_clk_id,
 };
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 3/3] clk: clk-st: avoid clkdev lookup leak at remove
  2018-12-07 11:09 [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
  2018-12-07 11:09 ` [PATCH v7 1/3] clkdev: add managed clkdev lookup registration Matti Vaittinen
  2018-12-07 11:10 ` [PATCH v7 2/3] clk: clk-max77686: Clean clkdev lookup leak and use devm Matti Vaittinen
@ 2018-12-07 11:10 ` Matti Vaittinen
  2019-02-06 19:05   ` Stephen Boyd
  2019-01-31 13:24 ` [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
  3 siblings, 1 reply; 11+ messages in thread
From: Matti Vaittinen @ 2018-12-07 11:10 UTC (permalink / raw)
  To: matti.vaittinen, mazziesaccount, corbet, cw00.choi, krzk,
	b.zolnierkie, mturquette, sboyd, linux, pombredanne, sre, vkoul,
	linux, pavel, sjhuang, andrew.smirnov, djkurtz, akshu.agrawal,
	rafael.j.wysocki, linux-doc, linux-kernel, linux-clk,
	linux-arm-kernel

Use devm based clkdev lookup registration to avoid leaking lookup
structures.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/clk/x86/clk-st.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/x86/clk-st.c b/drivers/clk/x86/clk-st.c
index 3a0996f2d556..25d4b97aff9b 100644
--- a/drivers/clk/x86/clk-st.c
+++ b/drivers/clk/x86/clk-st.c
@@ -52,7 +52,8 @@ static int st_clk_probe(struct platform_device *pdev)
 		0, st_data->base + MISCCLKCNTL1, OSCCLKENB,
 		CLK_GATE_SET_TO_DISABLE, NULL);
 
-	clk_hw_register_clkdev(hws[ST_CLK_GATE], "oscout1", NULL);
+	devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], "oscout1",
+				    NULL);
 
 	return 0;
 }
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 0/3] clk: clkdev add managed lookup registrations
  2018-12-07 11:09 [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
                   ` (2 preceding siblings ...)
  2018-12-07 11:10 ` [PATCH v7 3/3] clk: clk-st: avoid clkdev lookup leak at remove Matti Vaittinen
@ 2019-01-31 13:24 ` Matti Vaittinen
  2019-01-31 15:21   ` Russell King - ARM Linux admin
  3 siblings, 1 reply; 11+ messages in thread
From: Matti Vaittinen @ 2019-01-31 13:24 UTC (permalink / raw)
  To: mazziesaccount, corbet, cw00.choi, krzk, b.zolnierkie,
	mturquette, sboyd, linux, pombredanne, sre, vkoul, linux, pavel,
	sjhuang, andrew.smirnov, djkurtz, akshu.agrawal,
	rafael.j.wysocki, linux-doc, linux-kernel, linux-clk,
	linux-arm-kernel

Hello All,

On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> Series adds managed clkdev lookup interfaces and cleans few drivers
> 
> Few clk drivers appear to be leaking clkdev lookup registrations at
> driver remove. The patch series adds devm versions of lookup
> registrations and cleans up few drivers. Driver clean-up patches have
> not been tested as I lack the HW. All testing and comments if
> driver/device removal is even possible for changed drivers is highly
> appreciated. If removal is not possible I will gladly drop the patches
> from series - although leaking lookups may serve as bad example for new
> developers =)
> 
> Changed drivers are:
> clk-max77686 and clk-st
> 
> Please note that the patch #2 requires this change to work correctly:
> https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570

I guess the dependency mentioned abowe is already in (most) of the
trees. (I can't say for sure as I don't know what is the correct tree
for clkdev - is it linux-arm.git as Russel is maintaining clkdev? If
yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
"clk: of-provider: look at parent if registered device has no
provider info" seems to be sitting in maser branch).

So should I rebase this series to some other tree and resend? Or is this
something that is not wanted?

Br,
	Matti Vaittinen

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 0/3] clk: clkdev add managed lookup registrations
  2019-01-31 13:24 ` [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
@ 2019-01-31 15:21   ` Russell King - ARM Linux admin
  2019-01-31 19:38     ` Stephen Boyd
  0 siblings, 1 reply; 11+ messages in thread
From: Russell King - ARM Linux admin @ 2019-01-31 15:21 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: mazziesaccount, corbet, cw00.choi, krzk, b.zolnierkie,
	mturquette, sboyd, pombredanne, sre, vkoul, linux, pavel,
	sjhuang, andrew.smirnov, djkurtz, akshu.agrawal,
	rafael.j.wysocki, linux-doc, linux-kernel, linux-clk,
	linux-arm-kernel

On Thu, Jan 31, 2019 at 03:24:52PM +0200, Matti Vaittinen wrote:
> Hello All,
> 
> On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> > Series adds managed clkdev lookup interfaces and cleans few drivers
> > 
> > Few clk drivers appear to be leaking clkdev lookup registrations at
> > driver remove. The patch series adds devm versions of lookup
> > registrations and cleans up few drivers. Driver clean-up patches have
> > not been tested as I lack the HW. All testing and comments if
> > driver/device removal is even possible for changed drivers is highly
> > appreciated. If removal is not possible I will gladly drop the patches
> > from series - although leaking lookups may serve as bad example for new
> > developers =)
> > 
> > Changed drivers are:
> > clk-max77686 and clk-st
> > 
> > Please note that the patch #2 requires this change to work correctly:
> > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> 
> I guess the dependency mentioned abowe is already in (most) of the
> trees. (I can't say for sure as I don't know what is the correct tree
> for clkdev - is it linux-arm.git as Russel is maintaining clkdev?

Yes, I'm supposed to be maintaining clkdev, but I'm busy with other
stuff (such as reorganising my network, helping people with SFP issues,
I'm supposed to be replying to Arend over a brcmfmac issue that has been
on-going since Christmas which I haven't yet been able to doing the next
test...)

> If yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> "clk: of-provider: look at parent if registered device has no
> provider info" seems to be sitting in maser branch).

It's there because it's part of the mainline kernel and has been
since v5.0-rc1.  So, if you depend on that commit, basing off
v5.0-rc1 is probably sane, unless there's something else that
conflicts.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 0/3] clk: clkdev add managed lookup registrations
  2019-01-31 15:21   ` Russell King - ARM Linux admin
@ 2019-01-31 19:38     ` Stephen Boyd
  2019-02-01  8:40       ` Matti Vaittinen
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2019-01-31 19:38 UTC (permalink / raw)
  To: Matti Vaittinen, Russell King - ARM Linux admin
  Cc: mazziesaccount, corbet, cw00.choi, krzk, b.zolnierkie,
	mturquette, pombredanne, sre, vkoul, linux, pavel, sjhuang,
	andrew.smirnov, djkurtz, akshu.agrawal, rafael.j.wysocki,
	linux-doc, linux-kernel, linux-clk, linux-arm-kernel

Quoting Russell King - ARM Linux admin (2019-01-31 07:21:47)
> On Thu, Jan 31, 2019 at 03:24:52PM +0200, Matti Vaittinen wrote:
> > Hello All,
> > 
> > On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> > > Series adds managed clkdev lookup interfaces and cleans few drivers
> > > 
> > > Few clk drivers appear to be leaking clkdev lookup registrations at
> > > driver remove. The patch series adds devm versions of lookup
> > > registrations and cleans up few drivers. Driver clean-up patches have
> > > not been tested as I lack the HW. All testing and comments if
> > > driver/device removal is even possible for changed drivers is highly
> > > appreciated. If removal is not possible I will gladly drop the patches
> > > from series - although leaking lookups may serve as bad example for new
> > > developers =)
> > > 
> > > Changed drivers are:
> > > clk-max77686 and clk-st
> > > 
> > > Please note that the patch #2 requires this change to work correctly:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > 
> > I guess the dependency mentioned abowe is already in (most) of the
> > trees. (I can't say for sure as I don't know what is the correct tree
> > for clkdev - is it linux-arm.git as Russel is maintaining clkdev?
> 
> Yes, I'm supposed to be maintaining clkdev, but I'm busy with other
> stuff (such as reorganising my network, helping people with SFP issues,
> I'm supposed to be replying to Arend over a brcmfmac issue that has been
> on-going since Christmas which I haven't yet been able to doing the next
> test...)
> 
> > If yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > "clk: of-provider: look at parent if registered device has no
> > provider info" seems to be sitting in maser branch).
> 
> It's there because it's part of the mainline kernel and has been
> since v5.0-rc1.  So, if you depend on that commit, basing off
> v5.0-rc1 is probably sane, unless there's something else that
> conflicts.
> 

I can pick up the patches in the clk tree. Clk drivers will be the users
so it's probably simplest to merge the clkdev patch in clk tree and then
consumers can be updated at the same time in a patch stack on top of
that. Otherwise, if I can get a stable branch/tag to pile the clk driver
patches on I can apply them that way and then the clkdev patch can go
via Russell.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 0/3] clk: clkdev add managed lookup registrations
  2019-01-31 19:38     ` Stephen Boyd
@ 2019-02-01  8:40       ` Matti Vaittinen
  0 siblings, 0 replies; 11+ messages in thread
From: Matti Vaittinen @ 2019-02-01  8:40 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Russell King - ARM Linux admin, mazziesaccount, corbet,
	cw00.choi, krzk, b.zolnierkie, mturquette, pombredanne, sre,
	vkoul, linux, pavel, sjhuang, andrew.smirnov, djkurtz,
	akshu.agrawal, rafael.j.wysocki, linux-doc, linux-kernel,
	linux-clk, linux-arm-kernel

On Thu, Jan 31, 2019 at 11:38:37AM -0800, Stephen Boyd wrote:
> Quoting Russell King - ARM Linux admin (2019-01-31 07:21:47)
> > On Thu, Jan 31, 2019 at 03:24:52PM +0200, Matti Vaittinen wrote:
> > > Hello All,
> > > 
> > > On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> > > > Series adds managed clkdev lookup interfaces and cleans few drivers
> > > > 
> > > > Few clk drivers appear to be leaking clkdev lookup registrations at
> > > > driver remove. The patch series adds devm versions of lookup
> > > > registrations and cleans up few drivers. Driver clean-up patches have
> > > > not been tested as I lack the HW. All testing and comments if
> > > > driver/device removal is even possible for changed drivers is highly
> > > > appreciated. If removal is not possible I will gladly drop the patches
> > > > from series - although leaking lookups may serve as bad example for new
> > > > developers =)
> > > > 
> > > > Changed drivers are:
> > > > clk-max77686 and clk-st
> > > > 
> > > > Please note that the patch #2 requires this change to work correctly:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > > 
> > > I guess the dependency mentioned abowe is already in (most) of the
> > > trees. (I can't say for sure as I don't know what is the correct tree
> > > for clkdev - is it linux-arm.git as Russel is maintaining clkdev?
> > 
> > Yes, I'm supposed to be maintaining clkdev, but I'm busy with other
> > stuff (such as reorganising my network, helping people with SFP issues,
> > I'm supposed to be replying to Arend over a brcmfmac issue that has been
> > on-going since Christmas which I haven't yet been able to doing the next
> > test...)
> > 
> > > If yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > > "clk: of-provider: look at parent if registered device has no
> > > provider info" seems to be sitting in maser branch).
> > 
> > It's there because it's part of the mainline kernel and has been
> > since v5.0-rc1.  So, if you depend on that commit, basing off
> > v5.0-rc1 is probably sane, unless there's something else that
> > conflicts.
> > 
> 
> I can pick up the patches in the clk tree. Clk drivers will be the users
> so it's probably simplest to merge the clkdev patch in clk tree and then
> consumers can be updated at the same time in a patch stack on top of
> that.

This sounds good to me if it is Ok to Russel. Please just let me know if
you want me to rebase (to clk-next?) and resend the series.

> Otherwise, if I can get a stable branch/tag to pile the clk driver
> patches on I can apply them that way and then the clkdev patch can go
> via Russell.

I can for sure go with this approach as well if it has some advantages
to you. Again, please just let me know if I should resend series and if
I should use something else but clk-next as a base.

Br,
	Matti Vaittinen

-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 1/3] clkdev: add managed clkdev lookup registration
  2018-12-07 11:09 ` [PATCH v7 1/3] clkdev: add managed clkdev lookup registration Matti Vaittinen
@ 2019-02-06 19:05   ` Stephen Boyd
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-02-06 19:05 UTC (permalink / raw)
  To: akshu.agrawal, andrew.smirnov, b.zolnierkie, corbet, cw00.choi,
	djkurtz, krzk, linux-arm-kernel, linux-clk, linux-doc,
	linux-kernel, linux, linux, matti.vaittinen, mazziesaccount,
	mturquette, pavel, pombredanne, rafael.j.wysocki, sjhuang, sre,
	vkoul

Quoting Matti Vaittinen (2018-12-07 03:09:39)
> Clkdev registration lacks of managed registration functions and it
> seems few drivers do not drop clkdev lookups at exit. Add
> devm_clk_hw_register_clkdev and devm_clk_release_clkdev to ease lookup
> releasing at exit.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/3] clk: clk-max77686: Clean clkdev lookup leak and use devm
  2018-12-07 11:10 ` [PATCH v7 2/3] clk: clk-max77686: Clean clkdev lookup leak and use devm Matti Vaittinen
@ 2019-02-06 19:05   ` Stephen Boyd
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-02-06 19:05 UTC (permalink / raw)
  To: akshu.agrawal, andrew.smirnov, b.zolnierkie, corbet, cw00.choi,
	djkurtz, krzk, linux-arm-kernel, linux-clk, linux-doc,
	linux-kernel, linux, linux, matti.vaittinen, mazziesaccount,
	mturquette, pavel, pombredanne, rafael.j.wysocki, sjhuang, sre,
	vkoul

Quoting Matti Vaittinen (2018-12-07 03:10:15)
> clk-max77686 never clean clkdev lookup at remove. This can cause
> oops if clk-max77686 is removed and inserted again. Fix leak by
> using new devm clkdev lookup registration. Simplify also error
> path by using new devm_of_clk_add_hw_provider.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 3/3] clk: clk-st: avoid clkdev lookup leak at remove
  2018-12-07 11:10 ` [PATCH v7 3/3] clk: clk-st: avoid clkdev lookup leak at remove Matti Vaittinen
@ 2019-02-06 19:05   ` Stephen Boyd
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-02-06 19:05 UTC (permalink / raw)
  To: akshu.agrawal, andrew.smirnov, b.zolnierkie, corbet, cw00.choi,
	djkurtz, krzk, linux-arm-kernel, linux-clk, linux-doc,
	linux-kernel, linux, linux, matti.vaittinen, mazziesaccount,
	mturquette, pavel, pombredanne, rafael.j.wysocki, sjhuang, sre,
	vkoul

Quoting Matti Vaittinen (2018-12-07 03:10:51)
> Use devm based clkdev lookup registration to avoid leaking lookup
> structures.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-02-06 19:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-07 11:09 [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
2018-12-07 11:09 ` [PATCH v7 1/3] clkdev: add managed clkdev lookup registration Matti Vaittinen
2019-02-06 19:05   ` Stephen Boyd
2018-12-07 11:10 ` [PATCH v7 2/3] clk: clk-max77686: Clean clkdev lookup leak and use devm Matti Vaittinen
2019-02-06 19:05   ` Stephen Boyd
2018-12-07 11:10 ` [PATCH v7 3/3] clk: clk-st: avoid clkdev lookup leak at remove Matti Vaittinen
2019-02-06 19:05   ` Stephen Boyd
2019-01-31 13:24 ` [PATCH v7 0/3] clk: clkdev add managed lookup registrations Matti Vaittinen
2019-01-31 15:21   ` Russell King - ARM Linux admin
2019-01-31 19:38     ` Stephen Boyd
2019-02-01  8:40       ` Matti Vaittinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).