linux-kernel - Re: CONFIG_COMMON_CLK vs CONFIG_HAVE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 Dec 2019 13:51:13 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     "Enrico Weigelt, metux IT consult" <lkml@...ux.net>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Michael Turquette <mturquette@...libre.com>
Cc:     linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: CONFIG_COMMON_CLK vs CONFIG_HAVE_CLK

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.