* CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK @ 2019-12-12 2:09 Kuninori Morimoto 2019-12-12 10:57 ` Enrico Weigelt, metux IT consult 0 siblings, 1 reply; 4+ messages in thread From: Kuninori Morimoto @ 2019-12-12 2:09 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd; +Cc: linux-clk, linux-kernel Hi I noticed that there are some CONFIG_HAVE_CLK vs CONFIG_COMMON_CLK mismatch. Because of it, I got compile error at clk_set_min_rate() on SH. SH will have HAVE_CLK, but doesn't have COMMON_CLK. > ARCH=sh make allyesconfig > make ... drivers/devfreq/tegra30-devfreq.o: In function `tegra_devfreq_target': tegra30-devfreq.c:(.text+0x368): undefined reference to `clk_set_min_rate' clk_set_min_rate() is under HAVE_CLK at clk.h --- clk.h --- => #ifdef CONFIG_HAVE_CLK ... int clk_set_min_rate(struct clk *clk, unsigned long rate); ... #else /* !CONFIG_HAVE_CLK */ static inline int clk_set_min_rate(struct clk *clk, unsigned long rate) ... ------------- It is implemented at clk.c. But it will be compiled via COMMON_CLK --- Makefile --- ... => obj-$(CONFIG_COMMON_CLK) += clk.o ... ---------------- Thank you for your help !! Best regards --- Kuninori Morimoto ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK 2019-12-12 2:09 CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK Kuninori Morimoto @ 2019-12-12 10:57 ` Enrico Weigelt, metux IT consult 2019-12-12 21:51 ` Stephen Boyd 0 siblings, 1 reply; 4+ messages in thread From: Enrico Weigelt, metux IT consult @ 2019-12-12 10:57 UTC (permalink / raw) To: Kuninori Morimoto, Michael Turquette, Stephen Boyd Cc: linux-clk, linux-kernel On 12.12.19 03:09, Kuninori Morimoto wrote: > I noticed that there are some CONFIG_HAVE_CLK vs CONFIG_COMMON_CLK mismatch. > Because of it, I got compile error at clk_set_min_rate() on SH. > SH will have HAVE_CLK, but doesn't have COMMON_CLK. > > > ARCH=sh make allyesconfig > > make > ... > drivers/devfreq/tegra30-devfreq.o: In function `tegra_devfreq_target': > tegra30-devfreq.c:(.text+0x368): undefined reference to `clk_set_min_rate' > > clk_set_min_rate() is under HAVE_CLK at clk.h > > --- clk.h --- > => #ifdef CONFIG_HAVE_CLK > ... > int clk_set_min_rate(struct clk *clk, unsigned long rate); > ... > #else /* !CONFIG_HAVE_CLK */ > static inline int clk_set_min_rate(struct clk *clk, unsigned long rate) > ... > ------------- > > It is implemented at clk.c. > But it will be compiled via COMMON_CLK > > --- Makefile --- > ... > => obj-$(CONFIG_COMMON_CLK) += clk.o You've got CONFIG_HAVE_CLK enabled, but CONFIG_COMMON_CLK disabled ? hmm, the whole CONFIG_HAVE_CLK looks a bit weird to me. I wonder what's the actual purpose of having this arch-specific. IMHO, we should sort out whether there are some things that some arch really *needs*, and what could be optional - then split that into separate modules along this line. It seems that clk_set_min_rate() belongs to CONFIG_COMMON_CLK, and tegra30-devfreq.c needds to depend on CONFIG_COMMON_CLK. --mtx --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK 2019-12-12 10:57 ` Enrico Weigelt, metux IT consult @ 2019-12-12 21:51 ` Stephen Boyd 2019-12-13 4:41 ` Kuninori Morimoto 0 siblings, 1 reply; 4+ messages in thread From: Stephen Boyd @ 2019-12-12 21:51 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult, Kuninori Morimoto, Michael Turquette Cc: linux-clk, linux-kernel Quoting Enrico Weigelt, metux IT consult (2019-12-12 02:57:14) > On 12.12.19 03:09, Kuninori Morimoto wrote: > > > I noticed that there are some CONFIG_HAVE_CLK vs CONFIG_COMMON_CLK mismatch. > > Because of it, I got compile error at clk_set_min_rate() on SH. > > SH will have HAVE_CLK, but doesn't have COMMON_CLK. > > > > > ARCH=sh make allyesconfig > > > make > > ... > > drivers/devfreq/tegra30-devfreq.o: In function `tegra_devfreq_target': > > tegra30-devfreq.c:(.text+0x368): undefined reference to `clk_set_min_rate' > > > > clk_set_min_rate() is under HAVE_CLK at clk.h > > > > --- clk.h --- > > => #ifdef CONFIG_HAVE_CLK > > ... > > int clk_set_min_rate(struct clk *clk, unsigned long rate); > > ... > > #else /* !CONFIG_HAVE_CLK */ > > static inline int clk_set_min_rate(struct clk *clk, unsigned long rate) > > ... > > ------------- > > > > It is implemented at clk.c. > > But it will be compiled via COMMON_CLK > > > > --- Makefile --- > > ... > > => obj-$(CONFIG_COMMON_CLK) += clk.o > > You've got CONFIG_HAVE_CLK enabled, but CONFIG_COMMON_CLK disabled ? > > hmm, the whole CONFIG_HAVE_CLK looks a bit weird to me. I wonder what's > the actual purpose of having this arch-specific. > > IMHO, we should sort out whether there are some things that some arch > really *needs*, and what could be optional - then split that into > separate modules along this line. > > It seems that clk_set_min_rate() belongs to CONFIG_COMMON_CLK, and > tegra30-devfreq.c needds to depend on CONFIG_COMMON_CLK. > Years ago there wasn't a common clk framework. Just CONFIG_HAVE_CLK and architectures implementing the API defined in the clk.h header file. Then the common clk framework was created and we got CONFIG_COMMON_CLK. When new clk API features are added to the common clk framework, we typically limit their implementation and scope to CONFIG_COMMON_CLK so that architectures are encouraged to migrate to the common clk framework. I'm not really tracking the other implementations of the clk API, but I thought we were down to a handful of implementations that haven't migrated. I suppose SH is one of the big ones. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK 2019-12-12 21:51 ` Stephen Boyd @ 2019-12-13 4:41 ` Kuninori Morimoto 0 siblings, 0 replies; 4+ messages in thread From: Kuninori Morimoto @ 2019-12-13 4:41 UTC (permalink / raw) To: Stephen Boyd Cc: Enrico Weigelt, metux IT consult, Michael Turquette, linux-clk, linux-kernel Hi > > > --- clk.h --- > > > => #ifdef CONFIG_HAVE_CLK > > > ... > > > int clk_set_min_rate(struct clk *clk, unsigned long rate); > > > ... > > > #else /* !CONFIG_HAVE_CLK */ > > > static inline int clk_set_min_rate(struct clk *clk, unsigned long rate) > > > ... > > > ------------- (snip) > > > --- Makefile --- > > > ... > > > => obj-$(CONFIG_COMMON_CLK) += clk.o > > > > You've got CONFIG_HAVE_CLK enabled, but CONFIG_COMMON_CLK disabled ? > > > > hmm, the whole CONFIG_HAVE_CLK looks a bit weird to me. I wonder what's > > the actual purpose of having this arch-specific. > > > > IMHO, we should sort out whether there are some things that some arch > > really *needs*, and what could be optional - then split that into > > separate modules along this line. > > > > It seems that clk_set_min_rate() belongs to CONFIG_COMMON_CLK, and > > tegra30-devfreq.c needds to depend on CONFIG_COMMON_CLK. > > > > Years ago there wasn't a common clk framework. Just CONFIG_HAVE_CLK and > architectures implementing the API defined in the clk.h header file. > Then the common clk framework was created and we got CONFIG_COMMON_CLK. > When new clk API features are added to the common clk framework, we > typically limit their implementation and scope to CONFIG_COMMON_CLK so > that architectures are encouraged to migrate to the common clk > framework. I'm not really tracking the other implementations of the clk > API, but I thought we were down to a handful of implementations that > haven't migrated. I suppose SH is one of the big ones. I investigated about SH / HAVE_CLK / COMMON_CLK. In clk.h, some functions are defined under HAVE_CLK. For example clk_enable(). --- include/linux/clk.h --- ... => #ifdef CONFIG_HAVE_CLK ... int clk_set_rate(struct clk *clk, unsigned long rate); ... --------------------------- But, it is implementated under COMMON_CLK. --- drivers/clk/clk.c --- ... int clk_set_rate(struct clk *clk, unsigned long rate) ... --- drivers/clk/Makefiles --- ... => obj-$(CONFIG_COMMON_CLK) += clk.o ... ----------------------------- OTOH, SH has HAVE_CLK, but not have COMMON_CLK. And, it has own clock implementation at drivers/sh/clk/core.c. --- drivers/sh/clk/core.c --- ... int clk_set_rate(struct clk *clk, unsigned long rate) ... --- drivers/sh/clk/Makefile --- ... => obj-y := core.o ... ------------------------------- These mean, HAVE_CLK vs COMMON_CLK mismatch under clk.h is very matching to SH own clock. In other words, if we correct clk.h HAVE_CLK vs COMMON_CLK, It breaks SH clk. It is very confusable for me. But difficult to solve it. So far, I will add "depends on COMMON_CLK" to driver side. Thank you for your help !! Best regards --- Kuninori Morimoto ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-13 4:41 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-12 2:09 CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK Kuninori Morimoto 2019-12-12 10:57 ` Enrico Weigelt, metux IT consult 2019-12-12 21:51 ` Stephen Boyd 2019-12-13 4:41 ` Kuninori Morimoto
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).