[U-Boot] [PATCH 1/1] Changes for single binary image for u-boot for NAND/OneNAND flash.
Nishanth Menon
menon.nishanth at gmail.com
Wed Mar 4 08:25:18 CET 2009
Pillai, Manikandan said the following on 03/04/2009 08:35 AM:
>>> #if defined(CONFIG_ENV_IS_IN_NAND) /* Environment is in Nand
>>>
>> Flash */ \
>>
>>> - || defined(CONFIG_ENV_IS_IN_SPI_FLASH)
>>> + || defined(CONFIG_ENV_IS_IN_SPI_FLASH) \
>>> + || (defined(CONFIG_CMD_NAND) && defined(CONFIG_ENV_IS_RUNTIME_SEL))
>>>
>>>
>> Errr.... ENV_IS_IN_NAND Vs ENV_IS_RUNTIME_SEL is not clear.
>>
> [Pillai, Manikandan] I am not clear with the query
>
>
CONFIG_ENV_IS_RUNTIME_SEL is dependent only on CMD_NAND? if I had onenand and NOR, then what?
>>> +void print_board_info(void)
>>> +{
>>> + u32 btype;
>>> +
>>> + btype = get_board_type();
>>> +
>>> + display_board_info(btype);
>>> +}
>>> +
>>>
>>>
>> I dont think this is related to this...
>>
> [Pillai, Manikandan] The default EVM support does not have NAND. To build only
> For NAND, you need to enable this. I can put this in a separate patch in a series.
>
>
my comment -> move this as a seperate patch.. this patch seems to mix up
things doing different things and confusing for a review.
>>> -#if defined(CONFIG_CMD_ONENAND)
>>> +#if defined(CONFIG_CMD_NAND) && defined(CONFIG_ENV_IS_RUNTIME_SEL)
>>> + gpmc_config = gpmc_m_nand;
>>> + nand_cs_base = (gpmc_csx_t *)(GPMC_CONFIG_CS0_BASE +
>>> + (gpmc_index * GPMC_CONFIG_WIDTH));
>>> + base = PISMO1_NAND_BASE;
>>> + size = PISMO1_NAND_SIZE;
>>> + enable_gpmc_config(gpmc_config, nand_cs_base, base, size);
>>> + /* NAND and/or ONENAND is to be scanned */
>>> + is_nand = 0;
>>> + nand_init();
>>> + if (nand_info[0].size) {
>>> + is_nand = 1;
>>> + f_off = SMNAND_ENV_OFFSET;
>>> + f_sec = SZ_128K;
>>> + /* env setup */
>>> + boot_flash_base = base;
>>> + boot_flash_off = f_off;
>>> + boot_flash_sec = f_sec;
>>> + boot_flash_env_addr = f_off;
>>> +
>>> + env_name_spec = nand_env_name_spec;
>>> + env_ptr = nand_env_ptr;
>>> + env_get_char_spec = nand_env_get_char_spec;
>>> + env_init = nand_env_init;
>>> + saveenv = nand_saveenv;
>>> + env_relocate_spec = nand_env_relocate_spec;
>>> + gpmc_index++;
>>> + }
>>>
>>>
>> with a change like above in a common omap3 file, you are essentially
>> bottlenecking scalability to other OMAP3 platforms which use different
>> NAND. Eg. we assume SZ_128K ;) kinda wrong rt? sector size is dependent
>> on the nand device we plug in..
>>
> [Pillai, Manikandan] I can plug out the initialization stuff and put it in
> a board dependent file and invoke the same from the common omap3 locations
> for the type of board.
>
>
that might be a nice idea.. will look for your changes.. :)
>>> printf("OMAP%s-%s rev %d, CPU-OPP2 L3-165MHz\n", cpu_s,
>>> - sec_s, get_cpu_rev());
>>> - printf("%s + %s/%s\n", sysinfo.board_string,
>>> - mem_s, sysinfo.nand_string);
>>> + sec_s, get_cpu_rev());
>>> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
>>> + printf("%s + %s/", sysinfo.board_string,
>>> + mem_s);
>>> +#if defined(CONFIG_CMD_NAND)
>>> + if (is_nand)
>>> + printf("%s\n", "NAND");
>>> +#endif
>>> +#if defined(CONFIG_CMD_ONENAND)
>>> + if (is_onenand)
>>> + printf("%s\n", "ONENAND");
>>> +#endif
>>> +#else
>>> + printf("%s + %s/%s\n", sysinfo.board_string,
>>> + mem_s, sysinfo.nand_string);
>>> +#endif
>>>
>>>
>> I have this feel that we could improve this #ifdef mess.. using
>> variables maybe?
>>
> [Pillai, Manikandan] other suggestions welcome Personally, I felt
> here the #ifdef is not so dirty.
>
;) not after a time of adding multiple #ifdefs :D... as I said, how
about using variables to do it?
>>> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
>>> + gpmc_init(); /* in SRAM or SDRAM, finish GPMC */
>>>
>>>
>> Wooooaaah.. gpmc is TI OMAP2 or OMAP3 only.... NOT in other ARM or PPC
>> or other platforms.. NAK on this.
>>
> [Pillai, Manikandan] Got your point. But I don't have a solution to this
> Since I EVM is doing a scan, it requires the gpmc_init to be called late.
> An option is to have another function board_init_late() which can be used to
> called gpmc_init_late().
>
that could be an option.. but please keep other platforms in mind when
we send this patch.
Regards,
Nishanth Menon
More information about the U-Boot
mailing list