diff mbox

[v7,1/3] Documentation: common clk API

Message ID alpine.DEB.2.00.1203201611330.22974@utopia.booyaka.com
State New
Headers show

Commit Message

Paul Walmsley March 20, 2012, 11:31 p.m. UTC
Hello Sascha,

On Sat, 17 Mar 2012, Sascha Hauer wrote:

> On Fri, Mar 16, 2012 at 04:21:17PM -0600, Paul Walmsley wrote:
>  
> > If the common clock code is to go upstream now, it should be marked as 
> > experimental.
> 
> No, please don't do this. This effectively marks the architectures using
> the generic clock framework experimental. We can mark drivers as 'you
> have been warned', but marking an architecture as experimental is the
> wrong sign for both its users and people willing to adopt the framework.
> Also we get this:
> 
> warning: (ARCH_MX1 && MACH_MX21 && ARCH_MX25 && MACH_MX27) selects COMMON_CLK which has unmet direct dependencies (EXPERIMENTAL)
> 
> (and no, I don't want to support to clock frameworks in parallel)

It sounds as if your objection is with CONFIG_EXPERIMENTAL.  If that is 
indeed your objection, I personally have no objection to simply marking 
the code experimental in the Kconfig text.  (Patch at the bottom of this 
message.)

We need to indicate in some way that the existing code and API is very 
likely to change in ways that could involve quite a bit of work for 
adopters.

> > This is because we know the API is not well-defined, and 
> > that both the API and the underlying mechanics will almost certainly need 
> > to change for non-trivial uses of the rate changing code (e.g., DVFS with 
> > external I/O devices).
> 
> Please leave DVFS out of the game. DVFS will use the clock framework for
> the F part and the regulator framework for the V part, but the clock
> framework should not get extended with DVFS features. The notifiers we
> currently have in the clock framework should give enough information
> for DVFS implementations.

Sadly, that's not so.

Consider a CPUFreq driver as one common clock framework user.  This driver 
will attempt to repeatedly change the rate of a clock.  Changing that 
clock's rate may also involve changing the rate of several other clocks 
used by active devices.  So drivers for these devices will need to 
register rate change notifiers.  The notifier callbacks might involve 
heavyweight work, such as asserting flow control to an 
externally-connected device.

Suppose now that the last registered device in the notifier chain cannot 
handle the frequency transition and must abort it.  This in turn will 
require notifier callbacks to all of the previously-notified devices to 
abort the change.  And then shortly after that, CPUFreq is likely to 
request the same frequency change again: hammering a notifier chain that 
can never succeed.

Bad enough.  We know at least one way to solve this problem.  We can use 
something like the clk_{block,allow}_changes() functions that have been 
discussed for some time now.  But these quickly reveal another problem in 
the existing API.  By default, when there are multiple enabled users of a 
clock, another entity is allowed to change the clock's rate, or the rate 
of any parent of that clock (!).

This has several implications.  One that is significant is that for any 
non-trivial clock tree, any driver that cares about its clock's rate will 
need to implement notifier callbacks.  This is because the driver has no 
way of knowing if or when any other code on the system will attempt to 
change that clock's rate or source.  Or any parent clock's rate or source 
might change.  Should we really force all current drivers using the clock 
code to implement these callbacks?  Or can we restrict the situations in 
which the clock's rate or parent can change by placing restrictions on the 
API?  But then, driver code may need to be rewritten, and behavior 
assumptions may change.

> Even if they don't and we have to change something here this will have 
> no influence on the architectures implementing their clock tree with the 
> common clock framework.

That sounds pretty confident.  Are you sure that the semantics are so well 
understood?

For example, should we allow clk_set_rate() on disabled clocks?  How about 
prepared, but disabled clocks?  If so, what exactly should the clock 
framework do in these circumstances?  Should notifier callbacks go out 
immediately to registered callbacks?  Or should those callbacks be delayed 
until the clock is prepared or enabled?  How should that work when 
clk_enable() cannot block?  And are you confident that any other user of 
the clock framework will answer these undefined questions in the same way 
you would?

The above questions are simply "scratching the surface."  (Just as 
examples, there are still significant questions about locking, reentrancy, 
and so on - [1] is just one example)

These questions have reasonable answers that I think can be mostly aligned 
on.  Thinking through the use-cases, and implications, and answering them, 
should have been the first task in working on the common clock code.  I am 
sorry to say -- and perhaps this is partially my fault -- that it seems as 
if most people are not even aware that these questions exist, despite 
discussions at several conferences and on the mailing lists.

Anyway.  It is okay if we want to have some starter common clock framework 
in mainline; this is why deferring the merge hasn't been proposed.  But 
the point is that anyone who bases their code or platform on the common 
clock framework needs to be aware that, to satisfy one of its major 
use-cases, the behavior and/or API of the common clock code may need to 
change significantly.

Explicitly stating this is not only simple courtesy to its users, many of 
whom won't have been privy to its development.  It also is intended to 
make the code easier to change once it reaches mainline.  Once several 
platforms start using it, there will naturally be resistance and 
conservatism in changing its semantics and interface.  Many drivers may 
have to be changed, across many different maintainers.  And power 
management code may well need to be revalidated on the platforms that use 
it.  PM code, in my opinion, is generally the most difficult code to debug 
in the kernel.

So, until the API is well-defined and does all that it needs to do for its 
major users, we should at least have something like the following patch 
applied.


- Paul

1. King, Russell.  _Re: [PATCH v3 3/5] clk: introduce the common clock 
framework_.  2 Dec 2011 20:23:06 +0000.  linux-omap mailing list.  
http://www.spinics.net/lists/linux-omap/msg61024.html


From: Paul Walmsley <paul@pwsan.com>
Date: Tue, 20 Mar 2012 17:19:06 -0600
Subject: [PATCH] clk: note that the common clk code is still subject to
 significant change

Indicate that the common clk code and API is still likely to require
significant change.  The API is not well-defined and both it and the
underlying mechanics are likely to need significant changes to support
non-trivial uses of the rate changing code, such as DVFS with external
I/O devices.  So any platforms that switch their implementation over
to this may need to revise much of their driver code and revalidate
their implementations until the behavior of the code is
better-defined.

A good time for removing this help text would be after at least two
platforms that do DVFS on groups of external I/O devices have ported
their clock implementations over to the common clk code.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Mike Turquette <mturquette@ti.com>
---
 drivers/clk/Kconfig |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Comments

Nicolas Pitre March 21, 2012, 3:15 a.m. UTC | #1
On Tue, 20 Mar 2012, Paul Walmsley wrote:

> We need to indicate in some way that the existing code and API is very 
> likely to change in ways that could involve quite a bit of work for 
> adopters.

[...]

> Anyway.  It is okay if we want to have some starter common clock framework 
> in mainline; this is why deferring the merge hasn't been proposed.  But 
> the point is that anyone who bases their code or platform on the common 
> clock framework needs to be aware that, to satisfy one of its major 
> use-cases, the behavior and/or API of the common clock code may need to 
> change significantly.

Paul,

While I understand your concern, please don't forget that the perfect is 
the enemy of the good.

This common clk API has been under development for over *two* years 
already, with several attempts to merge it.  And each previous merge 
attempt aborted because someone came along at the last minute to do 
exactly what you are doing i.e. underline all the flaws and call for a 
redesign.  This is becoming a bad joke.

We finally have something that the majority of reviewers are happy with 
and which should cover the majority of existing cases out there.  Let's 
give it a chance, shall we? Otherwise one might ask where were you 
during those development years if you claim that the behavior and/or API 
of the common clock code still need to change significantly?

> Explicitly stating this is not only simple courtesy to its users, many of 
> whom won't have been privy to its development.  It also is intended to 
> make the code easier to change once it reaches mainline.

The code will be easier to change once it is in mainline, simply due to 
the fact that you can also change all its users at once.  And it is well 
possible that most users won't have to deal with the same magnitude of 
complexity as yours, again reducing the scope for resistance to changes.

Let's see how things go with the current code and improve it 
incrementally.  Otherwise no one will get involved with this API which 
is almost just as bad as not having the code merged at all.

> So, until the API is well-defined and does all that it needs to do for its 
> major users, [...]

No, the API simply can't ever be well defined if people don't actually 
start using it to eventually refine it further.  Major users were 
converted to it, and in most cases The API does all that it needs to do 
already.  Otherwise you'll be stuck in a catch22 situation forever.

And APIs in the Linux kernel do change all the time.  There is no stable 
API in the kernel.  Extensions will come about eventually, and existing 
users (simple ones by definition if the current API already meets their 
needs) should be converted over easily.


Nicolas
Saravana Kannan March 21, 2012, 3:26 a.m. UTC | #2
On 03/20/2012 08:15 PM, Nicolas Pitre wrote:
> On Tue, 20 Mar 2012, Paul Walmsley wrote:
>
>> We need to indicate in some way that the existing code and API is very
>> likely to change in ways that could involve quite a bit of work for
>> adopters.
>
> [...]
>
>> Anyway.  It is okay if we want to have some starter common clock framework
>> in mainline; this is why deferring the merge hasn't been proposed.  But
>> the point is that anyone who bases their code or platform on the common
>> clock framework needs to be aware that, to satisfy one of its major
>> use-cases, the behavior and/or API of the common clock code may need to
>> change significantly.
>
> Paul,
>
> While I understand your concern, please don't forget that the perfect is
> the enemy of the good.
>
> This common clk API has been under development for over *two* years
> already, with several attempts to merge it.  And each previous merge
> attempt aborted because someone came along at the last minute to do
> exactly what you are doing i.e. underline all the flaws and call for a
> redesign.  This is becoming a bad joke.
>
> We finally have something that the majority of reviewers are happy with
> and which should cover the majority of existing cases out there.  Let's
> give it a chance, shall we? Otherwise one might ask where were you
> during those development years if you claim that the behavior and/or API
> of the common clock code still need to change significantly?
>
>> Explicitly stating this is not only simple courtesy to its users, many of
>> whom won't have been privy to its development.  It also is intended to
>> make the code easier to change once it reaches mainline.
>
> The code will be easier to change once it is in mainline, simply due to
> the fact that you can also change all its users at once.  And it is well
> possible that most users won't have to deal with the same magnitude of
> complexity as yours, again reducing the scope for resistance to changes.
>
> Let's see how things go with the current code and improve it
> incrementally.  Otherwise no one will get involved with this API which
> is almost just as bad as not having the code merged at all.
>
>> So, until the API is well-defined and does all that it needs to do for its
>> major users, [...]
>
> No, the API simply can't ever be well defined if people don't actually
> start using it to eventually refine it further.  Major users were
> converted to it, and in most cases The API does all that it needs to do
> already.  Otherwise you'll be stuck in a catch22 situation forever.
>
> And APIs in the Linux kernel do change all the time.  There is no stable
> API in the kernel.  Extensions will come about eventually, and existing
> users (simple ones by definition if the current API already meets their
> needs) should be converted over easily.

+1 to the general idea in Nicolas's email.

To add a few more thoughts, while I agree with Paul that there is room 
for improvement in the APIs, I think the difference in opinion comes 
when we ask the question:

"When we eventually refine the APIs in the future to be more expressive, 
do we think that the current APIs can or cannot be made as wrappers 
around the new more expressive APIs?"

Absolutes are almost never right, so I can't give an absolute answer. 
But I'm strongly leaning towards "we can" as the answer to the question. 
That combined with the reasons Nicholas mentioned is why I think the 
APIs should not be marked as experimental in anyway.

-Saravana
Paul Walmsley March 21, 2012, 7:30 a.m. UTC | #3
Hello Nico,

On Tue, 20 Mar 2012, Nicolas Pitre wrote:

> This common clk API has been under development for over *two* years 
> already, with several attempts to merge it.  And each previous merge 
> attempt aborted because someone came along at the last minute to do 
> exactly what you are doing i.e. underline all the flaws and call for a 
> redesign.  This is becoming a bad joke.

There is a misunderstanding.  I am not calling for a redesign.  I am 
simply stating that the current maturity level of the API and code should 
be documented in the Kconfig dependencies or description text before the 
code goes upstream.  The objectives are to make future changes easier, set 
expectations, and clearly disclose the extent to which the API and code 
will need to change.

> The code will be easier to change once it is in mainline, simply due to 
> the fact that you can also change all its users at once.  And it is well 
> possible that most users won't have to deal with the same magnitude of 
> complexity as yours, again reducing the scope for resistance to changes.

I hope you are right.  To me, these conclusions seem unlikely.  It seems 
equally likely that these rationales will make it much more difficult to 
change the code once it's upstream and platforms are depending on it.  
Particularly given the ongoing sensitivity to reducing "churn" of mainline 
code, so publicly discussed.  So it seems like a good idea to attempt to 
address these potential roadblocks and criticisms to major clock framework 
changes early.

> And APIs in the Linux kernel do change all the time.  There is no stable 
> API in the kernel.  Extensions will come about eventually, and existing 
> users (simple ones by definition if the current API already meets their 
> needs) should be converted over easily.

Yes, simple extensions should be straightforward.  Of greater concern are 
changes to the existing interface that will probably be required.  These 
could involve significant changes to driver or platform code.  It seems 
likely that there will be strong inertia to making these changes after 
platforms and drivers are converted.

However, if we clearly state that these API changes are likely until the 
API is well-defined, we will hopefully reduce some angst by disclosing
what some of us know.

...

One last comment to address which is orthogonal to the technical content 
of this discussion.

> Otherwise one might ask where were you during those development years if 
> you claim that the behavior and/or API of the common clock code still 
> need to change significantly?

One might ask this.  If one were to ask this, another might briefly 
outline the participation in public and private clock discussions at 
Linaro Connect in Budapest and Redwood Shores, at LPC in Santa Rosa, at 
ELCE/KS in Prague, at ELC in Redwood Shores, in conference calls, IRC 
sessions, and private E-mails with many of the people included in the 
header of this message, not to mention the list posts.

None of the concerns that have been described are new.  There has been an 
endeavour to discuss them with anyone who seemed even remotely interested.

Of course it is a personal source of regret that the participation could 
not have been greater, but this regret is hardly limited to the common 
clock project.


regards,

- Paul
Paul Walmsley March 21, 2012, 7:44 a.m. UTC | #4
Hello Saravana,

On Tue, 20 Mar 2012, Saravana Kannan wrote:

> To add a few more thoughts, while I agree with Paul that there is room for
> improvement in the APIs, I think the difference in opinion comes when we ask
> the question:
> 
> "When we eventually refine the APIs in the future to be more expressive, do we
> think that the current APIs can or cannot be made as wrappers around the new
> more expressive APIs?"
> 
> Absolutes are almost never right, so I can't give an absolute answer. 
> But I'm strongly leaning towards "we can" as the answer to the question.  
> That combined with the reasons Nicholas mentioned is why I think the 
> APIs should not be marked as experimental in anyway.

The resistance to documenting that the clock interface is not 
well-defined, and that therefore it is likely to change, and that users 
should not expect it to be stable, is perplexing to me.

Certainly a Kconfig help text change seems trivial enough.  But even the 
resistance to CONFIG_EXPERIMENTAL has been quite surprising to me, given 
that every single defconfig in arch/arm/defconfig sets it:

$ find arch/arm/configs -type f  | wc -l
122
$ fgrep -r CONFIG_EXPERIMENTAL=y arch/arm/configs | wc -l
122
$

(that includes iMX, by the way...)

Certainly, neither Kconfig change is going to prevent us on OMAP from 
figuring out what else is needed to convert our platform to the common 
clock code.  And given the level of enthusiasm on the lists, I don't think 
it's going to prevent many of the other ARM platforms from experimenting 
with the conversion, either.

So it would be interesting to know more about why you (or anyone else) 
perceive that the Kconfig changes would be harmful.


- Paul
Sascha Hauer March 21, 2012, 9:10 a.m. UTC | #5
On Wed, Mar 21, 2012 at 01:44:01AM -0600, Paul Walmsley wrote:
> Hello Saravana,
> 
> Certainly a Kconfig help text change seems trivial enough.  But even the 
> resistance to CONFIG_EXPERIMENTAL has been quite surprising to me, given 
> that every single defconfig in arch/arm/defconfig sets it:
> 
> $ find arch/arm/configs -type f  | wc -l
> 122
> $ fgrep -r CONFIG_EXPERIMENTAL=y arch/arm/configs | wc -l
> 122
> $
> 
> (that includes iMX, by the way...)
> 
> Certainly, neither Kconfig change is going to prevent us on OMAP from 
> figuring out what else is needed to convert our platform to the common 
> clock code.  And given the level of enthusiasm on the lists, I don't think 
> it's going to prevent many of the other ARM platforms from experimenting 
> with the conversion, either.
> 
> So it would be interesting to know more about why you (or anyone else) 
> perceive that the Kconfig changes would be harmful.

Mainly because COMMON_CLK is an invisible option which has to be
selected by architectures. So with the Kconfig change we either have to:

config ARCH_MXC
	depends on EXPERIMENTAL

or:

config ARCH_MXC
	select EXPERIMENTAL
	select COMMON_CLK

Neither of both seems very appealing to me.

You can add a warning to the Kconfig help text if you like, I
have no problem with that. As you said it will prevent noone
from using it anyway.

Sascha
Nicolas Pitre March 21, 2012, 1:23 p.m. UTC | #6
On Wed, 21 Mar 2012, Paul Walmsley wrote:

> Hello Nico,
> 
> On Tue, 20 Mar 2012, Nicolas Pitre wrote:
> 
> > This common clk API has been under development for over *two* years 
> > already, with several attempts to merge it.  And each previous merge 
> > attempt aborted because someone came along at the last minute to do 
> > exactly what you are doing i.e. underline all the flaws and call for a 
> > redesign.  This is becoming a bad joke.
> 
> There is a misunderstanding.  I am not calling for a redesign.  I am 
> simply stating that the current maturity level of the API and code should 
> be documented in the Kconfig dependencies or description text before the 
> code goes upstream.  The objectives are to make future changes easier, set 
> expectations, and clearly disclose the extent to which the API and code 
> will need to change.

Maybe there is no actual consensus on that extent.

> > The code will be easier to change once it is in mainline, simply due to 
> > the fact that you can also change all its users at once.  And it is well 
> > possible that most users won't have to deal with the same magnitude of 
> > complexity as yours, again reducing the scope for resistance to changes.
> 
> I hope you are right.  To me, these conclusions seem unlikely.  It seems 
> equally likely that these rationales will make it much more difficult to 
> change the code once it's upstream and platforms are depending on it.  

No code should go upstream if no one depends on it.  Therefore we change 
code that many platforms depend on all the time.  What is killing us is 
the need to synchronize with multiple external code bases scattered 
around.

> Particularly given the ongoing sensitivity to reducing "churn" of mainline 
> code, so publicly discussed.

Please stop buying into that crap.  We congratulate ourselves every 
merge cycles with the current process being able to deal with around 
half a million of lines changed every 3 months or so.  It's pointless 
churn that is frowned upon, not useful and incremental API changes.  I 
don't think that "pointless" would apply here.

> So it seems like a good idea to attempt to address these potential 
> roadblocks and criticisms to major clock framework changes early.

I understand your concern, however I don't think it is as serious as you 
are making it.

> One last comment to address which is orthogonal to the technical content 
> of this discussion.
> 
> > Otherwise one might ask where were you during those development years if 
> > you claim that the behavior and/or API of the common clock code still 
> > need to change significantly?
> 
> One might ask this.  If one were to ask this, another might briefly 
> outline the participation in public and private clock discussions at 
> Linaro Connect in Budapest and Redwood Shores, at LPC in Santa Rosa, at 
> ELCE/KS in Prague, at ELC in Redwood Shores, in conference calls, IRC 
> sessions, and private E-mails with many of the people included in the 
> header of this message, not to mention the list posts.

Sure.  I was there too and came across you many times.  I just don't 
understand why some apparent consensus was built around the current 
submission, that people involved appeared to be highly satisfied at 
last, given the emphasis you are giving to your present concern which 
doesn't seem to be widely shared.  This is a bit surprising.

> None of the concerns that have been described are new.  There has been an 
> endeavour to discuss them with anyone who seemed even remotely interested.

Let's change tactics then.  We've been discussing this code for over two 
years and no one could really benefit from it during that time.  Maybe 
future discussion could become more productive and concrete when some 
code is merged _and_ used at last.


Nicolas
Saravana Kannan March 21, 2012, 6:38 p.m. UTC | #7
On 03/21/2012 12:44 AM, Paul Walmsley wrote:
> Hello Saravana,
>
> On Tue, 20 Mar 2012, Saravana Kannan wrote:
>
>> To add a few more thoughts, while I agree with Paul that there is room for
>> improvement in the APIs, I think the difference in opinion comes when we ask
>> the question:
>>
>> "When we eventually refine the APIs in the future to be more expressive, do we
>> think that the current APIs can or cannot be made as wrappers around the new
>> more expressive APIs?"
>>
>> Absolutes are almost never right, so I can't give an absolute answer.
>> But I'm strongly leaning towards "we can" as the answer to the question.
>> That combined with the reasons Nicholas mentioned is why I think the
>> APIs should not be marked as experimental in anyway.
>
> The resistance to documenting that the clock interface is not
> well-defined, and that therefore it is likely to change, and that users
> should not expect it to be stable, is perplexing to me.
>
> Certainly a Kconfig help text change seems trivial enough.  But even the
> resistance to CONFIG_EXPERIMENTAL has been quite surprising to me, given
> that every single defconfig in arch/arm/defconfig sets it:
>
> $ find arch/arm/configs -type f  | wc -l
> 122
> $ fgrep -r CONFIG_EXPERIMENTAL=y arch/arm/configs | wc -l
> 122
> $
>
> (that includes iMX, by the way...)
>
> Certainly, neither Kconfig change is going to prevent us on OMAP from
> figuring out what else is needed to convert our platform to the common
> clock code.  And given the level of enthusiasm on the lists,

I think the enthusiasm we are seeing are from the clock driver 
developers. And for good reasons. Everyone is tired of having to write 
or rewrite their own implementation.

> I don't think
> it's going to prevent many of the other ARM platforms from experimenting
> with the conversion, either.
>
> So it would be interesting to know more about why you (or anyone else)
> perceive that the Kconfig changes would be harmful.

But the enthusiasm of the clock driver developers doesn't necessarily 
translate to users of the clock APIs (other driver devs). My worry with 
marking it as experimental in Kconfig and to a certain extent in the 
documentation is that it will discourage the driver devs from switching 
to the new APIs. If no one is using the new APIs, then platforms can't 
switch to the common clock framework either. If a lot of platform don't 
convert to the common clock framework, the effort to get it merged in 
now is not also very meaningful.

Also, at the rate at which we come to an agreement on new APIs, I think 
these new APIs should be considered quite stable :-) It's certainly 
better than what we have today. We should encourage driver devs to move 
to these new APIs and get the benefits while we go on another 2 yr cycle 
to agree on the next set of APIs improvements.

And as I said before, I think it's much less likely that we will break 
backward source compatibility when we do the next improvement. We could 
have mostly avoid this large scale change by calling the APIs 
prepare/unprepare/gate/ungate (or whatever) and make clk_enable/disable 
similar to clk_prepare_enable/clk_disable_unprepare(). That would have 
avoid a lot of drivers from having to mess with their clock calls. But 
we didn't think about that in the excitement for improved APIs. I think 
this will still be seared in our minds enough to make sure we don't 
repeat that in the future.

That covers all I have to say on this topic.

-Saravana
Mark Brown March 21, 2012, 7:07 p.m. UTC | #8
On Wed, Mar 21, 2012 at 11:38:58AM -0700, Saravana Kannan wrote:

> >So it would be interesting to know more about why you (or anyone else)
> >perceive that the Kconfig changes would be harmful.

> But the enthusiasm of the clock driver developers doesn't
> necessarily translate to users of the clock APIs (other driver
> devs). My worry with marking it as experimental in Kconfig and to a
> certain extent in the documentation is that it will discourage the
> driver devs from switching to the new APIs. If no one is using the
> new APIs, then platforms can't switch to the common clock framework

These aren't new APIs, the clock API has been around since forever.
For driver authors working on anything that isn't platform specific the
issue has been that it's not implemented at all on the overwhelming
majority of architectures and those that do all have their own random
implementations with their own random quirks and with no ability for
anything except the platform to even try to do incredibly basic stuff
like register a new clock.

Simply having something, anything, in place even if it's going to churn
is a massive step forward here for people working with clocks.
Tony Lindgren March 21, 2012, 7:33 p.m. UTC | #9
* Mark Brown <broonie@opensource.wolfsonmicro.com> [120321 12:11]:
> On Wed, Mar 21, 2012 at 11:38:58AM -0700, Saravana Kannan wrote:
> 
> > >So it would be interesting to know more about why you (or anyone else)
> > >perceive that the Kconfig changes would be harmful.
> 
> > But the enthusiasm of the clock driver developers doesn't
> > necessarily translate to users of the clock APIs (other driver
> > devs). My worry with marking it as experimental in Kconfig and to a
> > certain extent in the documentation is that it will discourage the
> > driver devs from switching to the new APIs. If no one is using the
> > new APIs, then platforms can't switch to the common clock framework
> 
> These aren't new APIs, the clock API has been around since forever.
> For driver authors working on anything that isn't platform specific the
> issue has been that it's not implemented at all on the overwhelming
> majority of architectures and those that do all have their own random
> implementations with their own random quirks and with no ability for
> anything except the platform to even try to do incredibly basic stuff
> like register a new clock.
> 
> Simply having something, anything, in place even if it's going to churn
> is a massive step forward here for people working with clocks.

Right, and now at least the people reading this thread are pretty
aware of the yet unsolved issues with clock fwk api :)

Regards,

Tony
Saravana Kannan March 21, 2012, 7:41 p.m. UTC | #10
On 03/21/2012 12:33 PM, Tony Lindgren wrote:
> * Mark Brown<broonie@opensource.wolfsonmicro.com>  [120321 12:11]:
>> On Wed, Mar 21, 2012 at 11:38:58AM -0700, Saravana Kannan wrote:
>>
>>>> So it would be interesting to know more about why you (or anyone else)
>>>> perceive that the Kconfig changes would be harmful.
>>
>>> But the enthusiasm of the clock driver developers doesn't
>>> necessarily translate to users of the clock APIs (other driver
>>> devs). My worry with marking it as experimental in Kconfig and to a
>>> certain extent in the documentation is that it will discourage the
>>> driver devs from switching to the new APIs. If no one is using the
>>> new APIs, then platforms can't switch to the common clock framework
>>
>> These aren't new APIs, the clock API has been around since forever.

I disagree. When I say clock API, I mean the actual functions and their 
semantics. Not the existence of a header file.

The meaning of clk_enable/disable has been changed and they won't work 
without calling clk_prepare/unprepare. So, these are definitely new 
APIs. If it weren't new APIs, then none of the general drivers would 
need to change.

>> For driver authors working on anything that isn't platform specific the
>> issue has been that it's not implemented at all on the overwhelming
>> majority of architectures and those that do all have their own random
>> implementations with their own random quirks and with no ability for
>> anything except the platform to even try to do incredibly basic stuff
>> like register a new clock.
>>
>> Simply having something, anything, in place even if it's going to churn
>> is a massive step forward here for people working with clocks.
>
> Right, and now at least the people reading this thread are pretty
> aware of the yet unsolved issues with clock fwk api :)

:-) Shhh... not so loud!

-Saravana
Mark Brown March 21, 2012, 7:56 p.m. UTC | #11
On Wed, Mar 21, 2012 at 12:41:41PM -0700, Saravana Kannan wrote:
> On 03/21/2012 12:33 PM, Tony Lindgren wrote:
> >* Mark Brown<broonie@opensource.wolfsonmicro.com>  [120321 12:11]:

> >>These aren't new APIs, the clock API has been around since forever.

> I disagree. When I say clock API, I mean the actual functions and
> their semantics. Not the existence of a header file.

> The meaning of clk_enable/disable has been changed and they won't
> work without calling clk_prepare/unprepare. So, these are definitely
> new APIs. If it weren't new APIs, then none of the general drivers
> would need to change.

clk_prepare() and clk_unprepare() are already there for any clk.h user
and are stubbed in no matter what, they're really not a meaningful
barrier here.
Saravana Kannan March 21, 2012, 8:04 p.m. UTC | #12
On 03/21/2012 12:56 PM, Mark Brown wrote:
> On Wed, Mar 21, 2012 at 12:41:41PM -0700, Saravana Kannan wrote:
>> On 03/21/2012 12:33 PM, Tony Lindgren wrote:
>>> * Mark Brown<broonie@opensource.wolfsonmicro.com>   [120321 12:11]:
>
>>>> These aren't new APIs, the clock API has been around since forever.
>
>> I disagree. When I say clock API, I mean the actual functions and
>> their semantics. Not the existence of a header file.
>
>> The meaning of clk_enable/disable has been changed and they won't
>> work without calling clk_prepare/unprepare. So, these are definitely
>> new APIs. If it weren't new APIs, then none of the general drivers
>> would need to change.
>
> clk_prepare() and clk_unprepare() are already there for any clk.h user
> and are stubbed in no matter what, they're really not a meaningful
> barrier here.

Isn't this whole experimental vs non-experimental discussion about the 
actual external facing clock APIs and not the platform driver facing APIs?

Sure, prepare/unprepare are already there in the .h file. But they are 
stubs and have no impact till we move to the common clock framework or 
platforms move to them with their own implementation (certainly not 
happening in upstream, so let's leave that part out of this discussion).

So. IMO, for all practical purposes, common clk fwk forces the move to 
these new APIs and hence IMO forces the new APIs.

-Saravana
Mark Brown March 21, 2012, 8:10 p.m. UTC | #13
On Wed, Mar 21, 2012 at 01:04:22PM -0700, Saravana Kannan wrote:

> Sure, prepare/unprepare are already there in the .h file. But they
> are stubs and have no impact till we move to the common clock
> framework or platforms move to them with their own implementation
> (certainly not happening in upstream, so let's leave that part out
> of this discussion).

> So. IMO, for all practical purposes, common clk fwk forces the move
> to these new APIs and hence IMO forces the new APIs.

Sure, if you want to look at it from that point of view - anything
wanting to run on a platform which uses the generic API needs to use
them, but there's no blocker on the user from this (it can convert with
or without the platform it runs on) - but it's hardly a tough sell.
Russell King - ARM Linux March 22, 2012, 12:42 a.m. UTC | #14
On Wed, Mar 21, 2012 at 12:41:41PM -0700, Saravana Kannan wrote:
> The meaning of clk_enable/disable has been changed and they won't work  
> without calling clk_prepare/unprepare. So, these are definitely new  
> APIs. If it weren't new APIs, then none of the general drivers would  
> need to change.

Yes and no.  I disagree that the meaning of clk_enable/disable() has
changed.  It hasn't.  What has changed is the preconditions for calling
those functions, and necessarily so in the interest of being able to
unify the different implementations.
diff mbox

Patch

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 2eaf17e..dd2d528 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -21,6 +21,11 @@  menuconfig COMMON_CLK
 	  this option for loadable modules requiring the common clock
 	  framework.
 
+	  The API and implementation enabled by this option is still
+	  incomplete.  The API and implementation is expected to be
+	  fluid and subject to change at any time, potentially
+	  breaking existing users.
+
 	  If in doubt, say "N".
 
 if COMMON_CLK