| Message ID | 1470915049-15249-1-git-send-email-broonie@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Stephen Boyd |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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.
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.
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
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(-)