Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
Hi, On Thu, Oct 10, 2013 at 12:37 PM, Anatolij Gustschin ag...@denx.de wrote: On Thu, 10 Oct 2013 11:23:55 -0700 Brian Norris computersforpe...@gmail.com wrote: Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in arch/powerpc/sysdev/fsl_soc.h and building should work then. Still want it around this line, probably, so we'll get compiler errors and not linker errors if someone tries to use it? extern struct platform_diu_data_ops diu_ops; I was wrong about this: we need to remove the #ifdef for this extern as well, because the 'diu_ops' symbol is used in either case with my patch. Sending out my patch now. Brian ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
Hi,
On Wed, 9 Oct 2013 12:29:31 -0700
Brian Norris computersforpe...@gmail.com wrote:
...
+#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'?
making mpc512x_setup_diu(), mpc512x_release_bootmem(),
mpc512x_valid_monitor_port() and void mpc512x_set_pixel_clock()
should be okay.
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.
Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in
arch/powerpc/sysdev/fsl_soc.h and building should work then.
Thanks,
Anatolij
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
Hello,
On Thu, Oct 10, 2013 at 9:09 AM, Anatolij Gustschin ag...@denx.de wrote:
On Wed, 9 Oct 2013 12:29:31 -0700
Brian Norris computersforpe...@gmail.com wrote:
...
+#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'?
making mpc512x_setup_diu(), mpc512x_release_bootmem(),
mpc512x_valid_monitor_port() and void mpc512x_set_pixel_clock()
should be okay.
And mpc512x_init_diu()?
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.
Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in
arch/powerpc/sysdev/fsl_soc.h and building should work then.
Still want it around this line, probably, so we'll get compiler errors
and not linker errors if someone tries to use it?
extern struct platform_diu_data_ops diu_ops;
But otherwise that looks OK to me. Shall I send a patch?
Brian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
Hello, On Thu, 10 Oct 2013 11:23:55 -0700 Brian Norris computersforpe...@gmail.com wrote: ... making mpc512x_setup_diu(), mpc512x_release_bootmem(), mpc512x_valid_monitor_port() and void mpc512x_set_pixel_clock() should be okay. And mpc512x_init_diu()? yes, it can be static, too. 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. Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in arch/powerpc/sysdev/fsl_soc.h and building should work then. Still want it around this line, probably, so we'll get compiler errors and not linker errors if someone tries to use it? extern struct platform_diu_data_ops diu_ops; But otherwise that looks OK to me. Shall I send a patch? yes, please. Thanks, Anatolij ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
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 2a63c90ab55ca3f054772c2e5ba7df810c48
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 g...@denx.de
CC: sta...@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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1] powerpc/mpc512x: silence build warning upon disabled DIU
On Fri, 27 Sep 2013 17:28:38 +0200 Gerhard Sittig g...@denx.de 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 applied, thanks! ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v1] powerpc/mpc512x: silence build warning upon disabled DIU
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 2a63c90ab55ca3f054772c2e5ba7df810c48
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
Signed-off-by: Gerhard Sittig g...@denx.de
CC: sta...@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)
--
1.7.10.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
