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

Reply via email to