+Tom for question below Hi Mario,
On 4 May 2018 at 02:04, Mario Six <[email protected]> wrote: > Hi Simon, > > On Thu, May 3, 2018 at 9:01 PM, Simon Glass <[email protected]> wrote: >> Hi Mario, >> >> On 27 April 2018 at 06:52, Mario Six <[email protected]> wrote: >>> Add a RAM driver for the MPC83xx architecture. >>> >>> Signed-off-by: Mario Six <[email protected]> >>> >>> --- >>> >>> 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 <[email protected]> >> >> 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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
