[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Valentin Longchamp]

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
 }
 
-- 
1.8.0.1

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Matthias Fuchs]

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


-- 
------------------------------------------------------------------------
Dipl.-Ing. Matthias Fuchs
Head of System Design

esd electronic system design gmbh
Vahrenwalder Str. 207 - 30165 Hannover - GERMANY
Phone: +49-511-37298-0 - Fax: +49-511-37298-68
Please visit our homepage http://www.esd.eu
Quality Products - Made in Germany
-------------------------------------------------------------------------
Gesch?ftsf?hrer: Klaus Detering
Amtsgericht Hannover HRB 51373 - VAT-ID DE 115672832
-------------------------------------------------------------------------

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Holger Brunck]

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

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Matthias Fuchs]

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

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Valentin Longchamp]

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

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Matthias Fuchs]

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

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141023/d6e6e205/attachment.pgp>

[U-Boot] [PATCH] Revert "common, env: Fix support for environment in i2c eeprom"

[Matthias Fuchs]

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>

[U-Boot] Revert "common, env: Fix support for environment in i2c eeprom"

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141027/fdbe1228/attachment.pgp>