[U-Boot] [u-boot PATCH v3 7/8] ARM: k2g: setup PRU ethernet MAC addresses

[Roger Quadros]

Hi,

On 08/02/17 13:51, Igor Grinberg wrote:
> Hi Roger,
> 
> On 02/08/17 10:51, Roger Quadros wrote:
>> Hi Igor,
>>
>> On 07/02/17 09:52, Igor Grinberg wrote:
>>> Hi Roger,
>>>
>>> On 02/06/17 11:36, Roger Quadros wrote:
>>>> PRU ethernet MAC address range is present in the
>>>> board EEPROM. Parse it and setup eth?addr
>>>> environment variables.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq at ti.com>
>>>> Reviewed-by: Lokesh Vutla <lokeshvutla at ti.com>
>>>> ---
>>>>  board/ti/ks2_evm/board_k2g.c | 19 +++++++++++++++++++
>>>>  1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/board/ti/ks2_evm/board_k2g.c b/board/ti/ks2_evm/board_k2g.c
>>>> index 40edbaa..a738dd2 100644
>>>> --- a/board/ti/ks2_evm/board_k2g.c
>>>> +++ b/board/ti/ks2_evm/board_k2g.c
>>>> @@ -12,6 +12,7 @@
>>>>  #include <asm/arch/psc_defs.h>
>>>>  #include <asm/arch/mmc_host_def.h>
>>>>  #include "mux-k2g.h"
>>>> +#include "../common/board_detect.h"
>>>>  
>>>>  #define SYS_CLK		24000000
>>>>  
>>>> @@ -149,6 +150,24 @@ int board_early_init_f(void)
>>>>  }
>>>>  #endif
>>>>  
>>>> +#ifdef CONFIG_BOARD_LATE_INIT
>>>> +int board_late_init(void)
>>>> +{
>>>> +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_TI_I2C_BOARD_DETECT)
>>>> +	int rc;
>>>> +
>>>> +	rc = ti_i2c_eeprom_am_get(CONFIG_EEPROM_BUS_ADDRESS,
>>>> +			CONFIG_EEPROM_CHIP_ADDRESS);
>>>> +	if (rc)
>>>> +		printf("ti_i2c_eeprom_init failed %d\n", rc);
>>>> +
>>>> +	board_ti_set_ethaddr(1);
>>>
>>> What if the MAC address has already been set in the environment?
>>
>> by whom?
>> At least as of now nobody is setting ethadddr1 on the k2g-ice board.
> 
> Well, for example by user... and it is eth1addr.

OK, I understand now.
> 
>>
>>> AFAIR, the MAC address in the environment has a higher precedence
>>> than others.
>>> May be I missed this, but I don't remember any discussion about changing
>>> this assumption.
>>> So, if the assumption is still correct, you shouldn't change the MAC in the env.
>>
>> I agree with you. I saw Olliver's "Retrieve MAC address from EEPROM" series.
>> However, that may not apply to TI boards yet because:
>> -the SoC's ethernet devices MAC addresses are stored in the SoC registers.
>> -the PRU ethernet MAC addresses which this patch is setting are stored in
>> EEPROM but they are not used at u-boot at all.
>>
>> I'm open to ideas if we can do what we're doing in a better way.
> 
> I think Tom's idea of a common function or may be a change to
> eth_setenv_enetaddr_*() functions that will handle the precedence
> is very sensible for such cases. 

Yes, this looks fine to me. Maybe there should be 2 functions? One which overrides
and one which doesn't?

> 
>>>
>>>> +#endif
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +#endif
>>>> +
>>>>  #ifdef CONFIG_SPL_BUILD
>>>>  void spl_init_keystone_plls(void)
>>>>  {
>>>>
>>>
>>
> 

-- 
cheers,
-roger