| Message ID | 1413278193-28886-1-git-send-email-valentin.longchamp@keymile.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Valentin, my patch fixed environment from i2c eeprom. I am not sure on which board I ran into that trouble. Probably PMC440. So reverting my former patch will break environment in i2c eeprom again on many boards. But perhaps there's a better way to fix that. Matthias On 10/14/2014 11:16 AM, Valentin Longchamp wrote: > Since i2c_init_all always sets the bus back to CONFIG_SYS_SPD_BUS_NUM > for compatibility reasons, it means that any eeprom not located on this > CONFIG_SYS_SPD_BUS_NUM is not accessible with the eeprom commands, even > if you change the bus number with an i2c dev command before. > > Furthermore i2c_init_all should disappear and is currently only called > from the early board initialisation sequences, it is not suited for > other usage. > > This reverts commit 01a0c64762e902971b34587a8a61b59e9ea51374. > > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> > > --- > > common/cmd_eeprom.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/common/cmd_eeprom.c b/common/cmd_eeprom.c > index a02f0cb..29f0f1f 100644 > --- a/common/cmd_eeprom.c > +++ b/common/cmd_eeprom.c > @@ -389,13 +389,8 @@ void eeprom_init (void) > #if defined(CONFIG_SPI) && !defined(CONFIG_ENV_EEPROM_IS_ON_I2C) > spi_init_f (); > #endif > -#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SYS_I2C_SOFT) || \ > - defined(CONFIG_SYS_I2C) > -#ifdef CONFIG_SYS_I2C > - i2c_init_all(); > -#else > - i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); > -#endif > +#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SYS_I2C_SOFT) > + i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); > #endif > } > >
On 10/14/2014 02:32 PM, Matthias Fuchs wrote: > Hi Valentin, > > my patch fixed environment from i2c eeprom. I am not sure on which board > I ran into that trouble. Probably PMC440. So reverting my former patch > will break environment in i2c eeprom again on many boards. > > But perhaps there's a better way to fix that. > do you have CONFIG_I2C_ENV_EEPROM_BUS defined in your board config? Regards Holger
On 10/14/2014 03:01 PM, Holger Brunck wrote: > On 10/14/2014 02:32 PM, Matthias Fuchs wrote: >> Hi Valentin, >> >> my patch fixed environment from i2c eeprom. I am not sure on which board >> I ran into that trouble. Probably PMC440. So reverting my former patch >> will break environment in i2c eeprom again on many boards. >> >> But perhaps there's a better way to fix that. >> > > do you have CONFIG_I2C_ENV_EEPROM_BUS defined in your board config? No, I will give it a try. Matthias
Hi Matthias, On 10/14/2014 02:32 PM, Matthias Fuchs wrote: > Hi Valentin, > > my patch fixed environment from i2c eeprom. I am not sure on which board > I ran into that trouble. Probably PMC440. So reverting my former patch > will break environment in i2c eeprom again on many boards. Good that you have answered to patch. I run into that trouble on our Keymile boards that have the environment in the EEPROM. On our boards, the environment eeprom works with or without your patch. However, with your patch, I cannot directly access ANY eeprom which is not on the CONFIG_I2C_ENV_EEPROM_BUS bus. > > But perhaps there's a better way to fix that. I think that the proposal that Holger made to actually define CONFIG_I2C_ENV_EEPROM_BUS for your boards is what I would try first. Valentin > > Matthias > > > On 10/14/2014 11:16 AM, Valentin Longchamp wrote: >> Since i2c_init_all always sets the bus back to CONFIG_SYS_SPD_BUS_NUM >> for compatibility reasons, it means that any eeprom not located on this >> CONFIG_SYS_SPD_BUS_NUM is not accessible with the eeprom commands, even >> if you change the bus number with an i2c dev command before. >> >> Furthermore i2c_init_all should disappear and is currently only called >> from the early board initialisation sequences, it is not suited for >> other usage. >> >> This reverts commit 01a0c64762e902971b34587a8a61b59e9ea51374. >> >> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> >> >> --- >> >> common/cmd_eeprom.c | 9 ++------- >> 1 file changed, 2 insertions(+), 7 deletions(-) >> >> diff --git a/common/cmd_eeprom.c b/common/cmd_eeprom.c >> index a02f0cb..29f0f1f 100644 >> --- a/common/cmd_eeprom.c >> +++ b/common/cmd_eeprom.c >> @@ -389,13 +389,8 @@ void eeprom_init (void) >> #if defined(CONFIG_SPI) && !defined(CONFIG_ENV_EEPROM_IS_ON_I2C) >> spi_init_f (); >> #endif >> -#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SYS_I2C_SOFT) || \ >> - defined(CONFIG_SYS_I2C) >> -#ifdef CONFIG_SYS_I2C >> - i2c_init_all(); >> -#else >> - i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); >> -#endif >> +#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SYS_I2C_SOFT) >> + i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); >> #endif >> } >> >> > >
Hi Valentin, On 10/14/2014 04:21 PM, Valentin Longchamp wrote: > Hi Matthias, > > On 10/14/2014 02:32 PM, Matthias Fuchs wrote: >> Hi Valentin, >> >> my patch fixed environment from i2c eeprom. I am not sure on which board >> I ran into that trouble. Probably PMC440. So reverting my former patch >> will break environment in i2c eeprom again on many boards. > > Good that you have answered to patch. I run into that trouble on our Keymile > boards that have the environment in the EEPROM. On our boards, the environment > eeprom works with or without your patch. However, with your patch, I cannot > directly access ANY eeprom which is not on the CONFIG_I2C_ENV_EEPROM_BUS bus. > >> >> But perhaps there's a better way to fix that. > > I think that the proposal that Holger made to actually define > CONFIG_I2C_ENV_EEPROM_BUS for your boards is what I would try first. I reverted my former patch and switchedd towards CONFIG_I2C_ENV_EEPROM_BUS. This works fine for me. So I ack your patch to revert mine :-) Matthias
On Thu, Oct 23, 2014 at 04:15:24PM +0200, Matthias Fuchs wrote: > Hi Valentin, > > On 10/14/2014 04:21 PM, Valentin Longchamp wrote: > > Hi Matthias, > > > > On 10/14/2014 02:32 PM, Matthias Fuchs wrote: > >> Hi Valentin, > >> > >> my patch fixed environment from i2c eeprom. I am not sure on which board > >> I ran into that trouble. Probably PMC440. So reverting my former patch > >> will break environment in i2c eeprom again on many boards. > > > > Good that you have answered to patch. I run into that trouble on our Keymile > > boards that have the environment in the EEPROM. On our boards, the environment > > eeprom works with or without your patch. However, with your patch, I cannot > > directly access ANY eeprom which is not on the CONFIG_I2C_ENV_EEPROM_BUS bus. > > > >> > >> But perhaps there's a better way to fix that. > > > > I think that the proposal that Holger made to actually define > > CONFIG_I2C_ENV_EEPROM_BUS for your boards is what I would try first. > I reverted my former patch and switchedd towards > CONFIG_I2C_ENV_EEPROM_BUS. This works fine for me. > > So I ack your patch to revert mine :-) Can you reply with an Acked-by for patchwork to pick up? And then a patch to update your board to work right as well please. Thanks!
Hi Valentin, On 10/14/2014 04:21 PM, Valentin Longchamp wrote: > Hi Matthias, > > On 10/14/2014 02:32 PM, Matthias Fuchs wrote: >> Hi Valentin, >> >> my patch fixed environment from i2c eeprom. I am not sure on which board >> I ran into that trouble. Probably PMC440. So reverting my former patch >> will break environment in i2c eeprom again on many boards. > > Good that you have answered to patch. I run into that trouble on our Keymile > boards that have the environment in the EEPROM. On our boards, the environment > eeprom works with or without your patch. However, with your patch, I cannot > directly access ANY eeprom which is not on the CONFIG_I2C_ENV_EEPROM_BUS bus. > >> >> But perhaps there's a better way to fix that. > > I think that the proposal that Holger made to actually define > CONFIG_I2C_ENV_EEPROM_BUS for your boards is what I would try first. I reverted my former patch and switched towards CONFIG_I2C_ENV_EEPROM_BUS. This works fine for me. I will post a patch soon. So Acked-by: Matthias Fuchs <matthias.fuchs@esd.eu>
On Tue, Oct 14, 2014 at 11:16:33AM +0200, Valentin Longchamp wrote: > Since i2c_init_all always sets the bus back to CONFIG_SYS_SPD_BUS_NUM > for compatibility reasons, it means that any eeprom not located on this > CONFIG_SYS_SPD_BUS_NUM is not accessible with the eeprom commands, even > if you change the bus number with an i2c dev command before. > > Furthermore i2c_init_all should disappear and is currently only called > from the early board initialisation sequences, it is not suited for > other usage. > > This reverts commit 01a0c64762e902971b34587a8a61b59e9ea51374. > > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> > Acked-by: Matthias Fuchs <matthias.fuchs@esd.eu> Applied to u-boot/master, thanks!
diff --git a/common/cmd_eeprom.c b/common/cmd_eeprom.c index a02f0cb..29f0f1f 100644 --- a/common/cmd_eeprom.c +++ b/common/cmd_eeprom.c @@ -389,13 +389,8 @@ void eeprom_init (void) #if defined(CONFIG_SPI) && !defined(CONFIG_ENV_EEPROM_IS_ON_I2C) spi_init_f (); #endif -#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SYS_I2C_SOFT) || \ - defined(CONFIG_SYS_I2C) -#ifdef CONFIG_SYS_I2C - i2c_init_all(); -#else - i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); -#endif +#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SYS_I2C_SOFT) + i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); #endif }
Since i2c_init_all always sets the bus back to CONFIG_SYS_SPD_BUS_NUM for compatibility reasons, it means that any eeprom not located on this CONFIG_SYS_SPD_BUS_NUM is not accessible with the eeprom commands, even if you change the bus number with an i2c dev command before. Furthermore i2c_init_all should disappear and is currently only called from the early board initialisation sequences, it is not suited for other usage. This reverts commit 01a0c64762e902971b34587a8a61b59e9ea51374. Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> --- common/cmd_eeprom.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)