[PATCH] clk: Make clock framework user selectable

[PATCH] clk: Make clock framework user selectable

[Mark Brown]

Some architectures like x86 don't use the clock API in architecture code
but one of the advantages of the common clock API is that it can be used
by regular devices to represent their clocking structures so we should
allow users to enable the clock API themselves even if the architecture
doesn't need it. This is more straightforward than going round every
architecture and convincing their maintainers to add the select.

Unfortunately some architectures do still provide a custom implementation
of the clock API which is incompatible with common clock. Provide a config
option HAS_CUSTOM_CLK which these architectures can select in order to
prevent COMMON_CLK being enabled.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/clk/Kconfig | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index e2d9bd760c84..08a0b2cd49c6 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -7,7 +7,8 @@ config HAVE_CLK_PREPARE
 	bool
 
 config COMMON_CLK
-	bool
+	bool "Common Clock Framework Support"
+	depends on !HAVE_CUSTOM_CLK
 	select HAVE_CLK_PREPARE
 	select CLKDEV_LOOKUP
 	select SRCU
@@ -19,6 +20,9 @@ config COMMON_CLK
 	  Architectures utilizing the common struct clk should select
 	  this option.
 
+config HAVE_CUSTOM_CLK
+	bool
+
 menu "Common Clock Framework"
 	depends on COMMON_CLK
 
-- 
2.8.1

Re: [PATCH] clk: Make clock framework user selectable

[Geert Uytterhoeven]

On Thu, Aug 11, 2016 at 1:30 PM, Mark Brown <broonie@kernel.org> wrote:
> Some architectures like x86 don't use the clock API in architecture code
> but one of the advantages of the common clock API is that it can be used
> by regular devices to represent their clocking structures so we should
> allow users to enable the clock API themselves even if the architecture
> doesn't need it. This is more straightforward than going round every
> architecture and convincing their maintainers to add the select.
>
> Unfortunately some architectures do still provide a custom implementation
> of the clock API which is incompatible with common clock. Provide a config
> option HAS_CUSTOM_CLK which these architectures can select in order to
> prevent COMMON_CLK being enabled.

The list is not that long anymore, so it looks fixable with not too much
effort:

$ git grep '\*clk_get\>'
arch/avr32/mach-at32ap/clock.c:struct clk *clk_get(struct device *dev,
const char *id)
arch/m68k/coldfire/clk.c:struct clk *clk_get(struct device *dev, const char *id)
arch/mips/ar7/clock.c:struct clk *clk_get(struct device *dev, const char *id)
arch/mips/bcm63xx/clk.c:struct clk *clk_get(struct device *dev, const char *id)
arch/mips/loongson64/lemote-2f/clock.c:struct clk *clk_get(struct
device *dev, const char *id)
arch/mips/txx9/generic/setup.c:struct clk *clk_get(struct device *dev,
const char *id)
arch/unicore32/kernel/clock.c:struct clk *clk_get(struct device *dev,
const char *id)
drivers/clk/clkdev.c:struct clk *clk_get(struct device *dev, const char *con_id)

> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -7,7 +7,8 @@ config HAVE_CLK_PREPARE
>         bool
>
>  config COMMON_CLK
> -       bool
> +       bool "Common Clock Framework Support"
> +       depends on !HAVE_CUSTOM_CLK
>         select HAVE_CLK_PREPARE
>         select CLKDEV_LOOKUP
>         select SRCU
> @@ -19,6 +20,9 @@ config COMMON_CLK
>           Architectures utilizing the common struct clk should select
>           this option.
>
> +config HAVE_CUSTOM_CLK

HAVE_LEGACY_CLK, to give a better clue?

> +       bool
> +
>  menu "Common Clock Framework"
>         depends on COMMON_CLK

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] clk: Make clock framework user selectable

[Geert Uytterhoeven]

On Thu, Aug 11, 2016 at 1:50 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Aug 11, 2016 at 1:30 PM, Mark Brown <broonie@kernel.org> wrote:
>> Some architectures like x86 don't use the clock API in architecture code
>> but one of the advantages of the common clock API is that it can be used
>> by regular devices to represent their clocking structures so we should
>> allow users to enable the clock API themselves even if the architecture
>> doesn't need it. This is more straightforward than going round every
>> architecture and convincing their maintainers to add the select.
>>
>> Unfortunately some architectures do still provide a custom implementation
>> of the clock API which is incompatible with common clock. Provide a config
>> option HAS_CUSTOM_CLK which these architectures can select in order to
>> prevent COMMON_CLK being enabled.
>
> The list is not that long anymore, so it looks fixable with not too much
> effort:
>
> $ git grep '\*clk_get\>'
> arch/avr32/mach-at32ap/clock.c:struct clk *clk_get(struct device *dev,
> const char *id)
> arch/m68k/coldfire/clk.c:struct clk *clk_get(struct device *dev, const char *id)
> arch/mips/ar7/clock.c:struct clk *clk_get(struct device *dev, const char *id)
> arch/mips/bcm63xx/clk.c:struct clk *clk_get(struct device *dev, const char *id)
> arch/mips/loongson64/lemote-2f/clock.c:struct clk *clk_get(struct
> device *dev, const char *id)
> arch/mips/txx9/generic/setup.c:struct clk *clk_get(struct device *dev,
> const char *id)
> arch/unicore32/kernel/clock.c:struct clk *clk_get(struct device *dev,
> const char *id)
> drivers/clk/clkdev.c:struct clk *clk_get(struct device *dev, const char *con_id)

Having your own clk_get() doesn't seem to be sufficient. Found a few more:

$ git grep 'long clk_get_rate\>'arch/arm/mach-davinci/clock.c:unsigned
long clk_get_rate(struct clk *clk)
arch/arm/mach-ep93xx/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/arm/mach-mmp/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/arm/mach-omap1/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/arm/mach-sa1100/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/arm/mach-w90x900/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/avr32/mach-at32ap/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/blackfin/mach-bf609/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/c6x/platforms/pll.c:unsigned long clk_get_rate(struct clk *clk)
arch/m68k/coldfire/clk.c:unsigned long clk_get_rate(struct clk *clk)
arch/mips/ar7/clock.c:unsigned long clk_get_rate(struct clk *clk)
arch/mips/bcm63xx/clk.c:unsigned long clk_get_rate(struct clk *clk)
arch/mips/lantiq/clk.c:unsigned long clk_get_rate(struct clk *clk)
arch/mips/loongson64/lemote-2f/clock.c:unsigned long clk_get_rate(struct clk *cl
arch/mips/ralink/clk.c:unsigned long clk_get_rate(struct clk *clk)
arch/mips/txx9/generic/setup.c:unsigned long clk_get_rate(struct clk *clk)
arch/unicore32/kernel/clock.c:unsigned long clk_get_rate(struct clk *clk)
drivers/clk/clk.c:unsigned long clk_get_rate(struct clk *clk)
drivers/sh/clk/core.c:unsigned long clk_get_rate(struct clk *clk)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] clk: Make clock framework user selectable

[Mark Brown]

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

On Thu, Aug 11, 2016 at 01:50:08PM +0200, Geert Uytterhoeven wrote:
> On Thu, Aug 11, 2016 at 1:30 PM, Mark Brown <broonie@kernel.org> wrote:

> > Unfortunately some architectures do still provide a custom implementation
> > of the clock API which is incompatible with common clock. Provide a config
> > option HAS_CUSTOM_CLK which these architectures can select in order to
> > prevent COMMON_CLK being enabled.

> The list is not that long anymore, so it looks fixable with not too much
> effort:

Yeah, it's probably a good project for someone.  TBH my main goal here
was to stop x86 blocking use of the clock API.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] clk: Make clock framework user selectable

[Stephen Boyd]

On 08/11, Mark Brown wrote:
> Some architectures like x86 don't use the clock API in architecture code
> but one of the advantages of the common clock API is that it can be used
> by regular devices to represent their clocking structures so we should
> allow users to enable the clock API themselves even if the architecture
> doesn't need it. This is more straightforward than going round every
> architecture and convincing their maintainers to add the select.
> 
> Unfortunately some architectures do still provide a custom implementation
> of the clock API which is incompatible with common clock. Provide a config
> option HAS_CUSTOM_CLK which these architectures can select in order to

s/HAS/HAVE/

> prevent COMMON_CLK being enabled.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Concept seems fine.

> ---
>  drivers/clk/Kconfig | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index e2d9bd760c84..08a0b2cd49c6 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -7,7 +7,8 @@ config HAVE_CLK_PREPARE
>  	bool
>  
>  config COMMON_CLK
> -	bool
> +	bool "Common Clock Framework Support"
> +	depends on !HAVE_CUSTOM_CLK

Is the plan that architectures will keep selecting this when they
want to enforce common clk framework on their arch? We could move
enabling this config to the defconfigs if we make of_clk_init() a
stub when this config is disabled. Interestingly arm already has
that ifdefed out so nothing fails to compile there, but not every
arch is following that design.

Also, are there patches for all the users to select
HAVE_CUSTOM_CLK? Otherwise we can get randconfig failures when
two places define clk API functions.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: Make clock framework user selectable

[Mark Brown]

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

On Mon, Aug 15, 2016 at 03:06:10PM -0700, Stephen Boyd wrote:
> On 08/11, Mark Brown wrote:

> >  config COMMON_CLK
> > -	bool
> > +	bool "Common Clock Framework Support"
> > +	depends on !HAVE_CUSTOM_CLK

> Is the plan that architectures will keep selecting this when they
> want to enforce common clk framework on their arch? We could move

I think that's a per-architecture thing - if the architecture doesn't
need control of clocks then there's no reason for it to force the API
on, but if the architecture is unlikely to do anything useful without
then it's a different story.

> enabling this config to the defconfigs if we make of_clk_init() a
> stub when this config is disabled. Interestingly arm already has
> that ifdefed out so nothing fails to compile there, but not every
> arch is following that design.

That's probably a good idea anyway.

> Also, are there patches for all the users to select
> HAVE_CUSTOM_CLK? Otherwise we can get randconfig failures when
> two places define clk API functions.

They're trivial to make if you apply this.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]