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

Hi

On Fri, 16 Mar 2012, Arnd Bergmann wrote:

> On Friday 16 March 2012, Arnd Bergmann wrote:
> > > 
> > > Can we shoe-horn this thing into 3.4 (it is a bit late, i know) so
> > > that platform ports can gather speed? Several people are waiting for a
> > > somewhat stable version before starting their ports.
> > > 
> > > And what is the path into mainline - will Russell carry it or Arnd
> > > (with Russell's blessing)?
> > 
> > I would suggest putting it into a late/* branch of arm-soc so it finally
> > gets some linux-next exposure, and then sending it in the second week of the
> > merge window.
> > 
> > Russell, please let me know if you want to carry it instead or if you
> > have found any last-minute show stoppers in the code.
> 
> FWIW, it's in arm-soc now [...]

If the common clock code is to go upstream now, it should be marked as 
experimental.  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).

While I understand and accept the motivation to get the common clk code 
upstream now, if platforms start to use it, they need to be aware that the 
behavior of the code can change significantly.  These platforms may need 
to revalidate their implementations or change the way that many of their 
drivers use the clock functions.

It also seems important to recognize that significant changes are still 
coming into the common clk code (e.g., the clk_set_rate() changes that 
came in just a few days ago).

So, the following is a patch to mark it as experimental.  It applies on 
the current head of arm-soc/next/clk 
(9d9f78ed9af0e465d2fd15550471956e7f559b9f).  This should be a good 
compromise: it allows simple platforms or platforms with maintainers with 
lots of time to start converting over to the common clk code, while still 
clearly indicating that the API and underlying code is likely to change in 
significant ways.

Once at least two major SoC ports have DVFS with external I/O devices 
safely up and running on their mainline kernels with the common clk code, 
I'd suggest removing the experimental designation.

...

None of this is meant to reflect on Mike, by the way, who is doing a good 
job in a difficult situation.

 
- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Fri, 16 Mar 2012 16:06:30 -0600
Subject: [PATCH] clk: mark the common clk code as EXPERIMENTAL for now

Mark the common clk code as depending on CONFIG_EXPERIMENTAL.  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 EXPERIMENTAL designation 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 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

On Fri, Mar 16, 2012 at 3:21 PM, Paul Walmsley <paul@pwsan.com> wrote:
> From: Paul Walmsley <paul@pwsan.com>
> Date: Fri, 16 Mar 2012 16:06:30 -0600
> Subject: [PATCH] clk: mark the common clk code as EXPERIMENTAL for now
>
> Mark the common clk code as depending on CONFIG_EXPERIMENTAL.  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 EXPERIMENTAL designation 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>

ACK.  This will set some reasonable expectations while things are in flux.

Arnd are you willing to take this in?

Thanks,
Mike

> ---
>  drivers/clk/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 2eaf17e..a0a83de 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -12,6 +12,7 @@ config HAVE_MACH_CLKDEV
>  menuconfig COMMON_CLK
>        bool "Common Clock Framework"
>        select HAVE_CLK_PREPARE
> +       depends on EXPERIMENTAL
>        ---help---
>          The common clock framework is a single definition of struct
>          clk, useful across many platforms, as well as an
> --
> 1.7.9.1
>

Hi Paul,

On Fri, Mar 16, 2012 at 04:21:17PM -0600, Paul Walmsley wrote:
> Hi
> 
> On Fri, 16 Mar 2012, Arnd Bergmann 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)

> 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. 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.

Sascha

On 03/16/2012 06:47 PM, Sascha Hauer wrote:
> Hi Paul,
> 
> On Fri, Mar 16, 2012 at 04:21:17PM -0600, Paul Walmsley wrote:
>> Hi
>>
>> On Fri, 16 Mar 2012, Arnd Bergmann 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)
> 

+1

For simple users at least, the api is perfectly adequate and it is not
experimental (unless new means experimental).

Rob

>> 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. 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.
> 
> Sascha
>