lkml.org 
[lkml]   [2019]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK

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

\
 
 \ /
  Last update: 2019-12-13 05:42    [W:0.770 / U:0.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site