On Fri, May 04, 2018 at 09:37:54PM +0000, Simon Glass wrote:
+Tom for question below
Hi Mario,
On 4 May 2018 at 02:04, Mario Six mario.six@gdsys.cc wrote:
Hi Simon,
On Thu, May 3, 2018 at 9:01 PM, Simon Glass sjg@chromium.org wrote:
Hi Mario,
On 27 April 2018 at 06:52, Mario Six mario.six@gdsys.cc wrote:
Add a RAM driver for the MPC83xx architecture.
Signed-off-by: Mario Six mario.six@gdsys.cc
v1 -> v2: No changes
arch/powerpc/cpu/mpc83xx/spd_sdram.c | 4 + drivers/ram/Kconfig | 8 + drivers/ram/Makefile | 1 + drivers/ram/mpc83xx_sdram.c | 948
+++++++++++++++++++++++++++++
include/dt-bindings/memory/mpc83xx-sdram.h | 143 +++++ include/mpc83xx.h | 6 + 6 files changed, 1110 insertions(+) create mode 100644 drivers/ram/mpc83xx_sdram.c create mode 100644 include/dt-bindings/memory/mpc83xx-sdram.h
Reviewed-by: Simon Glass sjg@chromium.org
Question below
[..]
+phys_size_t get_effective_memsize(void) +{ +#ifndef CONFIG_VERY_BIG_RAM
Can this (and the #ifdefs below in this file) be converted to
if (IS_ENABLED(CONFIG_...))
instead, to increase build coverage?
Yes, no problem, will convert for v3.
By the way, is this a practice that's generally encouraged, or is it just useful in special cases such as this one?
I think it is better in most cases as I don't really like #ifdefs in the code when they are easy to remove:
- visually confusing particularly where there is more than one term in the
#if
- creates multiple builds of the code, reducing build coverage for sandbox
- can sometimes be replaced with empty static inline functions, or even
build up logic (e.g. of_live_active() and its callers)
Tom, do you have any thoughts on this one?
Can this even be built for sandbox? If yes, we should do what we can to increase build coverage (and coverity coverage). Otherwise I don't think that if (CONFIG_IS_ENABLED(FOO)) is always better. If we're talking about: #ifdef FOO if (bar) { ... } #endif
Then yes, testing for CONFIG_IS_ENABLED(FOO) && bar is great. Or: #ifdef FOO ... a few lines ... #endif
It's also good. But if we're talking about a lot of lines, I think the added indent doesn't help and can start making it a bit harder with additional wrap.