[U-Boot] [PATCH 20/31] mpc83xx, kmeter1: extract common I2C options in keymile header

[Heiko Schocher <hs@denx.de>]

Hello Kim,

Kim Phillips wrote:
> On Wed, 28 Jan 2009 10:40:16 +0100
> Heiko Schocher <hs@denx.de> wrote:
> 
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>>  include/configs/keymile-common.h |    8 ++++----
>>  include/configs/kmeter1.h        |   19 -------------------
> 
> a lot of patches in this patchseries touch a lot of the same files -
> can you please reorganize the series so that we don't have to review
> something that's going to change again in a subsequent patch?

Yes, I will reorganize this patchset complete.

>>  2 files changed, 4 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/configs/keymile-common.h b/include/configs/keymile-common.h
>> index 99c3380..4ff6fb7 100644
>> --- a/include/configs/keymile-common.h
>> +++ b/include/configs/keymile-common.h
>> @@ -42,8 +42,6 @@
>>  #define CONFIG_CMD_IMMAP
>>  #define CONFIG_CMD_MII
>>  #define CONFIG_CMD_PING
>> -
>> -/* should go away, if kmeter I2C support is enabled */
>>  #define CONFIG_CMD_DTT
>>  #define CONFIG_CMD_EEPROM
>>  #define CONFIG_CMD_I2C
> 
> the comment alludes that something should go away yet nothing's going
> away but the comment itself.
> 
> If there is indeed nothing to take away, then these are the types of
> things you need to state why in your commit message.

Ok.

>> @@ -101,7 +99,6 @@
>>  #define CONFIG_SYS_SLOT_ID_OFF		(0x07)	/* register offset */
>>  #define CONFIG_SYS_SLOT_ID_MASK		(0x3f)	/* mask for slot ID bits */
>>
>> -#if defined(CONFIG_MGCOGE) || defined(CONFIG_MGSUVD)
>>  #define CONFIG_I2C_MULTI_BUS	1
>>  #define CONFIG_I2C_CMD_TREE	1
>>  #define CONFIG_SYS_MAX_I2C_BUS		2
>> @@ -109,7 +106,11 @@
>>  #define CONFIG_I2C_MUX		1
>>
>>  /* EEprom support */
>> +#if defined(CONFIG_MGCOGE) || defined(CONFIG_MGSUVD)
>>  #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
>> +#else
>> +#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	2
>> +#endif
> 
> if this differs per board, shouldn't it be set in the board config, and
> not the common config?

Yes, that would be the right place.

thanks
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany