+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?
Regards, Simon