[v3,7/8] ARM: k2g: setup PRU ethernet MAC addresses

Message ID 1486373775-29580-8-git-send-email-rogerq@ti.com
State New
Headers show
Series
  • am57xx-idk LCD and am571x-idk 6 port ethernet pinmux
Related show

Commit Message

Roger Quadros Feb. 6, 2017, 9:36 a.m.
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@ti.com>

Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

---
 board/ti/ks2_evm/board_k2g.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

-- 
2.7.4

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Tom Rini Feb. 7, 2017, 12:45 a.m. | #1
On Mon, Feb 06, 2017 at 11:36:14AM +0200, 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@ti.com>

> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>


Reviewed-by: Tom Rini <trini@konsulko.com>


-- 
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Lokesh Vutla Feb. 7, 2017, 3:22 a.m. | #2
On 2/6/2017 3:06 PM, 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@ti.com>

> Reviewed-by: Lokesh Vutla <lokeshvutla@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)


You might want to select CONFIG_TI_I2C_BOARD_DETECT and
CONFIG_BOARD_LATE_INIT for this to take effect. I do not see these
configs enabled or am I missing something?

Thanks and regards,
Lokesh

> +	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);

> +#endif

> +

> +	return 0;

> +}

> +#endif

> +

>  #ifdef CONFIG_SPL_BUILD

>  void spl_init_keystone_plls(void)

>  {

> 

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Igor Grinberg Feb. 7, 2017, 7:52 a.m. | #3
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@ti.com>

> Reviewed-by: Lokesh Vutla <lokeshvutla@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?
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.

> +#endif

> +

> +	return 0;

> +}

> +#endif

> +

>  #ifdef CONFIG_SPL_BUILD

>  void spl_init_keystone_plls(void)

>  {

> 


-- 
Regards,
Igor.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Tom Rini Feb. 7, 2017, 6:28 p.m. | #4
On Tue, Feb 07, 2017 at 09:52:25AM +0200, 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@ti.com>

> > Reviewed-by: Lokesh Vutla <lokeshvutla@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?

> 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.


This is true.  Can we perhaps come up with a helper function that's
normally called to set the "eth?addr" to MAC if unset already, instead
of having N instances of the same logic?

-- 
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros Feb. 8, 2017, 8:35 a.m. | #5
On 07/02/17 05:22, Lokesh Vutla wrote:
> 

> 

> On 2/6/2017 3:06 PM, 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@ti.com>

>> Reviewed-by: Lokesh Vutla <lokeshvutla@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)

> 

> You might want to select CONFIG_TI_I2C_BOARD_DETECT and

> CONFIG_BOARD_LATE_INIT for this to take effect. I do not see these

> configs enabled or am I missing something?


I was expecting k2g-ice board to have a new defconfig. But it seems that
we will continue to use k2g_evm_defconfig for that so I'll enable these
options there.

cheers,
-roger

> 

> Thanks and regards,

> Lokesh

> 

>> +	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);

>> +#endif

>> +

>> +	return 0;

>> +}

>> +#endif

>> +

>>  #ifdef CONFIG_SPL_BUILD

>>  void spl_init_keystone_plls(void)

>>  {

>>


-- 
cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros Feb. 8, 2017, 8:51 a.m. | #6
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@ti.com>

>> Reviewed-by: Lokesh Vutla <lokeshvutla@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.

> 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.
> 

>> +#endif

>> +

>> +	return 0;

>> +}

>> +#endif

>> +

>>  #ifdef CONFIG_SPL_BUILD

>>  void spl_init_keystone_plls(void)

>>  {

>>

> 


-- 
cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Igor Grinberg Feb. 8, 2017, 11:51 a.m. | #7
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@ti.com>

>>> Reviewed-by: Lokesh Vutla <lokeshvutla@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.

> 

>> 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. 

>>

>>> +#endif

>>> +

>>> +	return 0;

>>> +}

>>> +#endif

>>> +

>>>  #ifdef CONFIG_SPL_BUILD

>>>  void spl_init_keystone_plls(void)

>>>  {

>>>

>>

> 


-- 
Regards,
Igor.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Igor Grinberg Feb. 8, 2017, 11:52 a.m. | #8
Hi Tom,

On 02/07/17 20:28, Tom Rini wrote:
> On Tue, Feb 07, 2017 at 09:52:25AM +0200, 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@ti.com>

>>> Reviewed-by: Lokesh Vutla <lokeshvutla@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?

>> 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.

> 

> This is true.  Can we perhaps come up with a helper function that's

> normally called to set the "eth?addr" to MAC if unset already, instead

> of having N instances of the same logic?


That sounds like a good plan.

-- 
Regards,
Igor.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros Feb. 8, 2017, 12:04 p.m. | #9
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@ti.com>

>>>> Reviewed-by: Lokesh Vutla <lokeshvutla@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Tom Rini Feb. 8, 2017, 12:18 p.m. | #10
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@ti.com>

> >>>> Reviewed-by: Lokesh Vutla <lokeshvutla@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.

-- 
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros Feb. 8, 2017, 12:46 p.m. | #11
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@ti.com>

>>>>>> Reviewed-by: Lokesh Vutla <lokeshvutla@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Tom Rini Feb. 8, 2017, 12:50 p.m. | #12
On Wed, Feb 08, 2017 at 02:46:20PM +0200, Roger Quadros wrote:
> 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@ti.com>

> >>>>>> Reviewed-by: Lokesh Vutla <lokeshvutla@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?


It's too late for this merge window so yes, lets get the
eth_setenv_enetaddr doing the already set in env check added in and
acked by Joe, for the next release when we can pull this in too.

-- 
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Patch hide | download patch | download mbox

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);
+#endif
+
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_SPL_BUILD
 void spl_init_keystone_plls(void)
 {