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

[Roger Quadros]

On 08/02/17 14:18, Tom Rini wrote:
> On Wed, Feb 08, 2017 at 02:04:15PM +0200, Roger Quadros wrote:
>> 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?
> 
> What's the usecase for the overrides one?  If the user wants to go back
> to the stored ones, env default -f -a ; saveenv ; reset will do it.
> 
You are right. I don't see any use case for the override one.
Do you want me to patch eth_setenv_enetaddr_ in this series or it can be done separately?

-- 
cheers,
-roger