diff mbox

[U-Boot] Fix logic for selection of CONFIG_SYS_DEF_EEPROM_ADDR

Message ID 1324001741-15282-1-git-send-email-Kyle.D.Moffett@boeing.com
State Changes Requested
Headers show

Commit Message

Kyle Moffett Dec. 16, 2011, 2:15 a.m. UTC
A board with CONFIG_SPI and CONFIG_ENV_EEPROM_IS_ON_I2C will get:
  #define CONFIG_SYS_DEF_EEPROM_ADDR 0

Instead of the expected:
  #define CONFIG_SYS_DEF_EEPROM_ADDR CONFIG_SYS_I2C_EEPROM_ADDR

As a result, the "eeprom" command is unusable because it calls
i2c_read() and i2c_write() but always uses an address of 0x00.

This fixes the logic for that particular case, hopefully without
breaking any other board configurations.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Heiko Schocher <hs@denx.de>
---
 include/common.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Comments

Kyle Moffett Jan. 5, 2012, 5:59 p.m. UTC | #1
Any comments on this patch?

If not, could it please be applied/merged?  It fixes a definite
bug on the HWW-1U-1A board.

Cheers,
Kyle Moffett

--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/

On Dec 15, 2011, at 21:15, Kyle Moffett wrote:

> A board with CONFIG_SPI and CONFIG_ENV_EEPROM_IS_ON_I2C will get:
>  #define CONFIG_SYS_DEF_EEPROM_ADDR 0
> 
> Instead of the expected:
>  #define CONFIG_SYS_DEF_EEPROM_ADDR CONFIG_SYS_I2C_EEPROM_ADDR
> 
> As a result, the "eeprom" command is unusable because it calls
> i2c_read() and i2c_write() but always uses an address of 0x00.
> 
> This fixes the logic for that particular case, hopefully without
> breaking any other board configurations.
> 
> Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
> include/common.h |    6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/common.h b/include/common.h
> index 5cfdd76..8a1b401 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -402,13 +402,13 @@ extern void  pic_write (uchar reg, uchar val);
>  * Set this up regardless of board
>  * type, to prevent errors.
>  */
> -#if defined(CONFIG_SPI) || !defined(CONFIG_SYS_I2C_EEPROM_ADDR)
> +#if defined(CONFIG_SPI) && !defined(CONFIG_ENV_EEPROM_IS_ON_I2C)
> +# define CONFIG_SYS_DEF_EEPROM_ADDR 0
> +#elif !defined(CONFIG_SYS_I2C_EEPROM_ADDR)
> # define CONFIG_SYS_DEF_EEPROM_ADDR 0
> #else
> -#if !defined(CONFIG_ENV_EEPROM_IS_ON_I2C)
> # define CONFIG_SYS_DEF_EEPROM_ADDR CONFIG_SYS_I2C_EEPROM_ADDR
> #endif
> -#endif /* CONFIG_SPI || !defined(CONFIG_SYS_I2C_EEPROM_ADDR) */
> 
> #if defined(CONFIG_SPI)
> extern void spi_init_f (void);
> -- 
> 1.7.7.3
>
Wolfgang Denk Jan. 5, 2012, 7:55 p.m. UTC | #2
Dear Kyle Moffett,

In message <1324001741-15282-1-git-send-email-Kyle.D.Moffett@boeing.com> you wrote:
> A board with CONFIG_SPI and CONFIG_ENV_EEPROM_IS_ON_I2C will get:
>   #define CONFIG_SYS_DEF_EEPROM_ADDR 0
> 
> Instead of the expected:
>   #define CONFIG_SYS_DEF_EEPROM_ADDR CONFIG_SYS_I2C_EEPROM_ADDR
> 
> As a result, the "eeprom" command is unusable because it calls
> i2c_read() and i2c_write() but always uses an address of 0x00.
...
> -#if defined(CONFIG_SPI) || !defined(CONFIG_SYS_I2C_EEPROM_ADDR)
> +#if defined(CONFIG_SPI) && !defined(CONFIG_ENV_EEPROM_IS_ON_I2C)
> +# define CONFIG_SYS_DEF_EEPROM_ADDR 0

That should probably rather be 

# define CONFIG_SYS_DEF_EEPROM_ADDR CONFIG_SYS_SPI_EEPROM_ADDR

instead?

> +#elif !defined(CONFIG_SYS_I2C_EEPROM_ADDR)
>  # define CONFIG_SYS_DEF_EEPROM_ADDR 0
>  #else
> -#if !defined(CONFIG_ENV_EEPROM_IS_ON_I2C)
>  # define CONFIG_SYS_DEF_EEPROM_ADDR CONFIG_SYS_I2C_EEPROM_ADDR
>  #endif
> -#endif /* CONFIG_SPI || !defined(CONFIG_SYS_I2C_EEPROM_ADDR) */

All this code should be moved out of common.h.

Actually all users of CONFIG_SYS_DEF_EEPROM_ADDR (if they don't even
define this variable directly in their private code) only use I2C, so
they should probably rather use CONFIG_SYS_I2C_EEPROM_ADDR instead
(board/w7o/w7o.c, board/w7o/cmd_vpd.c, board/w7o/vpd.c,
board/mpl/common/common_util.c)

Only common/cmd_eeprom.c, common/env_eeprom.c appear to support both I2C
and SPI.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/include/common.h b/include/common.h
index 5cfdd76..8a1b401 100644
--- a/include/common.h
+++ b/include/common.h
@@ -402,13 +402,13 @@  extern void  pic_write (uchar reg, uchar val);
  * Set this up regardless of board
  * type, to prevent errors.
  */
-#if defined(CONFIG_SPI) || !defined(CONFIG_SYS_I2C_EEPROM_ADDR)
+#if defined(CONFIG_SPI) && !defined(CONFIG_ENV_EEPROM_IS_ON_I2C)
+# define CONFIG_SYS_DEF_EEPROM_ADDR 0
+#elif !defined(CONFIG_SYS_I2C_EEPROM_ADDR)
 # define CONFIG_SYS_DEF_EEPROM_ADDR 0
 #else
-#if !defined(CONFIG_ENV_EEPROM_IS_ON_I2C)
 # define CONFIG_SYS_DEF_EEPROM_ADDR CONFIG_SYS_I2C_EEPROM_ADDR
 #endif
-#endif /* CONFIG_SPI || !defined(CONFIG_SYS_I2C_EEPROM_ADDR) */
 
 #if defined(CONFIG_SPI)
 extern void spi_init_f (void);