On 11/03/15 17:09, Steven Kipisz wrote: > On 11/03/2015 07:29 AM, Igor Grinberg wrote: >> Hi Steve, >> >> On 11/03/15 14:22, Steve Kipisz wrote: >> >> [...] >> >>> Signed-off-by: Steve Kipisz <s-kipi...@ti.com> >>> --- >>> v2 Based on: >>> master a6104737 ARM: at91: sama5: change the environment address to >>> 0x6000 >>> >>> Build testing: MAKEALL -s omap4 -s omap5 (no warnings/build errors) >>> Boot Testing: >>> am57xx_evm_nodt_config: http://pastebin.ubuntu.com/13039296/ >>> beagle_x15_config: http://pastebin.ubuntu.com/13039331/ >>> >>> Changes in v2 (since v1): >>> - move the board detection code into the new routine >>> do_board_detect >>> - eliminate board.h and move the ix_xxx into board.c >>> - redo commit message to be more clear >>> >>> v1: http://marc.info/?t=144608007900002&r=1&w=2 >>> http://marc.info/?t=144608007900004&r=1&w=2 >>> (mailing list squashed original submission) >> >> [...] >> >>> +#define is_x15() board_am_is("BBRDX15_") >>> +#define is_am572x_evm() board_am_is("AM572PM_") >> >> I think board_is_* much more appropriate here... >> > Ok. so board_is_x15 and board_is_am572x_evm
Yep. Sounds better. >>> + >>> #ifdef CONFIG_DRIVER_TI_CPSW >>> #include <cpsw.h> >>> #endif >>> @@ -246,6 +249,54 @@ struct vcores_data beagle_x15_volts = { >>> .iva.pmic = &tps659038, >>> }; >>> >>> +#ifdef CONFIG_SPL_BUILD >>> +/* No env to setup for SPL */ >>> +static inline void setup_board_eeprom_env(void) { } >>> + >>> +/* Override function to read eeprom information */ >>> +void do_board_detect(void) >>> +{ >>> + struct ti_am_eeprom *ep; >>> + int rc; >>> + >>> + rc = ti_i2c_eeprom_am_get(CONFIG_EEPROM_BUS_ADDRESS, >>> + CONFIG_EEPROM_CHIP_ADDRESS, &ep); >>> + if (rc) >>> + printf("ti_i2c_eeprom_init failed %d\n", rc); >>> +} >> >> Do you really need this in SPL? > > Yes. We need to detect the board to determine DDR setup, pin mux, iodelay. > All of that needs to be done in SPL. X15 and EVM are the same, but more > boards will be added that have some differences. Ok. >> >>> + >>> +#else /* CONFIG_SPL_BUILD */ >>> + >>> +static void setup_board_eeprom_env(void) >>> +{ >>> + char *name = NULL; >> >> How about: >> >> char *name = "beagle_x15"; >> >>> + int rc; >>> + struct ti_am_eeprom_printable p; >>> + >>> + rc = ti_i2c_eeprom_am_get_print(CONFIG_EEPROM_BUS_ADDRESS, >>> + CONFIG_EEPROM_CHIP_ADDRESS, &p); >>> + if (rc) { >>> + printf("Invalid EEPROM data(@0x%p). Default to X15\n", >>> + TI_AM_EEPROM_DATA); >>> + goto invalid_eeprom; >>> + } >>> + >>> + if (is_x15()) >>> + name = "beagle_x15"; >> >> This will not be needed if the above comment is implemented. >> >>> + else if (is_am572x_evm()) >>> + name = "am57xx_evm"; >>> + else >>> + printf("Unidentified board claims %s in eeprom header\n", >>> + p.name); >>> + >>> +invalid_eeprom: >>> + set_board_info_env(name, "beagle_x15", p.version, p.serial); >> >> If the above comment is implemented, no more need for the >> default_name parameter... >> > Ok, I'll look at that. >>> +} >>> + >>> +/* Eeprom is alread read by SPL.. nothing more to do here.. */ >>> + >>> +#endif /* CONFIG_SPL_BUILD */ >> >> [...] >> >> > Steve K. > -- Regards, Igor. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot