[PATCH] clk.h: Fix shim ifdef guard (HAVE_CLK -> COMMON_CLK)

[PATCH] clk.h: Fix shim ifdef guard (HAVE_CLK -> COMMON_CLK)

[Jeff Mahoney]

Commit 93abe8e4 (clk: add non HAVE_CLK routines) added shims for
the clk code but HAVE_CLK isn't enough. It's possible to have the
clk support but not enable it. We end up with full prototypes for code
that is never built - causing module linking to fail later.

This patch changes the guard to use COMMON_CLK, which actually guards
the code.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 include/linux/clk.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -120,7 +120,7 @@ static inline void clk_unprepare(struct
 }
 #endif
 
-#ifdef CONFIG_HAVE_CLK
+#ifdef CONFIG_COMMON_CLK
 /**
  * clk_get - lookup and obtain a reference to a clock producer.
  * @dev: device for clock "consumer"
@@ -276,7 +276,7 @@ struct clk *clk_get_parent(struct clk *c
  */
 struct clk *clk_get_sys(const char *dev_id, const char *con_id);
 
-#else /* !CONFIG_HAVE_CLK */
+#else /* !CONFIG_COMMON_CLK */
 
 static inline struct clk *clk_get(struct device *dev, const char *id)
 {


-- 
Jeff Mahoney
SUSE Labs

Re: [PATCH] clk.h: Fix shim ifdef guard (HAVE_CLK -> COMMON_CLK)

[Russell King]

On Mon, Aug 27, 2012 at 11:28:15AM -0400, Jeff Mahoney wrote:
> Commit 93abe8e4 (clk: add non HAVE_CLK routines) added shims for
> the clk code but HAVE_CLK isn't enough. It's possible to have the
> clk support but not enable it. We end up with full prototypes for code
> that is never built - causing module linking to fail later.
> 
> This patch changes the guard to use COMMON_CLK, which actually guards
> the code.

This is wrong.  COMMON_CLK is an _implementation_ of the CLK API.  It
is not the only implementation in the kernel.  Conditionalizing like
this breaks existing users.

HAVE_CLK is the right thing here - if you define HAVE_CLK then you _are_
providing an implementation of clk_get() et.al.  If you're not, then you
do not define HAVE_CLK.  Simples.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH] clk.h: Fix shim ifdef guard (HAVE_CLK -> COMMON_CLK)

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 8/27/12 2:52 PM, Russell King wrote:
> On Mon, Aug 27, 2012 at 11:28:15AM -0400, Jeff Mahoney wrote:
>> Commit 93abe8e4 (clk: add non HAVE_CLK routines) added shims for 
>> the clk code but HAVE_CLK isn't enough. It's possible to have
>> the clk support but not enable it. We end up with full prototypes
>> for code that is never built - causing module linking to fail
>> later.
>> 
>> This patch changes the guard to use COMMON_CLK, which actually
>> guards the code.
> 
> This is wrong.  COMMON_CLK is an _implementation_ of the CLK API.
> It is not the only implementation in the kernel.  Conditionalizing
> like this breaks existing users.
> 
> HAVE_CLK is the right thing here - if you define HAVE_CLK then you
> _are_ providing an implementation of clk_get() et.al.  If you're
> not, then you do not define HAVE_CLK.  Simples.
> 

Ok. Thanks for the feedback.

It would seem, then, that the powerpc clock implementation doesn't
implement the full API and that's where we're running into problems.

Specifically, the use of clk_devm_get in
drivers/usb/chipidea/ci13xxx_imx.c and
drivers/char/hw_random/exynos-rng.ko is causing module link errors on
powerpc configs for the EFIKA hardware. The EXYNOS RNG driver should
probably have a dependency on ARCH_EXYNOS. The chipidea driver looks
like it was developed on ARM hardware - does it exist anywhere else?

Should the clk_devm stuff be moved someplace generic? I don't have a
horse in this race. My only interest here is to get these configs to
build again. :)

- -Jeff

- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.18 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJQO95XAAoJEB57S2MheeWy+QQQAK8HRdlhtyj3yy9S1zQ8c15x
72xwM52Yhhwl1L4OsuD4oNjzwGxj54j7VMfe9TMkrrozC7eySlnm3ErSnVNfn6Dp
RnUvjAs+Wr9vYR18f3IgFrYMnu2M3PkxBFciv/K94soCZ90iKuFs6yebqtkVUzP3
XfVmVdJpJpe+GXvGBW7lo8PfZNyLbxhleTqSXftTszuLcTlF8361aZmxz3UKpiWD
EB5a079s1CKf1KAu61P0drEIVHgAayffHjm4TDAE3AYzM7nwcvqjZQcjxSfI9eHF
iPOeS0Kt2+9TrLJjp9p/dZX3dLFHhG5LtS6qZq372JVjZggPGknAdV/MK33qEzuN
Fnr+ag5DeB/Kujfw2NcC+RQoayrMBK52eSxZO451xwHZef0+JP2eVLhht9zfmzDh
4cRj6gybKklEry/kABhy5O59iNVrjn+WrbVFwGO5aezZbAoqnZm2Sb5pU6aDLV6m
p3IMTfFMA13rh8sn/XstFN50T2xG4SP1DMqiV6AbfcfSXyRM0vQ9etOkfjp2ftq+
blbjwNBVp8Hp47q04LIw1XPxDMOEpBX6bVFiIqOWT9GvhvFysetNpB6Ae00FIo7p
1Zdv0Bq9IkWhZ+BxLeqgMUO445GIqUk7k2AElI8G3cCIKZlNp5Od/hFx7rQJ6Gwq
dqydYWi1ogVqg3YpSI7a
=/UyR
-----END PGP SIGNATURE-----

Re: [PATCH] clk.h: Fix shim ifdef guard (HAVE_CLK -> COMMON_CLK)

[Russell King]

On Mon, Aug 27, 2012 at 04:53:44PM -0400, Jeff Mahoney wrote:
> Should the clk_devm stuff be moved someplace generic? I don't have a
> horse in this race. My only interest here is to get these configs to
> build again. :)

devm_clk_get() should not be in clkdev.c, but should be entirely separate,
allowing anyone who is using the clk API to use it.  But unfortunately,
that's not how it ended up getting added...  Moving it is something that's
on my very long todo list.

It should still remain in drivers/clk, but it should be in a separate file,
probably built conditionally by HAVE_CLK being defined.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH] clk.h: Fix shim ifdef guard (HAVE_CLK -> COMMON_CLK)

[Jeff Mahoney]

Ugh. Ignore this one. I tested a build but just started getting build failures on different configs. I'll look into why later this afternoon.

-Jeff

--
Jeff Mahoney
(apologies for the top post -- from my mobile)

On Aug 27, 2012, at 11:27 AM, "Jeff Mahoney<jeffm@suse.com>" <jeffm@suse.com> wrote:

> Commit 93abe8e4 (clk: add non HAVE_CLK routines) added shims for
> the clk code but HAVE_CLK isn't enough. It's possible to have the
> clk support but not enable it. We end up with full prototypes for code
> that is never built - causing module linking to fail later.
> 
> This patch changes the guard to use COMMON_CLK, which actually guards
> the code.
> 
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> ---
> include/linux/clk.h |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -120,7 +120,7 @@ static inline void clk_unprepare(struct
> }
> #endif
> 
> -#ifdef CONFIG_HAVE_CLK
> +#ifdef CONFIG_COMMON_CLK
> /**
>  * clk_get - lookup and obtain a reference to a clock producer.
>  * @dev: device for clock "consumer"
> @@ -276,7 +276,7 @@ struct clk *clk_get_parent(struct clk *c
>  */
> struct clk *clk_get_sys(const char *dev_id, const char *con_id);
> 
> -#else /* !CONFIG_HAVE_CLK */
> +#else /* !CONFIG_COMMON_CLK */
> 
> static inline struct clk *clk_get(struct device *dev, const char *id)
> {
> 
> 
> -- 
> Jeff Mahoney
> SUSE Labs