| Message ID | 1486373775-29580-8-git-send-email-rogerq@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
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>
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) > { >
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) > { >
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?
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) >> { >>
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) >> { >> >
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) >>> { >>> >> >
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.
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) >>>> { >>>> >>> >> >
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.
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?
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.
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) {