[PATCH 1/2] clk: Fix comment for end of CONFIG_COMMON_CLK section

[PATCH 1/2] clk: Fix comment for end of CONFIG_COMMON_CLK section

[Mark Brown]

There is no else block so the #endif is for the end of the section for
CONFIG_COMMON_CLK.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/linux/clk.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/clk.h b/include/linux/clk.h
index b025272..4b27d15 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -81,7 +81,7 @@ int clk_notifier_register(struct clk *clk, struct notifier_block *nb);
 
 int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb);
 
-#endif /* !CONFIG_COMMON_CLK */
+#endif /* CONFIG_COMMON_CLK */
 
 /**
  * clk_get - lookup and obtain a reference to a clock producer.
-- 
1.7.9.1

[PATCH 2/2] clkdev: Implement managed clk_get()

[Mark Brown]

Allow clk API users to simplify their cleanup paths by providing a
managed version of clk_get().

Due to the lack of a standard struct clk to look up the device from a
managed clk_put() is not provided, it would be very unusual to use this
function so it's not a big loss.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 Documentation/driver-model/devres.txt |    3 +++
 drivers/clk/clkdev.c                  |   25 +++++++++++++++++++++++++
 include/linux/clk.h                   |   20 ++++++++++++++++++++
 3 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 2a596a4..3a10848 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -276,3 +276,6 @@ REGULATOR
   devm_regulator_get()
   devm_regulator_put()
   devm_regulator_bulk_get()
+
+CLOCK
+  devm_clk_get()
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6db161f..8af1d2c 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -89,6 +89,31 @@ void clk_put(struct clk *clk)
 }
 EXPORT_SYMBOL(clk_put);
 
+static void devm_clk_release(struct device *dev, void *res)
+{
+	clk_put(*(struct clk **)res);
+}
+
+struct clk *devm_clk_get(struct device *dev, const char *id)
+{
+	struct clk **ptr, *clk;
+
+	ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	clk = clk_get(dev, id);
+	if (!IS_ERR(clk)) {
+		*ptr = clk;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return clk;
+}
+EXPORT_SYMBOL_GPL(devm_clk_get);
+
 void clkdev_add(struct clk_lookup *cl)
 {
 	mutex_lock(&clocks_mutex);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 4b27d15..fd96e4a 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -101,6 +101,26 @@ int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb);
 struct clk *clk_get(struct device *dev, const char *id);
 
 /**
+ * devm_clk_get - lookup and obtain a managed reference to a clock producer.
+ * @dev: device for clock "consumer"
+ * @id: clock comsumer ID
+ *
+ * Returns a struct clk corresponding to the clock producer, or
+ * valid IS_ERR() condition containing errno.  The implementation
+ * uses @dev and @id to determine the clock consumer, and thereby
+ * the clock producer.  (IOW, @id may be identical strings, but
+ * clk_get may return different clock producers depending on @dev.)
+ *
+ * Drivers must assume that the clock source is not enabled.
+ *
+ * devm_clk_get should not be called from within interrupt context.
+ *
+ * The clock will automatically be freed when the device is unbound
+ * from the bus.
+ */
+struct clk *devm_clk_get(struct device *dev, const char *id);
+
+/**
  * clk_prepare - prepare a clock source
  * @clk: clock source
  *
-- 
1.7.9.1

Re: [PATCH 1/2] clk: Fix comment for end of CONFIG_COMMON_CLK section

[Russell King - ARM Linux]

On Sun, Apr 01, 2012 at 12:32:39PM +0100, Mark Brown wrote:
> There is no else block so the #endif is for the end of the section for
> CONFIG_COMMON_CLK.

Get rid of the comment entirely.  For just a single level of ifdef it
makes no sense.

Re: [PATCH 1/2] clk: Fix comment for end of CONFIG_COMMON_CLK section

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

On Sun, Apr 01, 2012 at 02:02:16PM +0100, Russell King - ARM Linux wrote:
> On Sun, Apr 01, 2012 at 12:32:39PM +0100, Mark Brown wrote:
> > There is no else block so the #endif is for the end of the section for
> > CONFIG_COMMON_CLK.

> Get rid of the comment entirely.  For just a single level of ifdef it
> makes no sense.

That's actually my preference too, I was mostly just sticking with the
existing style.  I'll send out a version doing that shortly.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Stephen Boyd]

On 4/1/2012 4:32 AM, Mark Brown wrote:
> Allow clk API users to simplify their cleanup paths by providing a
> managed version of clk_get().

I was thinking the same thing last week.

>
> Due to the lack of a standard struct clk to look up the device from a
> managed clk_put() is not provided, it would be very unusual to use this
> function so it's not a big loss.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  Documentation/driver-model/devres.txt |    3 +++
>  drivers/clk/clkdev.c                  |   25 +++++++++++++++++++++++++
>  include/linux/clk.h                   |   20 ++++++++++++++++++++
>  3 files changed, 48 insertions(+), 0 deletions(-)
>

But why is this part of clkdev.c? devm_clk_get() should work regardless
of the implementation of clk_get() so can we put it into some other file
that is compiled if HAVE_CLK=y so everyone benefits from this and not
just users who select CLKDEV_LOOKUP?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

On Sun, Apr 01, 2012 at 08:26:10AM -0700, Stephen Boyd wrote:
> On 4/1/2012 4:32 AM, Mark Brown wrote:

> >  Documentation/driver-model/devres.txt |    3 +++
> >  drivers/clk/clkdev.c                  |   25 +++++++++++++++++++++++++
> >  include/linux/clk.h                   |   20 ++++++++++++++++++++
> >  3 files changed, 48 insertions(+), 0 deletions(-)

> But why is this part of clkdev.c? devm_clk_get() should work regardless
> of the implementation of clk_get() so can we put it into some other file
> that is compiled if HAVE_CLK=y so everyone benefits from this and not
> just users who select CLKDEV_LOOKUP?

Mostly just because clk_get() is part of clkdev.c and I didn't feel like
creating a new file, though also because I really hope that we're going
to be moving away from open coding clock framework things so that we can
start to push clock API usage into non-SoC code.  Things like adding new
clocks are going to be a part of that.

To put it another way, why would a platform want to avoid clkdev?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Stephen Boyd]

On 04/01/12 08:34, Mark Brown wrote:
> On Sun, Apr 01, 2012 at 08:26:10AM -0700, Stephen Boyd wrote:
>> But why is this part of clkdev.c? devm_clk_get() should work regardless
>> of the implementation of clk_get() so can we put it into some other file
>> that is compiled if HAVE_CLK=y so everyone benefits from this and not
>> just users who select CLKDEV_LOOKUP?
> Mostly just because clk_get() is part of clkdev.c and I didn't feel like
> creating a new file, though also because I really hope that we're going
> to be moving away from open coding clock framework things so that we can
> start to push clock API usage into non-SoC code.  Things like adding new
> clocks are going to be a part of that.
>
> To put it another way, why would a platform want to avoid clkdev?

I hope we get a better clk_get() implementation with the unified struct
clk. Don't get me wrong, clkdev is a great improvement over open coding
clock framework stuff in each platform. But clkdev is really just
another platform specific implementation that most platforms decide to
use. Each platform has to select the option and it breaks if two
platforms implement __clk_get()/__clk_put() in conflicting ways. Ideally
the unified struct clk code guts clkdev and uses the core parts of it
for its own clk_get() implementation.

Sticking devm_clk_get() into clkdev.c is simple, no new file, smaller
diff. Great. But linking it to clkdev doesn't sound much better when
we're trying to get rid of platform specific code and this code is
entirely platform independent.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Russell King - ARM Linux]

On Mon, Apr 02, 2012 at 09:48:31AM -0700, Stephen Boyd wrote:
> I hope we get a better clk_get() implementation with the unified struct
> clk. Don't get me wrong, clkdev is a great improvement over open coding
> clock framework stuff in each platform. But clkdev is really just
> another platform specific implementation

Utter crap.  It is not platform specific.

> that most platforms decide to
> use. Each platform has to select the option and it breaks if two
> platforms implement __clk_get()/__clk_put() in conflicting ways.

They should go away with the common clock stuff: they are there to deal
with the implementation specific parts of struct clk, and as the common
clock stuff sorts that out, these should be provided by the common clk.

So any platform using the common clock will be compatible with any other
platform using the common clock.

If you somehow think that clkdev comes into that compatibility, you're
wrong.  It doesn't.

And if you think that a private clk implementation could have a unified
clk_get(), you're also barking mad.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Stephen Boyd]

On 04/02/12 09:52, Russell King - ARM Linux wrote:
> On Mon, Apr 02, 2012 at 09:48:31AM -0700, Stephen Boyd wrote:
>> I hope we get a better clk_get() implementation with the unified struct
>> clk. Don't get me wrong, clkdev is a great improvement over open coding
>> clock framework stuff in each platform. But clkdev is really just
>> another platform specific implementation
> Utter crap.  It is not platform specific.

It has compile-time platform hooks so it isn't entirely generic.

>
>> that most platforms decide to
>> use. Each platform has to select the option and it breaks if two
>> platforms implement __clk_get()/__clk_put() in conflicting ways.
> They should go away with the common clock stuff: they are there to deal
> with the implementation specific parts of struct clk, and as the common
> clock stuff sorts that out, these should be provided by the common clk.

Agreed. They should all be deleted and only one should exist.

>
> So any platform using the common clock will be compatible with any other
> platform using the common clock.
>
> If you somehow think that clkdev comes into that compatibility, you're
> wrong.  It doesn't.

I don't.

>
> And if you think that a private clk implementation could have a unified
> clk_get(), you're also barking mad.

I don't understand this. Maybe I'm barking mad already.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Russell King - ARM Linux]

On Sun, Apr 01, 2012 at 12:32:40PM +0100, Mark Brown wrote:
> Allow clk API users to simplify their cleanup paths by providing a
> managed version of clk_get().
> 
> Due to the lack of a standard struct clk to look up the device from a
> managed clk_put() is not provided, it would be very unusual to use this
> function so it's not a big loss.

Err, why?  The contents of struct clk has nothing to do with clk_put().
You're doing something really wrong here.

Remember, there is not going to _ever_ be the situation where a struct clk
is specific to any particular struct device - it's a 1:N mapping between
clks and devices.

So, until you sort out your misunderstanding, NAK.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Russell King - ARM Linux]

On Mon, Apr 02, 2012 at 10:04:03AM -0700, Stephen Boyd wrote:
> On 04/02/12 09:52, Russell King - ARM Linux wrote:
> > On Mon, Apr 02, 2012 at 09:48:31AM -0700, Stephen Boyd wrote:
> >> I hope we get a better clk_get() implementation with the unified struct
> >> clk. Don't get me wrong, clkdev is a great improvement over open coding
> >> clock framework stuff in each platform. But clkdev is really just
> >> another platform specific implementation
> > Utter crap.  It is not platform specific.
> 
> It has compile-time platform hooks so it isn't entirely generic.

Compile time hooks which are necessary to ensure safety of the provided
struct clk.  You can't implement a clk_get() which doesn't have either
knowledge of the struct clk or some kind of hook into platform specific
code.  That's a hard and unarguable fact.

> >> that most platforms decide to
> >> use. Each platform has to select the option and it breaks if two
> >> platforms implement __clk_get()/__clk_put() in conflicting ways.
> > They should go away with the common clock stuff: they are there to deal
> > with the implementation specific parts of struct clk, and as the common
> > clock stuff sorts that out, these should be provided by the common clk.
> 
> Agreed. They should all be deleted and only one should exist.

Utter crap.  Deleting them makes the non-common clock implementations
unsafe.  If a struct clk is provided by a module (and we do have some
which are) then the module reference count has to be held.  That's
what these hooks do.

When these platforms get converted over to the common clock, and the
issues surrounding dynamically registered and removed clocks are sane,
these hooks have to be used by the common clock to deal with the
refcounting so that common code knows when the structures can be freed.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Stephen Boyd]

On 04/02/12 10:08, Russell King - ARM Linux wrote:
> On Mon, Apr 02, 2012 at 10:04:03AM -0700, Stephen Boyd wrote:
>> On 04/02/12 09:52, Russell King - ARM Linux wrote:
>>> On Mon, Apr 02, 2012 at 09:48:31AM -0700, Stephen Boyd wrote:
>>>> I hope we get a better clk_get() implementation with the unified struct
>>>> clk. Don't get me wrong, clkdev is a great improvement over open coding
>>>> clock framework stuff in each platform. But clkdev is really just
>>>> another platform specific implementation
>>> Utter crap.  It is not platform specific.
>> It has compile-time platform hooks so it isn't entirely generic.
> Compile time hooks which are necessary to ensure safety of the provided
> struct clk.  You can't implement a clk_get() which doesn't have either
> knowledge of the struct clk or some kind of hook into platform specific
> code.  That's a hard and unarguable fact.
>
>>>> that most platforms decide to
>>>> use. Each platform has to select the option and it breaks if two
>>>> platforms implement __clk_get()/__clk_put() in conflicting ways.
>>> They should go away with the common clock stuff: they are there to deal
>>> with the implementation specific parts of struct clk, and as the common
>>> clock stuff sorts that out, these should be provided by the common clk.
>> Agreed. They should all be deleted and only one should exist.
> Utter crap.  Deleting them makes the non-common clock implementations
> unsafe.  If a struct clk is provided by a module (and we do have some
> which are) then the module reference count has to be held.  That's
> what these hooks do.
>
> When these platforms get converted over to the common clock, and the
> issues surrounding dynamically registered and removed clocks are sane,
> these hooks have to be used by the common clock to deal with the
> refcounting so that common code knows when the structures can be freed.

I'm saying that when every platform is using the common clock code we
would only have one __clk_get() implementation and we should be able to
delete clkdev.h entirely.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Mon, Apr 02, 2012 at 09:48:31AM -0700, Stephen Boyd wrote:

> I hope we get a better clk_get() implementation with the unified struct
> clk. Don't get me wrong, clkdev is a great improvement over open coding
> clock framework stuff in each platform. But clkdev is really just
> another platform specific implementation that most platforms decide to
> use. Each platform has to select the option and it breaks if two

> Sticking devm_clk_get() into clkdev.c is simple, no new file, smaller
> diff. Great. But linking it to clkdev doesn't sound much better when
> we're trying to get rid of platform specific code and this code is
> entirely platform independent.

Why wouldn't we want to continue to use clkdev with the generic clock
framework?  There's nothing particularly wrong with clkdev and we need a
standard mechanism for doing this anyway.  Frankly I was very surprised
when I looked just now and realised that the generic framework doesn't
use it automatically, I might just send a patch for that...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Russell King - ARM Linux]

On Mon, Apr 02, 2012 at 10:16:03AM -0700, Stephen Boyd wrote:
> On 04/02/12 10:08, Russell King - ARM Linux wrote:
> > Utter crap.  Deleting them makes the non-common clock implementations
> > unsafe.  If a struct clk is provided by a module (and we do have some
> > which are) then the module reference count has to be held.  That's
> > what these hooks do.
> >
> > When these platforms get converted over to the common clock, and the
> > issues surrounding dynamically registered and removed clocks are sane,
> > these hooks have to be used by the common clock to deal with the
> > refcounting so that common code knows when the structures can be freed.
> 
> I'm saying that when every platform is using the common clock code we
> would only have one __clk_get() implementation and we should be able to
> delete clkdev.h entirely.

No you did not, you said quite clearly that clkdev should go away and
be replaced by something else, because you see clkdev as just another
"platform specific implementation" (your words).  You were definitely
not talking about _just_ the backends for clkdev's clk_get().

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Turquette, Mike]

On Mon, Apr 2, 2012 at 10:16 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Apr 02, 2012 at 09:48:31AM -0700, Stephen Boyd wrote:
>
>> I hope we get a better clk_get() implementation with the unified struct
>> clk. Don't get me wrong, clkdev is a great improvement over open coding
>> clock framework stuff in each platform. But clkdev is really just
>> another platform specific implementation that most platforms decide to
>> use. Each platform has to select the option and it breaks if two
>
>> Sticking devm_clk_get() into clkdev.c is simple, no new file, smaller
>> diff. Great. But linking it to clkdev doesn't sound much better when
>> we're trying to get rid of platform specific code and this code is
>> entirely platform independent.
>
> Why wouldn't we want to continue to use clkdev with the generic clock
> framework?  There's nothing particularly wrong with clkdev and we need a
> standard mechanism for doing this anyway.  Frankly I was very surprised
> when I looked just now and realised that the generic framework doesn't
> use it automatically, I might just send a patch for that...

In the interest of getting merged I kept the common clk series as
small as I could.  It still came out much larger than the original
patches, so you can see why non-critical bits like the above are
missing.

I will take this opportunity to chime in and say that we can improve
clk_get.  I'd personally like to see clk_get return an opaque cookie
that is per-device.  This will really help out the clk_set_rate case
where we want to start managing multiple different drivers which might
invoke a set_rate on the same clock (which is increasingly more likely
now that we can propagate rate change requests up to shared parents).

So essentially clk_get(dev, con_id) would return a random u32 (or
maybe not random... maybe generated from a hashing function or
whatever), and all  further clk_* ops would use that id.  Thus the
common clk framework would know exactly which driver was calling any
given function.

The above suggestion is not critical for single dimensional operations
such as clk_(un)prepare and clk_enable/clk_disable.  Use counting is
adequate.  Tracking driver requests is very useful for two-dimensional
calls like clk_set_rate, where we might want to keep a plist of rates
that drivers had requested so that clk_set_rate is no longer a
He-Who-Writes-Last-Wins affair.

/me dons flame-retardant suit.

Regards,
Mike

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Stephen Boyd]

On 04/02/12 10:21, Russell King - ARM Linux wrote:
> On Mon, Apr 02, 2012 at 10:16:03AM -0700, Stephen Boyd wrote:
>> On 04/02/12 10:08, Russell King - ARM Linux wrote:
>>> Utter crap.  Deleting them makes the non-common clock implementations
>>> unsafe.  If a struct clk is provided by a module (and we do have some
>>> which are) then the module reference count has to be held.  That's
>>> what these hooks do.
>>>
>>> When these platforms get converted over to the common clock, and the
>>> issues surrounding dynamically registered and removed clocks are sane,
>>> these hooks have to be used by the common clock to deal with the
>>> refcounting so that common code knows when the structures can be freed.
>> I'm saying that when every platform is using the common clock code we
>> would only have one __clk_get() implementation and we should be able to
>> delete clkdev.h entirely.
> No you did not, you said quite clearly that clkdev should go away and
> be replaced by something else, because you see clkdev as just another
> "platform specific implementation" (your words).  You were definitely
> not talking about _just_ the backends for clkdev's clk_get().

Sorry. I was speaking about the future when we have one struct clk
definition. I suppose that wasn't clear because I wasn't explicit.

I do think we should improve/replace clkdev by keeping the core parts
(device name and connection name mapping) and internalizing it in the
common clock code. Your comment about a 1:N mapping between clocks and
devices sounds unfortunate.

Ideally we should be generating struct clks at runtime when clk_get() is
called. This way we can tie each caller of clk_get() to a different
instance of a struct clk that eventually all maps back to the same
struct clk_hw in the hardware layer. Right now clkdev is managing the
mapping at too high of a level and denies any such dynamic allocation
scheme.

If we did this it would help us locate bad drivers who all share some
clock in common. Finding which driver is keeping a clock on is annoying
when you have to trace each clk_enable/disable call and see who is
calling it to figure out which driver hasn't released its vote.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

On Mon, Apr 02, 2012 at 06:04:43PM +0100, Russell King - ARM Linux wrote:
> On Sun, Apr 01, 2012 at 12:32:40PM +0100, Mark Brown wrote:
> > Allow clk API users to simplify their cleanup paths by providing a
> > managed version of clk_get().

> > Due to the lack of a standard struct clk to look up the device from a
> > managed clk_put() is not provided, it would be very unusual to use this
> > function so it's not a big loss.

> Err, why?  The contents of struct clk has nothing to do with clk_put().
> You're doing something really wrong here.

It does for a devm_clk_put().  Normally this would end up being:

	void devm_clk_put(struct clk *clk);

but the devres stuff needs us to have a struct device to get the
underlying allocation/mapping and undo it.

> Remember, there is not going to _ever_ be the situation where a struct clk
> is specific to any particular struct device - it's a 1:N mapping between
> clks and devices.

Right, absolutely - to do it as above struct clk would be allocated per
user and indirect to the actual clock implementation (which some people
were muttering about for other reasons, though I can't remember what
those were off the top of my head).  Probably what would actually end up
happening is that we'd instead have a signature like:

	devm_clk_put(struct device *dev, struct clk *clk);

but I didn't particularly feel like making that decision right now,
especially if we do end up going with per user allocations and can use
the more idiomatic signature.

> So, until you sort out your misunderstanding, NAK.

I think I understand just fine, thanks.

In any case, we'd only really need a devm_clk_put() if someone wants one
which is a bit of a corner case in the first place so just ignoring the
issue until that happens should be fine.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Russell King - ARM Linux]

On Mon, Apr 02, 2012 at 10:34:00AM -0700, Stephen Boyd wrote:
> I do think we should improve/replace clkdev by keeping the core parts
> (device name and connection name mapping) and internalizing it in the
> common clock code. Your comment about a 1:N mapping between clocks and
> devices sounds unfortunate.

Think about it.  You have a bus with a common clock feeding all peripherals
to qualify the data on the bus.  Do you:

(a) create N individual clocks parented to a single clock for the bus
(b) return a single clock for the entire bus

Re: [PATCH 2/2] clkdev: Implement managed clk_get()

[Russell King - ARM Linux]

On Mon, Apr 02, 2012 at 06:34:12PM +0100, Mark Brown wrote:
> On Mon, Apr 02, 2012 at 06:04:43PM +0100, Russell King - ARM Linux wrote:
> > On Sun, Apr 01, 2012 at 12:32:40PM +0100, Mark Brown wrote:
> > > Allow clk API users to simplify their cleanup paths by providing a
> > > managed version of clk_get().
> 
> > > Due to the lack of a standard struct clk to look up the device from a
> > > managed clk_put() is not provided, it would be very unusual to use this
> > > function so it's not a big loss.
> 
> > Err, why?  The contents of struct clk has nothing to do with clk_put().
> > You're doing something really wrong here.
> 
> It does for a devm_clk_put().  Normally this would end up being:
> 
> 	void devm_clk_put(struct clk *clk);
> 
> but the devres stuff needs us to have a struct device to get the
> underlying allocation/mapping and undo it.

So why can't we do:

	void devm_clk_put(struct device *, struct clk *);

just like other interfaces which free up devres stuff take a struct device.

> Probably what would actually end up
> happening is that we'd instead have a signature like:
> 
> 	devm_clk_put(struct device *dev, struct clk *clk);
> 
> but I didn't particularly feel like making that decision right now,
> especially if we do end up going with per user allocations and can use
> the more idiomatic signature.

Well that's what other subsystems do, so I see no reason to be different.
To be different would make us intentionally different from other
implementations which would be bad - and means that we're more reliant
on the underlying clk implementation.

That's bad news, especially for something that's going to end up being
used in multiple drivers (in terms of fixing the API if the underlying
implementation changes.)

Given that we're at this cross-roads already, just pass the struct device
and be done with it.