Revert "common, env: Fix support for environment in i2c eeprom"

PATCH In review Valentin Longchamp 4 participants 9 messages Oct 14, 2014 – Oct 27, 2014

Share & tools

Valentin Longchamp

author

2014-10-14 09:16 UTC #0 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

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 <[email protected]>

---

 common/cmd_eeprom.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

common/cmd_eeprom.c +2 -7

index a02f0cb..29f0f1f 100644
@@ -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
 }

Signature · 1 line

1.8.0.1

Matthias Fuchs

2014-10-14 12:32 UTC #1 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

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

Quoted 39 lines · 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 <[email protected]>
---
 common/cmd_eeprom.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
common/cmd_eeprom.c +2 -7
index a02f0cb..29f0f1f 100644
@@ -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
 }
 

Signature · 13 lines

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

Holger Brunck

2014-10-14 13:01 UTC #2 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

Quoted 8 lines · 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.

do you have CONFIG_I2C_ENV_EEPROM_BUS defined in your board config?

Regards
Holger

Matthias Fuchs

2014-10-14 14:20 UTC #3 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

Quoted 11 lines · Holger Brunck

Quoted 8 lines · 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.
do you have CONFIG_I2C_ENV_EEPROM_BUS defined in your board config?

No, I will give it a try.
Matthias

Valentin Longchamp

author

2014-10-14 14:21 UTC #4 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

In reply to Matthias Fuchs

Hi Matthias,

Quoted 5 lines · 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.

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.

Quoted 2 lines

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

Quoted 46 lines

Matthias
Quoted 39 lines · 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 <[email protected]>
---
 common/cmd_eeprom.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
common/cmd_eeprom.c +2 -7
index a02f0cb..29f0f1f 100644
@@ -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
 }
 

Matthias Fuchs

2014-10-23 14:15 UTC #5 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

Hi Valentin,

Quoted 15 lines · Valentin Longchamp

Hi Matthias,
Quoted 5 lines · 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.
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.
Quoted 1 line

Quoted 4 lines

Quoted 1 line
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

Tom Rini

2014-10-23 15:11 UTC #6 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

Quoted 18 lines · Matthias Fuchs

Hi Valentin,
Quoted 15 lines · Valentin Longchamp
Hi Matthias,
Quoted 5 lines · 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.
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.
Quoted 1 line

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
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!

Signature · 8 lines

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>

Matthias Fuchs

2014-10-23 15:29 UTC #7 lore raw

PATCH 1/1 Revert "common, env: Fix support for environment in i2c eeprom"

In reply to Valentin Longchamp

Hi Valentin,

Quoted 15 lines · Valentin Longchamp

Hi Matthias,
Quoted 5 lines · 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.
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.
Quoted 1 line

Quoted 4 lines

Quoted 1 line
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.

Acked-by Matthias Fuchs <[email protected]>

Tom Rini

2014-10-27 22:19 UTC #8 lore raw

In reply to Valentin Longchamp

Quoted 5 lines · 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.

Quoted 8 lines

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 <[email protected]>
Acked-by Matthias Fuchs <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

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>