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
