From: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
To: linux@armlinux.org.uk, sboyd@codeaurora.org,
matti.vaittinen@fi.rohmeurope.com, mazziesaccount@gmail.com
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org
Subject: [PATCH] clk: clkdev - add managed versions of lookup registrations
Date: Thu, 28 Jun 2018 10:54:53 +0300 [thread overview]
Message-ID: <20180628075453.GA7793@localhost.localdomain> (raw)
Add devm_clk_hw_register_clkdev, devm_clk_register_clkdev and
devm_clk_release_clkdev as a first styep to clean up drivers which are
leaking clkdev lookups at driver remove.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
While searching for example on how clk drivers release clkdev at exit
I found that many of those don't. Simple grep for clkdev under
drivers/clk suggest that bunch of drivers which call clk_register_clkdev
or clk_hw_register_clkdev never call clkdev_drop. In order to help
cleaning this up I decided to add devm versions of clk_register_clkdev
and clk_hw_register_clkdev. I hope this would allow simply converting
bunch of calls to clk_register_clkdev and clk_hw_register_clkdev into
managed versions without building further clean up logic.
Please review this carefully. I have only performed some simple tests
with my BD71837 driver. And as I have only limited understanding on
clkdev lookups I may have done mistakes. Thanks!
drivers/clk/clkdev.c | 165 +++++++++++++++++++++++++++++++++++++++++--------
include/linux/clkdev.h | 8 +++
2 files changed, 147 insertions(+), 26 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 7513411140b6..4752a0004a6c 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -390,7 +390,7 @@ void clkdev_drop(struct clk_lookup *cl)
}
EXPORT_SYMBOL(clkdev_drop);
-static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
+static struct clk_lookup *do_clk_register_clkdev(struct clk_hw *hw,
const char *con_id,
const char *dev_id, ...)
{
@@ -404,6 +404,24 @@ static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
return cl;
}
+static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
+ const char *con_id, const char *dev_id)
+{
+ struct clk_lookup *cl;
+
+ /*
+ * 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 = do_clk_register_clkdev(hw, con_id, "%s",
+ dev_id);
+ else
+ cl = do_clk_register_clkdev(hw, con_id, NULL);
+
+ return cl;
+}
+
/**
* clk_register_clkdev - register one clock lookup for a struct clk
* @clk: struct clk to associate with all clk_lookups
@@ -421,22 +439,18 @@ static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
int clk_register_clkdev(struct clk *clk, const char *con_id,
const char *dev_id)
{
- struct clk_lookup *cl;
-
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ int rval = 0;
- /*
- * 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);
+ if (IS_ERR(clk)) {
+ rval = PTR_ERR(clk);
+ } else {
+ struct clk_lookup *cl;
- return cl ? 0 : -ENOMEM;
+ cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, dev_id);
+ if (!cl)
+ rval = -ENOMEM;
+ }
+ return rval;
}
EXPORT_SYMBOL(clk_register_clkdev);
@@ -456,21 +470,120 @@ EXPORT_SYMBOL(clk_register_clkdev);
*/
int clk_hw_register_clkdev(struct clk_hw *hw, const char *con_id,
const char *dev_id)
+{
+ int rval = 0;
+
+ if (IS_ERR(hw)) {
+ rval = PTR_ERR(hw);
+ } else {
+ struct clk_lookup *cl;
+
+ cl = __clk_register_clkdev(hw, con_id, dev_id);
+ if (!cl)
+ rval = -ENOMEM;
+ }
+
+ return rval;
+}
+EXPORT_SYMBOL(clk_hw_register_clkdev);
+
+static void devm_clkdev_release(struct device *dev, void *res)
+{
+ clkdev_drop(*(struct clk_lookup **)res);
+}
+
+static int devm_clk_match_clkdev(struct device *dev, void *res, void *data)
+{
+ struct clk_lookup **l = res;
+
+ if (!l || !*l) {
+ WARN_ON(!l || !*l);
+ return 0;
+ }
+
+ return *l == data;
+}
+
+/**
+ * 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 or
+ * with devm_clk_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)
+{
+ struct clk_lookup **cl = NULL;
if (IS_ERR(hw))
return PTR_ERR(hw);
+ cl = devres_alloc(devm_clkdev_release, sizeof(*cl), GFP_KERNEL);
+ if (cl) {
+ *cl = __clk_register_clkdev(hw, con_id, dev_id);
+ if (*cl)
+ devres_add(dev, cl);
+ else
+ devres_free(cl);
+ }
+ return (cl && *cl) ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(devm_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);
-
- return cl ? 0 : -ENOMEM;
+/**
+ * devm_clk_register_clkdev - managed clk lookup registration for a struct clk
+ * @dev: device this lookup is bound
+ * @clk: struct clk to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_id: 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 clks
+ * from a previous clk_register() call, and return the error code for
+ * those. This is to permit this function to be called immediately
+ * after clk_register().
+ */
+int devm_clk_register_clkdev(struct device *dev, struct clk *clk,
+ const char *con_id, const char *dev_id)
+{
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+ return devm_clk_hw_register_clkdev(dev, __clk_get_hw(clk), con_id,
+ dev_id);
}
-EXPORT_SYMBOL(clk_hw_register_clkdev);
+EXPORT_SYMBOL(devm_clk_register_clkdev);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index 4890ff033220..27ebeae8ae26 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -52,4 +52,12 @@ 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_register_clkdev(struct device *dev, struct clk *clk,
+ const char *con_id, const char *dev_id);
+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
WARNING: multiple messages have this Message-ID (diff)
From: matti.vaittinen@fi.rohmeurope.com (Matti Vaittinen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: clkdev - add managed versions of lookup registrations
Date: Thu, 28 Jun 2018 10:54:53 +0300 [thread overview]
Message-ID: <20180628075453.GA7793@localhost.localdomain> (raw)
Add devm_clk_hw_register_clkdev, devm_clk_register_clkdev and
devm_clk_release_clkdev as a first styep to clean up drivers which are
leaking clkdev lookups at driver remove.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
While searching for example on how clk drivers release clkdev at exit
I found that many of those don't. Simple grep for clkdev under
drivers/clk suggest that bunch of drivers which call clk_register_clkdev
or clk_hw_register_clkdev never call clkdev_drop. In order to help
cleaning this up I decided to add devm versions of clk_register_clkdev
and clk_hw_register_clkdev. I hope this would allow simply converting
bunch of calls to clk_register_clkdev and clk_hw_register_clkdev into
managed versions without building further clean up logic.
Please review this carefully. I have only performed some simple tests
with my BD71837 driver. And as I have only limited understanding on
clkdev lookups I may have done mistakes. Thanks!
drivers/clk/clkdev.c | 165 +++++++++++++++++++++++++++++++++++++++++--------
include/linux/clkdev.h | 8 +++
2 files changed, 147 insertions(+), 26 deletions(-)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 7513411140b6..4752a0004a6c 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -390,7 +390,7 @@ void clkdev_drop(struct clk_lookup *cl)
}
EXPORT_SYMBOL(clkdev_drop);
-static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
+static struct clk_lookup *do_clk_register_clkdev(struct clk_hw *hw,
const char *con_id,
const char *dev_id, ...)
{
@@ -404,6 +404,24 @@ static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
return cl;
}
+static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
+ const char *con_id, const char *dev_id)
+{
+ struct clk_lookup *cl;
+
+ /*
+ * 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 = do_clk_register_clkdev(hw, con_id, "%s",
+ dev_id);
+ else
+ cl = do_clk_register_clkdev(hw, con_id, NULL);
+
+ return cl;
+}
+
/**
* clk_register_clkdev - register one clock lookup for a struct clk
* @clk: struct clk to associate with all clk_lookups
@@ -421,22 +439,18 @@ static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
int clk_register_clkdev(struct clk *clk, const char *con_id,
const char *dev_id)
{
- struct clk_lookup *cl;
-
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ int rval = 0;
- /*
- * 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);
+ if (IS_ERR(clk)) {
+ rval = PTR_ERR(clk);
+ } else {
+ struct clk_lookup *cl;
- return cl ? 0 : -ENOMEM;
+ cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, dev_id);
+ if (!cl)
+ rval = -ENOMEM;
+ }
+ return rval;
}
EXPORT_SYMBOL(clk_register_clkdev);
@@ -456,21 +470,120 @@ EXPORT_SYMBOL(clk_register_clkdev);
*/
int clk_hw_register_clkdev(struct clk_hw *hw, const char *con_id,
const char *dev_id)
+{
+ int rval = 0;
+
+ if (IS_ERR(hw)) {
+ rval = PTR_ERR(hw);
+ } else {
+ struct clk_lookup *cl;
+
+ cl = __clk_register_clkdev(hw, con_id, dev_id);
+ if (!cl)
+ rval = -ENOMEM;
+ }
+
+ return rval;
+}
+EXPORT_SYMBOL(clk_hw_register_clkdev);
+
+static void devm_clkdev_release(struct device *dev, void *res)
+{
+ clkdev_drop(*(struct clk_lookup **)res);
+}
+
+static int devm_clk_match_clkdev(struct device *dev, void *res, void *data)
+{
+ struct clk_lookup **l = res;
+
+ if (!l || !*l) {
+ WARN_ON(!l || !*l);
+ return 0;
+ }
+
+ return *l == data;
+}
+
+/**
+ * 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 or
+ * with devm_clk_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)
+{
+ struct clk_lookup **cl = NULL;
if (IS_ERR(hw))
return PTR_ERR(hw);
+ cl = devres_alloc(devm_clkdev_release, sizeof(*cl), GFP_KERNEL);
+ if (cl) {
+ *cl = __clk_register_clkdev(hw, con_id, dev_id);
+ if (*cl)
+ devres_add(dev, cl);
+ else
+ devres_free(cl);
+ }
+ return (cl && *cl) ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(devm_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);
-
- return cl ? 0 : -ENOMEM;
+/**
+ * devm_clk_register_clkdev - managed clk lookup registration for a struct clk
+ * @dev: device this lookup is bound
+ * @clk: struct clk to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_id: 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 clks
+ * from a previous clk_register() call, and return the error code for
+ * those. This is to permit this function to be called immediately
+ * after clk_register().
+ */
+int devm_clk_register_clkdev(struct device *dev, struct clk *clk,
+ const char *con_id, const char *dev_id)
+{
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+ return devm_clk_hw_register_clkdev(dev, __clk_get_hw(clk), con_id,
+ dev_id);
}
-EXPORT_SYMBOL(clk_hw_register_clkdev);
+EXPORT_SYMBOL(devm_clk_register_clkdev);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index 4890ff033220..27ebeae8ae26 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -52,4 +52,12 @@ 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_register_clkdev(struct device *dev, struct clk *clk,
+ const char *con_id, const char *dev_id);
+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
next reply other threads:[~2018-06-28 7:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-28 7:54 Matti Vaittinen [this message]
2018-06-28 7:54 ` [PATCH] clk: clkdev - add managed versions of lookup registrations Matti Vaittinen
2018-07-09 6:33 ` Stephen Boyd
2018-07-09 6:33 ` Stephen Boyd
2018-07-09 6:33 ` Stephen Boyd
[not found] ` <CANhJrGN9UkmAAp7-O1dgcviP21uYrAT3-BPLinKuAozR2uQvhQ@mail.gmail.com>
2018-07-17 7:42 ` Matti Vaittinen
2018-07-17 7:42 ` Matti Vaittinen
2018-07-17 7:42 ` Matti Vaittinen
2018-07-30 12:55 ` Matti Vaittinen
2018-07-30 12:55 ` Matti Vaittinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180628075453.GA7793@localhost.localdomain \
--to=matti.vaittinen@fi.rohmeurope.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mazziesaccount@gmail.com \
--cc=sboyd@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.