[U-Boot] [PATCH V6 2/5] omap-common: add nand spl support

[Simon Schwarz]

Hi Aneesh,

<snip>
>> diff --git a/arch/arm/cpu/armv7/omap-common/spl.c
>> b/arch/arm/cpu/armv7/omap-common/spl.c
>> index d177652..7ec5c7c 100644
>> --- a/arch/arm/cpu/armv7/omap-common/spl.c
>> +++ b/arch/arm/cpu/armv7/omap-common/spl.c
>> @@ -26,6 +26,7 @@
>> #include<asm/u-boot.h>
>> #include<asm/utils.h>
>> #include<asm/arch/sys_proto.h>
>> +#include<nand.h>
>> #include<mmc.h>
>> #include<fat.h>
>> #include<timestamp_autogenerated.h>
>> @@ -107,6 +108,7 @@ static void parse_image_header(const struct
>> image_header *header)
>> }
>> }
>>
>> +#ifdef CONFIG_SPL_MMC_SUPPORT
>> static void mmc_load_image_raw(struct mmc *mmc)
>> {
>> u32 image_size_sectors, err;
>> @@ -140,7 +142,9 @@ end:
>> hang();
>> }
>> }
>> +#endif /* CONFIG_SPL_MMC_SUPPORT */
>
> here..
>
>>
>> +#ifdef CONFIG_SPL_MMC_SUPPORT
>> static void mmc_load_image_fat(struct mmc *mmc)
>> {
>> s32 err;
>> @@ -173,7 +177,9 @@ end:
>> hang();
>> }
>> }
>> +#endif /* CONFIG_SPL_MMC_SUPPORT */
>
> and here..
>
> You start the same the #ifdef again immediately after the #endif. Why
> don't you club them together into just one #ifdef block.
>
> Actually, since we have garbage collection of un-used functions, I
> think doing the calls under #ifdef should be enough, which you have
> taken care in board_init_r(). That may help to avoid some #ifdef
> clutter.

I have to dig out this mail again. I found that removing the ifdefs 
breaks OMAP4 since it lacks some NAND specific defines.

So I wonder - is it ok to define them to some garbage-value in the file 
and emit an #error if the corresponding LIB is set in SPL?

This would state in the file that these defines are used and it would 
avoid the need of defining them.

This would look like:
#ifndef CONFIG_SPL_NAND_BLA
#ifdef CONFIG_SPL_NAND_SUPPORT
#error CONFIG_SPL_NAND_BLA is not set although \
	CONFIG_SPL_NAND_SUPPORT is active
#else
#define CONFIG_SPL_NAND_BLA 0x0
#endif
#endif

Maybe this can be simplified by some macro...

I find it really annoying to set values for code I don't want to use...

Regards
Simon