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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot