[PATCH] Revert "clocksource/drivers/sp804: Add COMPILE_TEST to CONFIG_ARM_TIMER_SP804"

[PATCH] Revert "clocksource/drivers/sp804: Add COMPILE_TEST to CONFIG_ARM_TIMER_SP804"

[Helmut Grohne]

This reverts commit dfc82faad72520769ca146f857e65c23632eed5a.

The commit effectively makes ARM_TIMER_SP804 depend on COMPILE_TEST,
which makes it unselectable for practical uses.

Link: https://lore.kernel.org/lkml/20190618120719.a4kgyiuljm5uivfq@laureti-dev/
Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>
---
 drivers/clocksource/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 5e9317dc3d39..72e924374591 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -393,7 +393,7 @@ config ARM_GLOBAL_TIMER
 	  This options enables support for the ARM global timer unit
 
 config ARM_TIMER_SP804
-	bool "Support for Dual Timer SP804 module" if COMPILE_TEST
+	bool "Support for Dual Timer SP804 module"
 	depends on GENERIC_SCHED_CLOCK && CLKDEV_LOOKUP
 	select CLKSRC_MMIO
 	select TIMER_OF if OF
-- 
2.11.0

Re: [PATCH] Revert "clocksource/drivers/sp804: Add COMPILE_TEST to CONFIG_ARM_TIMER_SP804"

[Thomas Gleixner]

On Thu, 15 Aug 2019, Helmut Grohne wrote:

> This reverts commit dfc82faad72520769ca146f857e65c23632eed5a.
> 
> The commit effectively makes ARM_TIMER_SP804 depend on COMPILE_TEST,
> which makes it unselectable for practical uses.
> 
> Link: https://lore.kernel.org/lkml/20190618120719.a4kgyiuljm5uivfq@laureti-dev/
> Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>
> ---
>  drivers/clocksource/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 5e9317dc3d39..72e924374591 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -393,7 +393,7 @@ config ARM_GLOBAL_TIMER
>  	  This options enables support for the ARM global timer unit
>  
>  config ARM_TIMER_SP804
> -	bool "Support for Dual Timer SP804 module" if COMPILE_TEST
> +	bool "Support for Dual Timer SP804 module"

The obvious fix is to add

    	depends on ARM || ARM64 || COMPILE_TEST

instead of reverting the whole thing. Care to do that?

Thanks,

	tglx

[PATCH] clocksource/drivers/sp804: make CONFIG_ARM_TIMER_SP804 selectable again

[Helmut Grohne]

Adding a dependency on CONFIG_COMPILE_TEST makes the relevant item
unselectable for practical purposes. The correct solution is to add a
dependency alternative on the relevant architecture.

Fixes: dfc82faad72520 ("clocksource/drivers/sp804: Add COMPILE_TEST to CONFIG_ARM_TIMER_SP804")
Link: https://lore.kernel.org/lkml/20190618120719.a4kgyiuljm5uivfq@laureti-dev/
Link: https://lore.kernel.org/lkml/alpine.DEB.2.21.1908152227590.1908@nanos.tec.linutronix.de/
Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>
---
 drivers/clocksource/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Hi Thomas,

On Thu, Aug 15, 2019 at 10:30:39PM +0200, Thomas Gleixner wrote:
> The obvious fix is to add
> 
>       depends on ARM || ARM64 || COMPILE_TEST
> 
> instead of reverting the whole thing. Care to do that?

Incidentally, that's what I proposed earlier as RFC. Resending that
variant now.

I also note that there are likely more instances for this pattern.
Should they be fixed in a similar way? You can find a lot using the
following incantation:

    $ git describe --tags
    v5.3-rc4
    $ git ls-files -- "*/Kconfig" | xargs git grep --cached 'bool .* if COMPILE_TEST$' -- | wc -l
    185
    $

Seems like an anti-pattern to me. It is particularly common in the
clocksource subtree.

Helmut

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 5e9317dc3d39..7081a250573b 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -393,7 +393,8 @@ config ARM_GLOBAL_TIMER
 	  This options enables support for the ARM global timer unit
 
 config ARM_TIMER_SP804
-	bool "Support for Dual Timer SP804 module" if COMPILE_TEST
+	bool "Support for Dual Timer SP804 module"
+	depends on ARM || ARM64 || COMPILE_TEST
 	depends on GENERIC_SCHED_CLOCK && CLKDEV_LOOKUP
 	select CLKSRC_MMIO
 	select TIMER_OF if OF
-- 
2.11.0

Re: [PATCH] clocksource/drivers/sp804: make CONFIG_ARM_TIMER_SP804 selectable again

[Thomas Gleixner]

Helmut,

On Fri, 16 Aug 2019, Helmut Grohne wrote:
> I also note that there are likely more instances for this pattern.
> Should they be fixed in a similar way? You can find a lot using the
> following incantation:
> 
>     $ git describe --tags
>     v5.3-rc4
>     $ git ls-files -- "*/Kconfig" | xargs git grep --cached 'bool .* if COMPILE_TEST$' -- | wc -l
>     185
>     $
> 
> Seems like an anti-pattern to me. It is particularly common in the
> clocksource subtree.

After some rumaging I figured out that the idea behind this is that the
platforms which need those clocksources use 'select $CLOCKSOURCE' which
works despite the 'if COMPILE_TEST'.

The 'if COMPILE_TEST' is there to hide the config option when there is no
platform which needs it and expose it for compile test purposes.

So if your particular platform does not use 'select ARM_TIMER_SP804' then
the right fix is to add it to that platform.

Thanks,

	tglx