[v1] powerpc/mpc512x: silence build warning upon disabled DIU

Brian Norris computersforpeace at gmail.com
Thu Oct 10 06:29:31 EST 2013


Hi all,

(Please keep me on the CC list, as I'm not subscribed)

On Fri, Sep 27, 2013 at 05:28:38PM +0200, Gerhard Sittig wrote:
> a disabled Kconfig option results in a reference to a not implemented
> routine when the IS_ENABLED() macro is used for both conditional
> implementation of the routine as well as a C language source code test
> at the call site -- the "if (0) func();" construct only gets eliminated
> later by the optimizer, while the compiler already has emitted its
> warning about "func()" being undeclared
> 
> provide an empty implementation for the mpc512x_setup_diu() and
> mpc512x_init_diu() routines in case of the disabled option, to avoid the
> compiler warning which is considered fatal and breaks compilation
> 
> the bug appeared with commit 2abbbb63c90ab55ca3f054772c2e5ba7df810c48
> "powerpc/mpc512x: move common code to shared.c file", how to reproduce:
> 
>   make mpc512x_defconfig
>   echo CONFIG_FB_FSL_DIU=n >> .config && make olddefconfig
>   make
> 
>     CC      arch/powerpc/platforms/512x/mpc512x_shared.o
>   .../arch/powerpc/platforms/512x/mpc512x_shared.c: In function 'mpc512x_init_early':
>   .../arch/powerpc/platforms/512x/mpc512x_shared.c:456:3: error: implicit declaration of function 'mpc512x_init_diu' [-Werror=implicit-function-declaration]
>   .../arch/powerpc/platforms/512x/mpc512x_shared.c: In function 'mpc512x_setup_arch':
>   .../arch/powerpc/platforms/512x/mpc512x_shared.c:469:3: error: implicit declaration of function 'mpc512x_setup_diu' [-Werror=implicit-function-declaration]
>   cc1: all warnings being treated as errors
>   make[4]: *** [arch/powerpc/platforms/512x/mpc512x_shared.o] Error 1

I just ran across this same compile issue, and I have a few thoughts
below.

> Signed-off-by: Gerhard Sittig <gsi at denx.de>
> CC: <stable at vger.kernel.org> # v3.11
> 
> ---
> arch/powerpc/platforms/512x/mpc512x_shared.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> index a82a41b..1a7b1d0 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -303,6 +303,9 @@ void __init mpc512x_setup_diu(void)
>  	diu_ops.release_bootmem		= mpc512x_release_bootmem;
>  }
>  
> +#else
> +void __init mpc512x_setup_diu(void) { /* EMPTY */ }
> +void __init mpc512x_init_diu(void) { /* EMPTY */ }
>  #endif
>  
>  void __init mpc512x_init_IRQ(void)

I see an alternative solution:

Can't almost all of the code in mpc512x_shared.c be declared 'static'?
Then, you can get the real benefit of IS_ENABLED() by removing the

    #if IS_ENABLED(CONFIG_FB_FSL_DIU)

from around all the DIU code, and it will automatically be removed by
the compiler when it is not used.

I think the current patch is necessary for immediate use, and it can be
sent to stable. But I might suggest a follow-up patch or 2 that makes
the functions static and kills the #ifdef entirely.

Thanks,
Brian


More information about the Linuxppc-dev mailing list