From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Fri, 04 May 2018 21:37:54 +0000 Subject: [U-Boot] [PATCH v2 01/10] ram: Add driver for MPC83xx In-Reply-To: References: <20180427125227.1026-1-mario.six@gdsys.cc> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de +Tom for question below Hi Mario, On 4 May 2018 at 02:04, Mario Six wrote: > Hi Simon, > > On Thu, May 3, 2018 at 9:01 PM, Simon Glass wrote: >> Hi Mario, >> >> On 27 April 2018 at 06:52, Mario Six wrote: >>> Add a RAM driver for the MPC83xx architecture. >>> >>> Signed-off-by: Mario Six >>> >>> --- >>> >>> 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 >> >> 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