[U-Boot] [PATCH] mgcoge, mgsuvd: added support for the IVM EEprom.

[U-Boot] [PATCH] mgcoge, mgsuvd: added support for the IVM EEprom.

[Heiko Schocher]

The EEprom contains some Manufacturerinformation,
which are read from u-boot at boot time, and saved
in same Environmentvars.

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 board/keymile/common/common.c |  285 +++++++++++++++++++++++++++++++++++++++++
 include/configs/mgcoge.h      |    5 +
 include/configs/mgsuvd.h      |    5 +
 lib_ppc/board.c               |    6 +
 4 files changed, 301 insertions(+), 0 deletions(-)

diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
index a6ed379..25c5900 100644
--- a/board/keymile/common/common.c
+++ b/board/keymile/common/common.c
@@ -38,6 +38,291 @@
 extern int i2c_mgsuvd_read (void);
 #endif

+int ivm_calc_crc (unsigned char *buf, int len)
+{
+	const unsigned short cCrc16Table[16] = {
+		0x0000, 0xCC01, 0xD801, 0x1400,
+		0xF001, 0x3C00, 0x2800, 0xE401,
+		0xA001, 0x6C00, 0x7800, 0xB401,
+		0x5000, 0x9C01, 0x8801, 0x4400};
+
+	unsigned short crc     = 0;   /* final result */
+	unsigned short r1      = 0;   /* temp */
+	unsigned char  byte    = 0;   /* input buffer */
+	int	i;
+
+	/* calculate CRC from array data */
+	for (i = 0; i < len; i++) {
+		byte = buf[i];
+
+		/* lower 4 bits */
+		r1 = cCrc16Table[crc & 0xF];
+		crc = ((crc) >> 4) & 0x0FFF;
+		crc = crc ^ r1 ^ cCrc16Table[byte & 0xF];
+
+		/* upper 4 bits */
+		r1 = cCrc16Table[crc & 0xF];
+		crc = (crc >> 4) & 0x0FFF;
+		crc = crc ^ r1 ^ cCrc16Table[(byte >> 4) & 0xF];
+	}
+	return crc;
+}
+
+
+static int ivm_get_value (unsigned char *buf, int len, char *name, int off, int check)
+{
+	unsigned short	val;
+	unsigned char	valbuf[30];
+
+	if ((buf[off + 0] != buf[off + 2]) && (buf[off + 2] != buf[off + 4])) {
+		printf ("%s Error corrupted %s\n", __FUNCTION__, name);
+		val = -1;
+	} else {
+		val = buf[off + 0] + (buf[off + 1] << 8);
+		if ((val == 0) && (check == 1))
+			val = -1;
+	}
+	sprintf ((char *)valbuf, "%x", val);
+	setenv (name, (char *)valbuf);
+	return val;
+}
+
+#define BTChar	unsigned char
+
+#define INVENTORYBLOCKSIZE	0x100
+#define INVENTORYDATAADDRESS	0x21
+#define INVENTORYDATASIZE	(INVENTORYBLOCKSIZE - INVENTORYDATAADDRESS - 3)
+typedef  enum {
+	eSymbolShortText	=  0, /* Symbol;ShortText */
+	eManufId		=  1, /* ManufacturerId */
+	eManufSerialNumber	=  2, /* ManufacturerSerialNumber */
+	eManufPartNumber	=  3, /* ManufacturerPartNumber */
+	eManufBuildState	=  4, /* ManufacturerBuildState */
+	eSuplierPartNumber	=  5, /* SuplierPartNumber */
+	eDeliveryDate		=  6, /* DeliveryDate */
+	eSupplierBuildState	=  7, /* SupplierBuildState */
+	eCustomerId		=  8, /* CustomerId */
+	eCustomerProdId		=  9, /* CustomerProdId */
+	eChangeHistory		= 10, /* ChangeHistory  (not used) */
+	eSymbolOnly
+} InventoryData;
+
+static char toAscii(char c)
+{
+	return (c<' ' || c>'~') ? '.' : c;
+}
+
+static int ivm_findInventoryString (InventoryData aType, BTChar* const aString,
+		unsigned long maxLength, unsigned char *buf)
+{
+	int xcode = 0;
+	BTChar cr = '\r';
+	/* Semikolon char */
+	BTChar sc = ';';
+	/* Number of CR found */
+	unsigned long crFound = 0;
+	/* Current address */
+	unsigned long address = INVENTORYDATAADDRESS;
+	/* String length */
+	unsigned long strSize = 0;
+	/* Number of CR to skip */
+	unsigned long nbrOfCR = aType;
+	/* Semicolon to end */
+	int endWithSemikolon = 0;
+
+	/* initialise string */
+	memset(aString, '\0', maxLength);
+
+	switch (aType)
+	{
+		case eSymbolOnly:
+			nbrOfCR = 0;
+			endWithSemikolon = 1;
+		break;
+		default:
+			nbrOfCR = aType;
+			endWithSemikolon = 0;
+	}
+
+	/* Look for the requested number of CR. */
+	while ((crFound != nbrOfCR) && (address < INVENTORYDATASIZE))
+	{
+		if ((buf[address] == cr)) {
+			crFound++;
+		}
+		address++;
+	}
+
+	/* the expected number of CR was found until the end of the IVM
+	 *  content --> fill string */
+	if (address < INVENTORYDATASIZE)
+	{
+		/* Copy the IVM string in the corresponding string */
+		for (; (buf[address] != cr)			&&   /* not cr found */
+			((buf[address] != sc) ||  (!endWithSemikolon))  &&   /* not semikolon found */
+			(strSize < (maxLength-1)			&&
+			(address < INVENTORYDATASIZE)); address++)
+		{
+			strSize += sprintf((char *)aString + strSize, "%c", toAscii(buf[address]));
+		}
+
+		/* copy phase is done: check if everything is ok. If not, the inventory
+		 * data is most probably corrupted: tell the world there is a problem! */
+		if (address == INVENTORYDATASIZE)
+		{
+			xcode = -1;
+			printf ("Error end of string not found\n");
+		} else if ((strSize >= (maxLength-1)) && (buf[address] != cr)) {
+			xcode = -1;
+			printf ("string too long till next CR\n");
+		}
+	} else {
+		/* some CR are missing... the inventory data is most probably corrupted */
+		xcode = -1;
+		printf ("not enough cr found\n");
+	}
+	return xcode;
+}
+
+#define GET_STRING(name, which, len) \
+	if (ivm_findInventoryString (which, valbuf, len, buf) == 0) { \
+		setenv (name, (char *)valbuf); \
+	}
+
+static int ivm_check_crc (unsigned char *buf, int block)
+{
+	unsigned long	crc;
+	unsigned long	crceeprom;
+
+	crc = ivm_calc_crc (buf, CFG_IVM_EEPROM_PAGE_LEN - 2);
+	crceeprom = (buf[CFG_IVM_EEPROM_PAGE_LEN - 1] + \
+			buf[CFG_IVM_EEPROM_PAGE_LEN - 2] * 256);
+	if (crc != crceeprom) {
+		printf ("Error CRC Block: %d EEprom: calculated: %lx EEprom: %lx\n",
+			block, crc, crceeprom);
+		return -1;
+	}
+	return 0;
+}
+
+static int ivm_analyse_block2 (unsigned char *buf, int len)
+{
+	unsigned char	valbuf[CFG_IVM_EEPROM_PAGE_LEN];
+	unsigned long	nbrsAdrs;
+
+	/* IVM_MacAddress */
+	sprintf ((char *)valbuf, "%02X:%02X:%02X:%02X:%02X:%02X",
+			buf[1],
+			buf[2],
+			buf[3],
+			buf[4],
+			buf[5],
+			buf[6]);	
+	setenv ("IVM_MacAddress", (char *)valbuf);
+	if (getenv ("ethaddr") == NULL)
+		setenv ((char *)"ethaddr", (char *)valbuf);
+	/* IVM_MacCount */
+	nbrsAdrs = (buf[10] << 24) +
+		   (buf[11] << 16) +
+		   (buf[12] << 8)  +
+		    buf[13];
+	if (nbrsAdrs == 0xffffffff)
+		nbrsAdrs = 1;
+	sprintf ((char *)valbuf, "%lx", nbrsAdrs);
+	setenv ("IVM_MacCount", (char *)valbuf);
+	return 0;
+}
+
+int ivm_analyse_eeprom (unsigned char *buf, int len)
+{
+	unsigned short	val;
+	unsigned char	valbuf[CFG_IVM_EEPROM_PAGE_LEN];
+	unsigned char	*tmp;
+
+	/* used Code from ivm/src/devsrv_baseproductdata.cpp */
+	if (ivm_check_crc (buf, 0) != 0)
+		return -1;
+
+	ivm_get_value (buf, CFG_IVM_EEPROM_PAGE_LEN, "IVM_BoardId", 0, 1);
+	val = ivm_get_value (buf, CFG_IVM_EEPROM_PAGE_LEN, "IVM_HWKey", 6, 1);
+	if (val != 0xffff) {
+		sprintf ((char *)valbuf, "%x", ((val /100) % 10));
+		setenv ("IVM_HWVariant", (char *)valbuf);
+		sprintf ((char *)valbuf, "%x", (val % 100));
+		setenv ("IVM_HWVersion", (char *)valbuf);
+	}
+	ivm_get_value (buf, CFG_IVM_EEPROM_PAGE_LEN, "IVM_Functions", 12, 0);
+
+	GET_STRING("IVM_Symbol", eSymbolOnly, 8)
+	GET_STRING("IVM_DeviceName", eSymbolShortText, 64)
+	tmp = (unsigned char *) getenv("IVM_DeviceName");
+	if (tmp) {
+		int	len = strlen ((char *)tmp);
+		int	i = 0;
+
+		while (i < len) {
+			if (tmp[i] == ';') {
+				setenv ("IVM_ShortText", (char *)&tmp[i + 1]);
+				break;
+			}
+			i++;
+		}
+		if (i >= len)
+			setenv ("IVM_ShortText", NULL);
+	} else {
+		setenv ("IVM_ShortText", NULL);
+	}
+	GET_STRING("IVM_ManufacturerID", eManufId, 32)
+	GET_STRING("IVM_ManufacturerSerialNumber", eManufSerialNumber, 20)
+	GET_STRING("IVM_ManufacturerPartNumber", eManufPartNumber, 32)
+	GET_STRING("IVM_ManufacturerBuildState", eManufBuildState, 32)
+	GET_STRING("IVM_SupplierPartNumber", eSuplierPartNumber, 32)
+	GET_STRING("IVM_DelieveryDate", eDeliveryDate, 32)
+	GET_STRING("IVM_SupplierBuildState", eSupplierBuildState, 32)
+	GET_STRING("IVM_CustomerID", eCustomerId, 32)
+	GET_STRING("IVM_CustomerProductID", eCustomerProdId, 32)
+
+	if (ivm_check_crc (&buf[CFG_IVM_EEPROM_PAGE_LEN * 2], 2) != 0)
+		return -2;
+	ivm_analyse_block2 (&buf[CFG_IVM_EEPROM_PAGE_LEN * 2], CFG_IVM_EEPROM_PAGE_LEN);
+
+	return 0;
+}
+
+int ivm_read_eeprom(void)
+{
+	I2C_MUX_DEVICE *dev = NULL;
+	uchar i2c_buffer[CFG_IVM_EEPROM_MAX_LEN];
+	uchar	*buf;
+	unsigned dev_addr = CFG_IVM_EEPROM_ADR;
+
+	/* First init the Bus, select the Bus */
+#if defined(CFG_I2C_IVM_BUS)
+	dev = i2c_mux_ident_muxstring ((uchar *)CFG_I2C_IVM_BUS);
+#else
+	buf = (unsigned char *) getenv ("EEprom_ivm");
+	if (buf != NULL)
+		dev = i2c_mux_ident_muxstring (buf);
+#endif
+	if (dev == NULL) {
+		printf ("Error couldnt add Bus for IVM\n");
+		return -1;
+	}
+	i2c_set_bus_num (dev->busid);
+
+	buf = (unsigned char *) getenv ("EEprom_ivm_addr");
+	if (buf != NULL)
+		dev_addr = simple_strtoul ((char *)buf, NULL, 16);
+
+	if (eeprom_read (dev_addr, 0, i2c_buffer, CFG_IVM_EEPROM_MAX_LEN) != 0) {
+		printf ("Error reading EEprom\n");
+		return -2;
+	}
+	
+	return ivm_analyse_eeprom (i2c_buffer, CFG_IVM_EEPROM_MAX_LEN);
+
+}
+
 #if defined(CFG_I2C_INIT_BOARD)
 #define DELAY_ABORT_SEQ		62
 #define DELAY_HALF_PERIOD	(500 / (CFG_I2C_SPEED / 1000))
diff --git a/include/configs/mgcoge.h b/include/configs/mgcoge.h
index 6564c15..b32eb80 100644
--- a/include/configs/mgcoge.h
+++ b/include/configs/mgcoge.h
@@ -212,6 +212,11 @@
 #define CFG_EEPROM_PAGE_WRITE_BITS 3
 #define CFG_EEPROM_PAGE_WRITE_DELAY_MS 10

+/* Support the IVM EEprom */
+#define	CFG_IVM_EEPROM_ADR	0x50
+#define CFG_IVM_EEPROM_MAX_LEN	0x400
+#define CFG_IVM_EEPROM_PAGE_LEN	0x100
+
 /* I2C SYSMON (LM75, AD7414 is almost compatible)			*/
 #define CONFIG_DTT_LM75		1	/* ON Semi's LM75		*/
 #define CONFIG_DTT_SENSORS	{0}	/* Sensor addresses		*/
diff --git a/include/configs/mgsuvd.h b/include/configs/mgsuvd.h
index 79954fe..36c7f26 100644
--- a/include/configs/mgsuvd.h
+++ b/include/configs/mgsuvd.h
@@ -382,6 +382,11 @@
 #define CFG_EEPROM_PAGE_WRITE_BITS 3
 #define CFG_EEPROM_PAGE_WRITE_DELAY_MS 10

+/* Support the IVM EEprom */
+#define	CFG_IVM_EEPROM_ADR	0x50
+#define CFG_IVM_EEPROM_MAX_LEN	0x400
+#define CFG_IVM_EEPROM_PAGE_LEN	0x100
+
 /* I2C SYSMON (LM75, AD7414 is almost compatible)			*/
 #define CONFIG_DTT_LM75		1	/* ON Semi's LM75		*/
 #define CONFIG_DTT_SENSORS	{0, 2, 4, 6}	/* Sensor addresses		*/
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index c02ac62..9b5ff41 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -79,6 +79,9 @@
 extern int update_flash_size (int flash_size);
 #endif

+#if defined(CONFIG_MGCOGE) || defined(CONFIG_MGSUVD)
+extern int ivm_read_eeprom(void);
+#endif
 #if defined(CONFIG_SC3)
 extern void sc3_read_eeprom(void);
 #endif
@@ -869,6 +872,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
 #endif	/* CONFIG_405GP, CONFIG_405EP */
 #endif	/* CFG_EXTBDINFO */

+#if defined(CONFIG_MGCOGE) || defined(CONFIG_MGSUVD)
+	ivm_read_eeprom();
+#endif
 #if defined(CONFIG_SC3)
 	sc3_read_eeprom();
 #endif
-- 
1.5.6.1

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

[U-Boot] [PATCH] mgcoge, mgsuvd: added support for the IVM EEprom.

[Wolfgang Denk]

Dear Heiko Schocher,

In message <48E08A41.1090708@denx.de> you wrote:
> The EEprom contains some Manufacturerinformation,
> which are read from u-boot at boot time, and saved
> in same Environmentvars.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
>  board/keymile/common/common.c |  285 +++++++++++++++++++++++++++++++++++++++++
>  include/configs/mgcoge.h      |    5 +
>  include/configs/mgsuvd.h      |    5 +
>  lib_ppc/board.c               |    6 +
>  4 files changed, 301 insertions(+), 0 deletions(-)
> 
> diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
> index a6ed379..25c5900 100644
> --- a/board/keymile/common/common.c
> +++ b/board/keymile/common/common.c
> @@ -38,6 +38,291 @@
>  extern int i2c_mgsuvd_read (void);
>  #endif
> 
> +int ivm_calc_crc (unsigned char *buf, int len)
> +{
> +	const unsigned short cCrc16Table[16] = {
> +		0x0000, 0xCC01, 0xD801, 0x1400,
> +		0xF001, 0x3C00, 0x2800, 0xE401,
> +		0xA001, 0x6C00, 0x7800, 0xB401,
> +		0x5000, 0x9C01, 0x8801, 0x4400};

If we need support for CRC16, it should be factored out and move to
lib_generic.

> +#define BTChar	unsigned char

Please get rid of this #define.

> +static char toAscii(char c)
> +{
> +	return (c<' ' || c>'~') ? '.' : c;
> +}

Mixed case identifiers like "toAscii" are deprecated.

Also, the name is misleading as it can be easily confised with the
standard function toascii() which implements a different function.

> +{
> +	int xcode = 0;
> +	BTChar cr = '\r';
> +	/* Semikolon char */
> +	BTChar sc = ';';

Come on. Do we really need variables for these? And do you think that
"sc" is easier to read or understand than ';'?

Please drop these.

> +	/* Number of CR found */
> +	unsigned long crFound = 0;
> +	/* Current address */
> +	unsigned long address = INVENTORYDATAADDRESS;
> +	/* String length */
> +	unsigned long strSize = 0;
> +	/* Number of CR to skip */
> +	unsigned long nbrOfCR = aType;
> +	/* Semicolon to end */
> +	int endWithSemikolon = 0;

This is difficult to read. Please put the comments in the  same  line
as  the  variable  declarations,  and  get  rid  of  these mixed case
identifiers.

> +	switch (aType)
> +	{

Incorrect brace style. See also rest of file.

> +		/* Copy the IVM string in the corresponding string */
> +		for (; (buf[address] != cr)			&&   /* not cr found */
> +			((buf[address] != sc) ||  (!endWithSemikolon))  &&   /* not semikolon found */

Maximum line length exceeded.

> +	/* IVM_MacAddress */
> +	sprintf ((char *)valbuf, "%02X:%02X:%02X:%02X:%02X:%02X",
> +			buf[1],
> +			buf[2],
> +			buf[3],
> +			buf[4],
> +			buf[5],
> +			buf[6]);	
> +	setenv ("IVM_MacAddress", (char *)valbuf);

Do we really need this? We already have "ethaddr" ?

> +	if (getenv ("ethaddr") == NULL)
> +		setenv ((char *)"ethaddr", (char *)valbuf);

Oops? You are even aware of this.

Please avoid duplicate data.

> +int ivm_analyse_eeprom (unsigned char *buf, int len)

...analyze...

> +	unsigned short	val;
> +	unsigned char	valbuf[CFG_IVM_EEPROM_PAGE_LEN];
> +	unsigned char	*tmp;
> +
> +	/* used Code from ivm/src/devsrv_baseproductdata.cpp */

What is ivm/src/devsrv_baseproductdata.cpp ?

> +	ivm_get_value (buf, CFG_IVM_EEPROM_PAGE_LEN, "IVM_BoardId", 0, 1);
> +	val = ivm_get_value (buf, CFG_IVM_EEPROM_PAGE_LEN, "IVM_HWKey", 6, 1);
> +	if (val != 0xffff) {
> +		sprintf ((char *)valbuf, "%x", ((val /100) % 10));
> +		setenv ("IVM_HWVariant", (char *)valbuf);
> +		sprintf ((char *)valbuf, "%x", (val % 100));
> +		setenv ("IVM_HWVersion", (char *)valbuf);
> +	}

You're really polluting the environment here.

Is this reasonable?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Lots of folks confuse bad management with destiny.   -- Frank Hubbard

[U-Boot] [PATCH] mgcoge, mgsuvd: added support for the IVM EEprom.

[Heiko Schocher]

Hello Wolfgang,

Wolfgang Denk wrote:
> In message <48E08A41.1090708@denx.de> you wrote:
>> The EEprom contains some Manufacturerinformation,
>> which are read from u-boot at boot time, and saved
>> in same Environmentvars.
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>>  board/keymile/common/common.c |  285 +++++++++++++++++++++++++++++++++++++++++
>>  include/configs/mgcoge.h      |    5 +
>>  include/configs/mgsuvd.h      |    5 +
>>  lib_ppc/board.c               |    6 +
>>  4 files changed, 301 insertions(+), 0 deletions(-)
>>
>> diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
>> index a6ed379..25c5900 100644
>> --- a/board/keymile/common/common.c
>> +++ b/board/keymile/common/common.c
>> @@ -38,6 +38,291 @@
>>  extern int i2c_mgsuvd_read (void);
>>  #endif
>>
>> +int ivm_calc_crc (unsigned char *buf, int len)
>> +{
>> +	const unsigned short cCrc16Table[16] = {
>> +		0x0000, 0xCC01, 0xD801, 0x1400,
>> +		0xF001, 0x3C00, 0x2800, 0xE401,
>> +		0xA001, 0x6C00, 0x7800, 0xB401,
>> +		0x5000, 0x9C01, 0x8801, 0x4400};
> 
> If we need support for CRC16, it should be factored out and move to
> lib_generic.

OK.

>> +#define BTChar	unsigned char
> 
> Please get rid of this #define.

Hmm... I got a source from the Boardmanufacturer, and I wanted to stay
in sync with this code as close as possible, so this looked as the best
way for me ... this affects also the most comments from you.
What to do with such code in common?

[...]
>> +	/* IVM_MacAddress */
>> +	sprintf ((char *)valbuf, "%02X:%02X:%02X:%02X:%02X:%02X",
>> +			buf[1],
>> +			buf[2],
>> +			buf[3],
>> +			buf[4],
>> +			buf[5],
>> +			buf[6]);	
>> +	setenv ("IVM_MacAddress", (char *)valbuf);
> 
> Do we really need this? We already have "ethaddr" ?

We dont need this, but the manufacturer of the Hardware
wants explicitly such a Environmentvariable.

>> +	if (getenv ("ethaddr") == NULL)
>> +		setenv ((char *)"ethaddr", (char *)valbuf);
> 
> Oops? You are even aware of this.

Yes.

> Please avoid duplicate data.

Hmm... the boardmanufacturer wants explicitly a "IVM_" before
all of "his" variables from the EEprom.

[...]
>> +	unsigned short	val;
>> +	unsigned char	valbuf[CFG_IVM_EEPROM_PAGE_LEN];
>> +	unsigned char	*tmp;
>> +
>> +	/* used Code from ivm/src/devsrv_baseproductdata.cpp */
> 
> What is ivm/src/devsrv_baseproductdata.cpp ?

Thats the source from the boardmanufacturer ...

>> +	ivm_get_value (buf, CFG_IVM_EEPROM_PAGE_LEN, "IVM_BoardId", 0, 1);
>> +	val = ivm_get_value (buf, CFG_IVM_EEPROM_PAGE_LEN, "IVM_HWKey", 6, 1);
>> +	if (val != 0xffff) {
>> +		sprintf ((char *)valbuf, "%x", ((val /100) % 10));
>> +		setenv ("IVM_HWVariant", (char *)valbuf);
>> +		sprintf ((char *)valbuf, "%x", (val % 100));
>> +		setenv ("IVM_HWVersion", (char *)valbuf);
>> +	}
> 
> You're really polluting the environment here.

I know.

> Is this reasonable?

Hmm... I think so, because the manufacturer wants all of this vars, and
if I am right, you or Detlev arranged it.

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

[U-Boot] [PATCH] mgcoge, mgsuvd: added support for the IVM EEprom.

[Jerry Van Baren]

Wolfgang Denk wrote:
> Dear Heiko Schocher,
> 
> In message <48E08A41.1090708@denx.de> you wrote:
>> The EEprom contains some Manufacturerinformation,
>> which are read from u-boot at boot time, and saved
>> in same Environmentvars.
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>

[snip]

>> +{
>> +	int xcode = 0;
>> +	BTChar cr = '\r';
>> +	/* Semikolon char */
>> +	BTChar sc = ';';
> 
> Come on. Do we really need variables for these? And do you think that
> "sc" is easier to read or understand than ';'?
> 
> Please drop these.
> 
>> +	/* Number of CR found */
>> +	unsigned long crFound = 0;
>> +	/* Current address */
>> +	unsigned long address = INVENTORYDATAADDRESS;
>> +	/* String length */
>> +	unsigned long strSize = 0;
>> +	/* Number of CR to skip */
>> +	unsigned long nbrOfCR = aType;
>> +	/* Semicolon to end */
>> +	int endWithSemikolon = 0;

Nitpick: s/kolon/colon/ (several places).  The mixture of English and 
German is jarring.

Best regards,
gvb

[U-Boot] [PATCH] mgcoge, mgsuvd: added support for the IVM EEprom.

[Wolfgang Denk]

Dear Heiko Schocher,

In message <48E08A41.1090708@denx.de> you wrote:
> The EEprom contains some Manufacturerinformation,
> which are read from u-boot at boot time, and saved
> in same Environmentvars.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
>  board/keymile/common/common.c |  285 +++++++++++++++++++++++++++++++++++++++++
>  include/configs/mgcoge.h      |    5 +
>  include/configs/mgsuvd.h      |    5 +
>  lib_ppc/board.c               |    6 +
>  4 files changed, 301 insertions(+), 0 deletions(-)

Does not apply:

Applying mgcoge, mgsuvd: added support for the IVM EEprom.
.dotest/patch:195: trailing whitespace.
                        buf[6]);
.dotest/patch:296: trailing whitespace.

error: board/keymile/common/common.c: does not exist in index
fatal: sha1 information is lacking or useless (board/keymile/common/common.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0007.

Please rebase and resubmit.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Where there's no emotion, there's no motive for violence.
	-- Spock, "Dagger of the Mind", stardate 2715.1