From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Thu, 29 Jan 2009 10:07:02 +0100 Subject: [U-Boot] [PATCH 20/31] mpc83xx, kmeter1: extract common I2C options in keymile header In-Reply-To: <20090128201021.a730bede.kim.phillips@freescale.com> References: <49802800.3020304@denx.de> <20090128201021.a730bede.kim.phillips@freescale.com> Message-ID: <498171B6.3070504@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Kim, Kim Phillips wrote: > On Wed, 28 Jan 2009 10:40:16 +0100 > Heiko Schocher wrote: > >> Signed-off-by: Heiko Schocher >> --- >> 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