[v3,1/7] clkdev: Hold clocks_mutex while iterating clocks list
diff mbox series

Message ID 20190404215344.6330-2-sboyd@kernel.org
State New
Headers show
Series
  • Rewrite clk parent handling
Related show

Commit Message

Stephen Boyd April 4, 2019, 9:53 p.m. UTC
We recently introduced a change to support devm clk lookups. That change
introduced a code-path that used clk_find() without holding the
'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
list and so we need to prevent the list from being modified while
iterating over it by holding the mutex. Similarly, we don't need to hold
the 'clocks_mutex' besides when we're dereferencing the clk_lookup
pointer we find or while we're modifying/iterating the 'clocks' list.

Let's ensure that we have the mutex held while iterating the list and
fix the callers of clk_find() to only hold the mutex as long as they
need to. This should fix this race and also nicely move clk creation
outside of the 'clocks_mutex'.

Fixes: 3eee6c7d119c ("clkdev: add managed clkdev lookup registration")
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clkdev.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

Comments

Matti Vaittinen April 5, 2019, 6:51 a.m. UTC | #1
Hello Stephen,

Thanks for taking care of this!

On Thu, 2019-04-04 at 14:53 -0700, Stephen Boyd wrote:
> We recently introduced a change to support devm clk lookups. That
> change
> introduced a code-path that used clk_find() without holding the
> 'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
> list and so we need to prevent the list from being modified while
> iterating over it by holding the mutex. Similarly, we don't need to
> hold
> the 'clocks_mutex' besides when we're dereferencing the clk_lookup
> pointer

/// Snip

> -out:
> +static struct clk_lookup *clk_find(const char *dev_id, const char
> *con_id)
> +{
> +	struct clk_lookup *cl;
> +
> +	mutex_lock(&clocks_mutex);
> +	cl = __clk_find(dev_id, con_id);
>  	mutex_unlock(&clocks_mutex);
>  
> -	return cl ? clk : ERR_PTR(-ENOENT);
> +	return cl;
> +}

I am not an expert on this but reading commit message abowe and seeing
the code for clk_find() looks a bit scary. If I understand it
correctly, the clocks_mutex should be held when dereferencing the
clk_lookup returned by clk_find. The clk_find implementation drops the
lock before returning - which makes me think I miss something here. How
can the caller ever safely dereference returned clk_lookup pointer?
Just reading abowe makes me think that lock should be taken by whoever
is calling the clk_find, and dropped only after caller has used the
found clk_lookup for whatever caller intends to use it. Maybe I am
missing something?

Br,
	Matti Vaittinen
Stephen Boyd April 5, 2019, 8:37 p.m. UTC | #2
Quoting Vaittinen, Matti (2019-04-04 23:51:43)
> On Thu, 2019-04-04 at 14:53 -0700, Stephen Boyd wrote:
> > We recently introduced a change to support devm clk lookups. That
> > change
> > introduced a code-path that used clk_find() without holding the
> > 'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
> > list and so we need to prevent the list from being modified while
> > iterating over it by holding the mutex. Similarly, we don't need to
> > hold
> > the 'clocks_mutex' besides when we're dereferencing the clk_lookup
> > pointer
> 
> /// Snip
> 
> > -out:
> > +static struct clk_lookup *clk_find(const char *dev_id, const char
> > *con_id)
> > +{
> > +     struct clk_lookup *cl;
> > +
> > +     mutex_lock(&clocks_mutex);
> > +     cl = __clk_find(dev_id, con_id);
> >       mutex_unlock(&clocks_mutex);
> >  
> > -     return cl ? clk : ERR_PTR(-ENOENT);
> > +     return cl;
> > +}
> 
> I am not an expert on this but reading commit message abowe and seeing
> the code for clk_find() looks a bit scary. If I understand it
> correctly, the clocks_mutex should be held when dereferencing the
> clk_lookup returned by clk_find. The clk_find implementation drops the
> lock before returning - which makes me think I miss something here. How
> can the caller ever safely dereference returned clk_lookup pointer?
> Just reading abowe makes me think that lock should be taken by whoever
> is calling the clk_find, and dropped only after caller has used the
> found clk_lookup for whatever caller intends to use it. Maybe I am
> missing something?
> 

The only user after this patch (devm) is doing a pointer comparison so
it looks OK. But yes, in general there shouldn't be users of clk_find()
that dereference the pointer because there isn't any protection besides
the mutex.
Matti Vaittinen April 8, 2019, 10:49 a.m. UTC | #3
On Fri, Apr 05, 2019 at 01:37:24PM -0700, Stephen Boyd wrote:
> Quoting Vaittinen, Matti (2019-04-04 23:51:43)
> > On Thu, 2019-04-04 at 14:53 -0700, Stephen Boyd wrote:
> > > We recently introduced a change to support devm clk lookups. That
> > > change
> > > introduced a code-path that used clk_find() without holding the
> > > 'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
> > > list and so we need to prevent the list from being modified while
> > > iterating over it by holding the mutex. Similarly, we don't need to
> > > hold
> > > the 'clocks_mutex' besides when we're dereferencing the clk_lookup
> > > pointer
> > 
> > /// Snip
> > 
> > > -out:
> > > +static struct clk_lookup *clk_find(const char *dev_id, const char
> > > *con_id)
> > > +{
> > > +     struct clk_lookup *cl;
> > > +
> > > +     mutex_lock(&clocks_mutex);
> > > +     cl = __clk_find(dev_id, con_id);
> > >       mutex_unlock(&clocks_mutex);
> > >  
> > > -     return cl ? clk : ERR_PTR(-ENOENT);
> > > +     return cl;
> > > +}
> > 
> > I am not an expert on this but reading commit message abowe and seeing
> > the code for clk_find() looks a bit scary. If I understand it
> > correctly, the clocks_mutex should be held when dereferencing the
> > clk_lookup returned by clk_find. The clk_find implementation drops the
> > lock before returning - which makes me think I miss something here. How
> > can the caller ever safely dereference returned clk_lookup pointer?
> > Just reading abowe makes me think that lock should be taken by whoever
> > is calling the clk_find, and dropped only after caller has used the
> > found clk_lookup for whatever caller intends to use it. Maybe I am
> > missing something?
> > 
> 
> The only user after this patch (devm) is doing a pointer comparison so
> it looks OK. But yes, in general there shouldn't be users of clk_find()
> that dereference the pointer because there isn't any protection besides
> the mutex.

If the only (intended) user for clk_find is devm_clk_release_clkdev,
then we might want to write it in devm_clk_release_clkdev - just to
avoid similar errors (as I did with devm) in the future? I might even
consider renaming __clk_find to clk_find or to clk_find_unsafe - but
that's all just nitpicking :) Go with what you like to maintain :D

Best regards
    Matti Vaittinen
Stephen Boyd April 8, 2019, 5 p.m. UTC | #4
Quoting Matti Vaittinen (2019-04-08 03:49:41)
> On Fri, Apr 05, 2019 at 01:37:24PM -0700, Stephen Boyd wrote:
> > Quoting Vaittinen, Matti (2019-04-04 23:51:43)
> > > On Thu, 2019-04-04 at 14:53 -0700, Stephen Boyd wrote:
> > > > We recently introduced a change to support devm clk lookups. That
> > > > change
> > > > introduced a code-path that used clk_find() without holding the
> > > > 'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
> > > > list and so we need to prevent the list from being modified while
> > > > iterating over it by holding the mutex. Similarly, we don't need to
> > > > hold
> > > > the 'clocks_mutex' besides when we're dereferencing the clk_lookup
> > > > pointer
> > > 
> > > /// Snip
> > > 
> > > > -out:
> > > > +static struct clk_lookup *clk_find(const char *dev_id, const char
> > > > *con_id)
> > > > +{
> > > > +     struct clk_lookup *cl;
> > > > +
> > > > +     mutex_lock(&clocks_mutex);
> > > > +     cl = __clk_find(dev_id, con_id);
> > > >       mutex_unlock(&clocks_mutex);
> > > >  
> > > > -     return cl ? clk : ERR_PTR(-ENOENT);
> > > > +     return cl;
> > > > +}
> > > 
> > > I am not an expert on this but reading commit message abowe and seeing
> > > the code for clk_find() looks a bit scary. If I understand it
> > > correctly, the clocks_mutex should be held when dereferencing the
> > > clk_lookup returned by clk_find. The clk_find implementation drops the
> > > lock before returning - which makes me think I miss something here. How
> > > can the caller ever safely dereference returned clk_lookup pointer?
> > > Just reading abowe makes me think that lock should be taken by whoever
> > > is calling the clk_find, and dropped only after caller has used the
> > > found clk_lookup for whatever caller intends to use it. Maybe I am
> > > missing something?
> > > 
> > 
> > The only user after this patch (devm) is doing a pointer comparison so
> > it looks OK. But yes, in general there shouldn't be users of clk_find()
> > that dereference the pointer because there isn't any protection besides
> > the mutex.
> 
> If the only (intended) user for clk_find is devm_clk_release_clkdev,
> then we might want to write it in devm_clk_release_clkdev - just to
> avoid similar errors (as I did with devm) in the future? I might even
> consider renaming __clk_find to clk_find or to clk_find_unsafe - but
> that's all just nitpicking :) Go with what you like to maintain :D
> 

Sure. I was thinking along the same lines after you asked.
Russell King - ARM Linux admin April 8, 2019, 10:21 p.m. UTC | #5
On Mon, Apr 08, 2019 at 10:00:02AM -0700, Stephen Boyd wrote:
> Quoting Matti Vaittinen (2019-04-08 03:49:41)
> > On Fri, Apr 05, 2019 at 01:37:24PM -0700, Stephen Boyd wrote:
> > > Quoting Vaittinen, Matti (2019-04-04 23:51:43)
> > > > On Thu, 2019-04-04 at 14:53 -0700, Stephen Boyd wrote:
> > > > > We recently introduced a change to support devm clk lookups. That
> > > > > change
> > > > > introduced a code-path that used clk_find() without holding the
> > > > > 'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
> > > > > list and so we need to prevent the list from being modified while
> > > > > iterating over it by holding the mutex. Similarly, we don't need to
> > > > > hold
> > > > > the 'clocks_mutex' besides when we're dereferencing the clk_lookup
> > > > > pointer
> > > > 
> > > > /// Snip
> > > > 
> > > > > -out:
> > > > > +static struct clk_lookup *clk_find(const char *dev_id, const char
> > > > > *con_id)
> > > > > +{
> > > > > +     struct clk_lookup *cl;
> > > > > +
> > > > > +     mutex_lock(&clocks_mutex);
> > > > > +     cl = __clk_find(dev_id, con_id);
> > > > >       mutex_unlock(&clocks_mutex);
> > > > >  
> > > > > -     return cl ? clk : ERR_PTR(-ENOENT);
> > > > > +     return cl;
> > > > > +}
> > > > 
> > > > I am not an expert on this but reading commit message abowe and seeing
> > > > the code for clk_find() looks a bit scary. If I understand it
> > > > correctly, the clocks_mutex should be held when dereferencing the
> > > > clk_lookup returned by clk_find. The clk_find implementation drops the
> > > > lock before returning - which makes me think I miss something here. How
> > > > can the caller ever safely dereference returned clk_lookup pointer?
> > > > Just reading abowe makes me think that lock should be taken by whoever
> > > > is calling the clk_find, and dropped only after caller has used the
> > > > found clk_lookup for whatever caller intends to use it. Maybe I am
> > > > missing something?
> > > > 
> > > 
> > > The only user after this patch (devm) is doing a pointer comparison so
> > > it looks OK. But yes, in general there shouldn't be users of clk_find()
> > > that dereference the pointer because there isn't any protection besides
> > > the mutex.
> > 
> > If the only (intended) user for clk_find is devm_clk_release_clkdev,
> > then we might want to write it in devm_clk_release_clkdev - just to
> > avoid similar errors (as I did with devm) in the future? I might even
> > consider renaming __clk_find to clk_find or to clk_find_unsafe - but
> > that's all just nitpicking :) Go with what you like to maintain :D
> > 
> 
> Sure. I was thinking along the same lines after you asked.

This is rubbish.  The reason clk_find() doesn't take the lock is that
you _need_ to hold the lock while you dereference the clk_lookup data.
The lock isn't protecting just the lookup, it protects what you do with
the result of the lookup as well.

So, as I say, adding locking inside clk_find() is completely
misunderstanding the locking here.
Matti Vaittinen April 9, 2019, 5:31 a.m. UTC | #6
On Mon, 2019-04-08 at 23:21 +0100, Russell King - ARM Linux admin
wrote:
> On Mon, Apr 08, 2019 at 10:00:02AM -0700, Stephen Boyd wrote:
> > Quoting Matti Vaittinen (2019-04-08 03:49:41)
> > > On Fri, Apr 05, 2019 at 01:37:24PM -0700, Stephen Boyd wrote:
> > > > Quoting Vaittinen, Matti (2019-04-04 23:51:43)
> > > > > On Thu, 2019-04-04 at 14:53 -0700, Stephen Boyd wrote:
> > > > > > We recently introduced a change to support devm clk
> > > > > > lookups. That
> > > > > > change
> > > > > > introduced a code-path that used clk_find() without holding
> > > > > > the
> > > > > > 'clocks_mutex'. Unfortunately, clk_find() iterates over the
> > > > > > 'clocks'
> > > > > > list and so we need to prevent the list from being modified
> > > > > > while
> > > > > > iterating over it by holding the mutex. Similarly, we don't
> > > > > > need to
> > > > > > hold
> > > > > > the 'clocks_mutex' besides when we're dereferencing the
> > > > > > clk_lookup
> > > > > > pointer
> > > > > 
> > > > > /// Snip
> > > > > 
> > > > > > -out:
> > > > > > +static struct clk_lookup *clk_find(const char *dev_id,
> > > > > > const char
> > > > > > *con_id)
> > > > > > +{
> > > > > > +     struct clk_lookup *cl;
> > > > > > +
> > > > > > +     mutex_lock(&clocks_mutex);
> > > > > > +     cl = __clk_find(dev_id, con_id);
> > > > > >       mutex_unlock(&clocks_mutex);
> > > > > >  
> > > > > > -     return cl ? clk : ERR_PTR(-ENOENT);
> > > > > > +     return cl;
> > > > > > +}
> > > > > 
> > > > > I am not an expert on this but reading commit message abowe
> > > > > and seeing
> > > > > the code for clk_find() looks a bit scary. If I understand it
> > > > > correctly, the clocks_mutex should be held when dereferencing
> > > > > the
> > > > > clk_lookup returned by clk_find. The clk_find implementation
> > > > > drops the
> > > > > lock before returning - which makes me think I miss something
> > > > > here. How
> > > > > can the caller ever safely dereference returned clk_lookup
> > > > > pointer?
> > > > > Just reading abowe makes me think that lock should be taken
> > > > > by whoever
> > > > > is calling the clk_find, and dropped only after caller has
> > > > > used the
> > > > > found clk_lookup for whatever caller intends to use it. Maybe
> > > > > I am
> > > > > missing something?
> > > > > 
> > > > 
> > > > The only user after this patch (devm) is doing a pointer
> > > > comparison so
> > > > it looks OK. But yes, in general there shouldn't be users of
> > > > clk_find()
> > > > that dereference the pointer because there isn't any protection
> > > > besides
> > > > the mutex.
> > > 
> > > If the only (intended) user for clk_find is
> > > devm_clk_release_clkdev,
> > > then we might want to write it in devm_clk_release_clkdev - just
> > > to
> > > avoid similar errors (as I did with devm) in the future? I might
> > > even
> > > consider renaming __clk_find to clk_find or to clk_find_unsafe -
> > > but
> > > that's all just nitpicking :) Go with what you like to maintain
> > > :D
> > > 
> > 
> > Sure. I was thinking along the same lines after you asked.
> 
> This is rubbish.  The reason clk_find() doesn't take the lock is that
> you _need_ to hold the lock while you dereference the clk_lookup
> data.

I think we all agreed on this already. Stephen pointed out that the
current user(s) of clk_find() do _not_ dereference the pointer.

> The lock isn't protecting just the lookup, it protects what you do
> with
> the result of the lookup as well.

And we agreed on this too.

Br,
	Matti Vaittinen
Russell King - ARM Linux admin April 9, 2019, 8:57 a.m. UTC | #7
On Tue, Apr 09, 2019 at 05:31:48AM +0000, Vaittinen, Matti wrote:
> On Mon, 2019-04-08 at 23:21 +0100, Russell King - ARM Linux admin
> wrote:
> > On Mon, Apr 08, 2019 at 10:00:02AM -0700, Stephen Boyd wrote:
> > > Quoting Matti Vaittinen (2019-04-08 03:49:41)
> > > > On Fri, Apr 05, 2019 at 01:37:24PM -0700, Stephen Boyd wrote:
> > > > > Quoting Vaittinen, Matti (2019-04-04 23:51:43)
> > > > > > On Thu, 2019-04-04 at 14:53 -0700, Stephen Boyd wrote:
> > > > > > > We recently introduced a change to support devm clk
> > > > > > > lookups. That
> > > > > > > change
> > > > > > > introduced a code-path that used clk_find() without holding
> > > > > > > the
> > > > > > > 'clocks_mutex'. Unfortunately, clk_find() iterates over the
> > > > > > > 'clocks'
> > > > > > > list and so we need to prevent the list from being modified
> > > > > > > while
> > > > > > > iterating over it by holding the mutex. Similarly, we don't
> > > > > > > need to
> > > > > > > hold
> > > > > > > the 'clocks_mutex' besides when we're dereferencing the
> > > > > > > clk_lookup
> > > > > > > pointer
> > > > > > 
> > > > > > /// Snip
> > > > > > 
> > > > > > > -out:
> > > > > > > +static struct clk_lookup *clk_find(const char *dev_id,
> > > > > > > const char
> > > > > > > *con_id)
> > > > > > > +{
> > > > > > > +     struct clk_lookup *cl;
> > > > > > > +
> > > > > > > +     mutex_lock(&clocks_mutex);
> > > > > > > +     cl = __clk_find(dev_id, con_id);
> > > > > > >       mutex_unlock(&clocks_mutex);
> > > > > > >  
> > > > > > > -     return cl ? clk : ERR_PTR(-ENOENT);
> > > > > > > +     return cl;
> > > > > > > +}
> > > > > > 
> > > > > > I am not an expert on this but reading commit message abowe
> > > > > > and seeing
> > > > > > the code for clk_find() looks a bit scary. If I understand it
> > > > > > correctly, the clocks_mutex should be held when dereferencing
> > > > > > the
> > > > > > clk_lookup returned by clk_find. The clk_find implementation
> > > > > > drops the
> > > > > > lock before returning - which makes me think I miss something
> > > > > > here. How
> > > > > > can the caller ever safely dereference returned clk_lookup
> > > > > > pointer?
> > > > > > Just reading abowe makes me think that lock should be taken
> > > > > > by whoever
> > > > > > is calling the clk_find, and dropped only after caller has
> > > > > > used the
> > > > > > found clk_lookup for whatever caller intends to use it. Maybe
> > > > > > I am
> > > > > > missing something?
> > > > > > 
> > > > > 
> > > > > The only user after this patch (devm) is doing a pointer
> > > > > comparison so
> > > > > it looks OK. But yes, in general there shouldn't be users of
> > > > > clk_find()
> > > > > that dereference the pointer because there isn't any protection
> > > > > besides
> > > > > the mutex.
> > > > 
> > > > If the only (intended) user for clk_find is
> > > > devm_clk_release_clkdev,
> > > > then we might want to write it in devm_clk_release_clkdev - just
> > > > to
> > > > avoid similar errors (as I did with devm) in the future? I might
> > > > even
> > > > consider renaming __clk_find to clk_find or to clk_find_unsafe -
> > > > but
> > > > that's all just nitpicking :) Go with what you like to maintain
> > > > :D
> > > > 
> > > 
> > > Sure. I was thinking along the same lines after you asked.
> > 
> > This is rubbish.  The reason clk_find() doesn't take the lock is that
> > you _need_ to hold the lock while you dereference the clk_lookup
> > data.
> 
> I think we all agreed on this already. Stephen pointed out that the
> current user(s) of clk_find() do _not_ dereference the pointer.

Which is actually another incorrect statement - clk_get_sys()
dereferences it, but Stephen's patch does rearrange the code there.
Stephen Boyd April 10, 2019, 4:45 p.m. UTC | #8
Quoting Russell King - ARM Linux admin (2019-04-09 01:57:17)
> On Tue, Apr 09, 2019 at 05:31:48AM +0000, Vaittinen, Matti wrote:
> > On Mon, 2019-04-08 at 23:21 +0100, Russell King - ARM Linux admin
> > wrote:
> > > On Mon, Apr 08, 2019 at 10:00:02AM -0700, Stephen Boyd wrote:
> > > > > 
> > > > 
> > > > Sure. I was thinking along the same lines after you asked.
> > > 
> > > This is rubbish.  The reason clk_find() doesn't take the lock is that
> > > you _need_ to hold the lock while you dereference the clk_lookup
> > > data.
> > 
> > I think we all agreed on this already. Stephen pointed out that the
> > current user(s) of clk_find() do _not_ dereference the pointer.
> 
> Which is actually another incorrect statement - clk_get_sys()
> dereferences it, but Stephen's patch does rearrange the code there.
> 

I've split this patch into two. One to fix the locking of the new
clk_find() user and the other to introduce a clk_find_hw() API that lets
me use it later on in this series. I'll resend the series with this
update.

Patch
diff mbox series

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 8c4435c53f09..db82eee8e209 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -36,7 +36,7 @@  static DEFINE_MUTEX(clocks_mutex);
  * Then we take the most specific entry - with the following
  * order of precedence: dev+con > dev only > con only.
  */
-static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
+static struct clk_lookup *__clk_find(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *p, *cl = NULL;
 	int match, best_found = 0, best_possible = 0;
@@ -46,6 +46,8 @@  static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	if (con_id)
 		best_possible += 1;
 
+	lockdep_assert_held(&clocks_mutex);
+
 	list_for_each_entry(p, &clocks, node) {
 		match = 0;
 		if (p->dev_id) {
@@ -70,25 +72,37 @@  static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	return cl;
 }
 
-static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
-				 const char *con_id)
+static struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *cl;
-	struct clk *clk = NULL;
+	struct clk_hw *hw = ERR_PTR(-ENOENT);
 
 	mutex_lock(&clocks_mutex);
+	cl = __clk_find(dev_id, con_id);
+	if (cl)
+		hw = cl->clk_hw;
+	mutex_unlock(&clocks_mutex);
 
-	cl = clk_find(dev_id, con_id);
-	if (!cl)
-		goto out;
+	return hw;
+}
 
-	clk = clk_hw_create_clk(dev, cl->clk_hw, dev_id, con_id);
-	if (IS_ERR(clk))
-		cl = NULL;
-out:
+static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
+{
+	struct clk_lookup *cl;
+
+	mutex_lock(&clocks_mutex);
+	cl = __clk_find(dev_id, con_id);
 	mutex_unlock(&clocks_mutex);
 
-	return cl ? clk : ERR_PTR(-ENOENT);
+	return cl;
+}
+
+static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
+				 const char *con_id)
+{
+	struct clk_hw *hw = clk_find_hw(dev_id, con_id);
+
+	return clk_hw_create_clk(dev, hw, dev_id, con_id);
 }
 
 struct clk *clk_get_sys(const char *dev_id, const char *con_id)