diff mbox

[U-Boot,v2,3/6] cm-t35: add EEPROM module and pass Linux a serial number

Message ID 1325764937-7342-1-git-send-email-grinberg@compulab.co.il
State Changes Requested
Headers show

Commit Message

Igor Grinberg Jan. 5, 2012, 12:02 p.m. UTC
From: Nikita Kiryanov <nikita@compulab.co.il>

Add board specific EEPROM handling module,
read the serial number from the EEPROM and pass it to Linux.

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
Changes in v2:
  * Use CONFIG_SYS_I2C_EEPROM_ADDR instead of a custom define
  * Fix strange linker warning: ".bss section overlaps previous sections"
    by changing the type of the eeprom_layout static global variable to int
    (probably this is a compiler bug).

 board/cm_t35/Makefile    |    4 ++-
 board/cm_t35/eeprom.c    |   78 ++++++++++++++++++++++++++++++++++++++++++++++
 board/cm_t35/eeprom.h    |   38 ++++++++++++++++++++++
 include/configs/cm_t35.h |    3 ++
 4 files changed, 122 insertions(+), 1 deletions(-)
 create mode 100644 board/cm_t35/eeprom.c
 create mode 100644 board/cm_t35/eeprom.h

Comments

Wolfgang Denk Jan. 5, 2012, 2:56 p.m. UTC | #1
Dear Igor Grinberg,

In message <1325764937-7342-1-git-send-email-grinberg@compulab.co.il> you wrote:
> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> Add board specific EEPROM handling module,
> read the serial number from the EEPROM and pass it to Linux.
...

>   * Fix strange linker warning: ".bss section overlaps previous sections"
>     by changing the type of the eeprom_layout static global variable to int
>     (probably this is a compiler bug).

Probably it is now.  Did you inspect the linke rmap?

> +static int cm_t3x_eeprom_read(uint offset, uchar *buf, int len)
> +{
> +	return i2c_read(CONFIG_SYS_I2C_EEPROM_ADDR, offset,
> +			CONFIG_SYS_I2C_EEPROM_ADDR_LEN, buf, len);
> +}

Is there any specific reason why you cannot use the standard
eeprom_read() function here?

> --- /dev/null
> +++ b/board/cm_t35/eeprom.h
...
> +#ifndef CONFIG_DRIVER_OMAP34XX_I2C
> +void get_board_serial(struct tag_serialnr *serialnr)
> +{
> +	/*
> +	 * This corresponds to what happens when we can communicate with the
> +	 * eeprom but don't get a valid board serial value.
> +	 */
> +	serialnr->low = 0;
> +	serialnr->high = 0;
> +};
> +#endif /* CONFIG_DRIVER_OMAP34XX_I2C */

Please do NOT place code into header files.


Best regards,

Wolfgang Denk
Nikita Kiryanov Jan. 9, 2012, 8:30 a.m. UTC | #2
On 01/05/2012 04:56 PM, Wolfgang Denk wrote:
> Dear Igor Grinberg,
>
> In message<1325764937-7342-1-git-send-email-grinberg@compulab.co.il>  you wrote:
>> From: Nikita Kiryanov<nikita@compulab.co.il>
>>
>> Add board specific EEPROM handling module,
>> read the serial number from the EEPROM and pass it to Linux.
> ...
>
>>    * Fix strange linker warning: ".bss section overlaps previous sections"
>>      by changing the type of the eeprom_layout static global variable to int
>>      (probably this is a compiler bug).
> Probably it is now.  Did you inspect the linke rmap?

u-boot.map shows the bss section aligning perfectly with the start of 
rel.dyn.

The difference between the original "working" version and the version 
with the warning
was an additional byte added by uchar eeprom_layout to the size of 
libcm_t35.o.
This shouldn't be a problem because the bss section is followed by an 
ALIGN(4), but
we decided to try changing eeprom_layout to an int and the problem went 
away.
When we tried to define 4 uchars the problem reappeared.

This suggests that this might be a compiler bug.

There's been some discussion about this in the following threads:
http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114646
http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/90723
and we're not aware of any fix to the issue.

>> +static int cm_t3x_eeprom_read(uint offset, uchar *buf, int len)
>> +{
>> +	return i2c_read(CONFIG_SYS_I2C_EEPROM_ADDR, offset,
>> +			CONFIG_SYS_I2C_EEPROM_ADDR_LEN, buf, len);
>> +}
> Is there any specific reason why you cannot use the standard
> eeprom_read() function here?

The inclusion of the entire eeprom library for this one function will just
increase the image size needlessly. Moreover we don't want to add
eeprom commands, because the eeprom on this module is for internal
use only.

>> --- /dev/null
>> +++ b/board/cm_t35/eeprom.h
> ...
>> +#ifndef CONFIG_DRIVER_OMAP34XX_I2C
>> +void get_board_serial(struct tag_serialnr *serialnr)
>> +{
>> +	/*
>> +	 * This corresponds to what happens when we can communicate with the
>> +	 * eeprom but don't get a valid board serial value.
>> +	 */
>> +	serialnr->low = 0;
>> +	serialnr->high = 0;
>> +};
>> +#endif /* CONFIG_DRIVER_OMAP34XX_I2C */
> Please do NOT place code into header files.

Will be fixed in the next version.

> Best regards,
>
> Wolfgang Denk
>
Albert ARIBAUD Feb. 6, 2012, 10:56 p.m. UTC | #3
Le 09/01/2012 09:30, Nikita Kiryanov a écrit :
> On 01/05/2012 04:56 PM, Wolfgang Denk wrote:
>> Dear Igor Grinberg,
>>
>> In message<1325764937-7342-1-git-send-email-grinberg@compulab.co.il>
>> you wrote:
>>> From: Nikita Kiryanov<nikita@compulab.co.il>
>>>
>>> Add board specific EEPROM handling module,
>>> read the serial number from the EEPROM and pass it to Linux.
>> ...
>>
>>> * Fix strange linker warning: ".bss section overlaps previous sections"
>>> by changing the type of the eeprom_layout static global variable to int
>>> (probably this is a compiler bug).
>> Probably it is now. Did you inspect the linke rmap?
>
> u-boot.map shows the bss section aligning perfectly with the start of
> rel.dyn.
>
> The difference between the original "working" version and the version
> with the warning
> was an additional byte added by uchar eeprom_layout to the size of
> libcm_t35.o.
> This shouldn't be a problem because the bss section is followed by an
> ALIGN(4), but
> we decided to try changing eeprom_layout to an int and the problem went
> away.
> When we tried to define 4 uchars the problem reappeared.
>
> This suggests that this might be a compiler bug.
>
> There's been some discussion about this in the following threads:
> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114646
> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/90723
> and we're not aware of any fix to the issue.

Which prompted me to test --no-check-sections with CS 2009q1. Adding it 
to LDFLAGS_u-boot does reduce the annoyance from errors to a warning, 
but there is no way to completely make it disappear.

Amicalement,
Igor Grinberg Feb. 7, 2012, 8:01 a.m. UTC | #4
Hi Albert,

On 02/07/12 00:56, Albert ARIBAUD wrote:
> Le 09/01/2012 09:30, Nikita Kiryanov a écrit :
>> On 01/05/2012 04:56 PM, Wolfgang Denk wrote:
>>> Dear Igor Grinberg,
>>>
>>> In message<1325764937-7342-1-git-send-email-grinberg@compulab.co.il>
>>> you wrote:
>>>> From: Nikita Kiryanov<nikita@compulab.co.il>
>>>>
>>>> Add board specific EEPROM handling module,
>>>> read the serial number from the EEPROM and pass it to Linux.
>>> ...
>>>
>>>> * Fix strange linker warning: ".bss section overlaps previous sections"
>>>> by changing the type of the eeprom_layout static global variable to int
>>>> (probably this is a compiler bug).
>>> Probably it is now. Did you inspect the linke rmap?
>>
>> u-boot.map shows the bss section aligning perfectly with the start of
>> rel.dyn.
>>
>> The difference between the original "working" version and the version
>> with the warning
>> was an additional byte added by uchar eeprom_layout to the size of
>> libcm_t35.o.
>> This shouldn't be a problem because the bss section is followed by an
>> ALIGN(4), but
>> we decided to try changing eeprom_layout to an int and the problem went
>> away.
>> When we tried to define 4 uchars the problem reappeared.
>>
>> This suggests that this might be a compiler bug.
>>
>> There's been some discussion about this in the following threads:
>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114646
>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/90723
>> and we're not aware of any fix to the issue.
> 
> Which prompted me to test --no-check-sections with CS 2009q1.
> Adding it to LDFLAGS_u-boot does reduce the annoyance from errors to a warning,
> but there is no way to completely make it disappear.

So the conclusion is still a tool chain bug, right?
Probably it doesn't hurt besides the annoying warning.
Albert ARIBAUD Feb. 17, 2012, 9:50 a.m. UTC | #5
Hi Igor,

Le 07/02/2012 09:01, Igor Grinberg a écrit :
> Hi Albert,
>
> On 02/07/12 00:56, Albert ARIBAUD wrote:
>> Le 09/01/2012 09:30, Nikita Kiryanov a écrit :
>>> On 01/05/2012 04:56 PM, Wolfgang Denk wrote:
>>>> Dear Igor Grinberg,
>>>>
>>>> In message<1325764937-7342-1-git-send-email-grinberg@compulab.co.il>
>>>> you wrote:
>>>>> From: Nikita Kiryanov<nikita@compulab.co.il>
>>>>>
>>>>> Add board specific EEPROM handling module,
>>>>> read the serial number from the EEPROM and pass it to Linux.
>>>> ...
>>>>
>>>>> * Fix strange linker warning: ".bss section overlaps previous sections"
>>>>> by changing the type of the eeprom_layout static global variable to int
>>>>> (probably this is a compiler bug).
>>>> Probably it is now. Did you inspect the linke rmap?
>>>
>>> u-boot.map shows the bss section aligning perfectly with the start of
>>> rel.dyn.
>>>
>>> The difference between the original "working" version and the version
>>> with the warning
>>> was an additional byte added by uchar eeprom_layout to the size of
>>> libcm_t35.o.
>>> This shouldn't be a problem because the bss section is followed by an
>>> ALIGN(4), but
>>> we decided to try changing eeprom_layout to an int and the problem went
>>> away.
>>> When we tried to define 4 uchars the problem reappeared.
>>>
>>> This suggests that this might be a compiler bug.
>>>
>>> There's been some discussion about this in the following threads:
>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114646
>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/90723
>>> and we're not aware of any fix to the issue.
>>
>> Which prompted me to test --no-check-sections with CS 2009q1.
>> Adding it to LDFLAGS_u-boot does reduce the annoyance from errors to a warning,
>> but there is no way to completely make it disappear.
>
> So the conclusion is still a tool chain bug, right?
> Probably it doesn't hurt besides the annoying warning.

Sorry for being slow.

Yes, it seems to be a toolchain bug and limited to a spurious warning. 
However, spurious warnings are always a pain when doing a MAKEALL arm, 
because the board is listed among the unclean and failed builds and then 
one must go through these individually and remember that this specific 
warning is both unimportant and unavoidable.

Amicalement,
Tom Rini Feb. 17, 2012, 5:10 p.m. UTC | #6
On 02/17/2012 02:50 AM, Albert ARIBAUD wrote:
> Hi Igor,
>
> Le 07/02/2012 09:01, Igor Grinberg a écrit :
>> Hi Albert,
>>
>> On 02/07/12 00:56, Albert ARIBAUD wrote:
>>> Le 09/01/2012 09:30, Nikita Kiryanov a écrit :
>>>> On 01/05/2012 04:56 PM, Wolfgang Denk wrote:
>>>>> Dear Igor Grinberg,
>>>>>
>>>>> In message<1325764937-7342-1-git-send-email-grinberg@compulab.co.il>
>>>>> you wrote:
>>>>>> From: Nikita Kiryanov<nikita@compulab.co.il>
>>>>>>
>>>>>> Add board specific EEPROM handling module,
>>>>>> read the serial number from the EEPROM and pass it to Linux.
>>>>> ...
>>>>>
>>>>>> * Fix strange linker warning: ".bss section overlaps previous
>>>>>> sections"
>>>>>> by changing the type of the eeprom_layout static global variable
>>>>>> to int
>>>>>> (probably this is a compiler bug).
>>>>> Probably it is now. Did you inspect the linke rmap?
>>>>
>>>> u-boot.map shows the bss section aligning perfectly with the start of
>>>> rel.dyn.
>>>>
>>>> The difference between the original "working" version and the version
>>>> with the warning
>>>> was an additional byte added by uchar eeprom_layout to the size of
>>>> libcm_t35.o.
>>>> This shouldn't be a problem because the bss section is followed by an
>>>> ALIGN(4), but
>>>> we decided to try changing eeprom_layout to an int and the problem went
>>>> away.
>>>> When we tried to define 4 uchars the problem reappeared.
>>>>
>>>> This suggests that this might be a compiler bug.
>>>>
>>>> There's been some discussion about this in the following threads:
>>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114646
>>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/90723
>>>> and we're not aware of any fix to the issue.
>>>
>>> Which prompted me to test --no-check-sections with CS 2009q1.
>>> Adding it to LDFLAGS_u-boot does reduce the annoyance from errors to
>>> a warning,
>>> but there is no way to completely make it disappear.
>>
>> So the conclusion is still a tool chain bug, right?
>> Probably it doesn't hurt besides the annoying warning.
>
> Sorry for being slow.
>
> Yes, it seems to be a toolchain bug and limited to a spurious warning.
> However, spurious warnings are always a pain when doing a MAKEALL arm,
> because the board is listed among the unclean and failed builds and then
> one must go through these individually and remember that this specific
> warning is both unimportant and unavoidable.

Have we gone and bugged any toolchain folks about what caused / fixed 
this problem so that perhaps we can option around it or otherwise squash 
this?
Albert ARIBAUD Feb. 18, 2012, 10:30 a.m. UTC | #7
Le 17/02/2012 18:10, Tom Rini a écrit :
> On 02/17/2012 02:50 AM, Albert ARIBAUD wrote:
>> Hi Igor,
>>
>> Le 07/02/2012 09:01, Igor Grinberg a écrit :
>>> Hi Albert,
>>>
>>> On 02/07/12 00:56, Albert ARIBAUD wrote:
>>>> Le 09/01/2012 09:30, Nikita Kiryanov a écrit :
>>>>> On 01/05/2012 04:56 PM, Wolfgang Denk wrote:
>>>>>> Dear Igor Grinberg,
>>>>>>
>>>>>> In message<1325764937-7342-1-git-send-email-grinberg@compulab.co.il>
>>>>>> you wrote:
>>>>>>> From: Nikita Kiryanov<nikita@compulab.co.il>
>>>>>>>
>>>>>>> Add board specific EEPROM handling module,
>>>>>>> read the serial number from the EEPROM and pass it to Linux.
>>>>>> ...
>>>>>>
>>>>>>> * Fix strange linker warning: ".bss section overlaps previous
>>>>>>> sections"
>>>>>>> by changing the type of the eeprom_layout static global variable
>>>>>>> to int
>>>>>>> (probably this is a compiler bug).
>>>>>> Probably it is now. Did you inspect the linke rmap?
>>>>>
>>>>> u-boot.map shows the bss section aligning perfectly with the start of
>>>>> rel.dyn.
>>>>>
>>>>> The difference between the original "working" version and the version
>>>>> with the warning
>>>>> was an additional byte added by uchar eeprom_layout to the size of
>>>>> libcm_t35.o.
>>>>> This shouldn't be a problem because the bss section is followed by an
>>>>> ALIGN(4), but
>>>>> we decided to try changing eeprom_layout to an int and the problem
>>>>> went
>>>>> away.
>>>>> When we tried to define 4 uchars the problem reappeared.
>>>>>
>>>>> This suggests that this might be a compiler bug.
>>>>>
>>>>> There's been some discussion about this in the following threads:
>>>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114646
>>>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/90723
>>>>> and we're not aware of any fix to the issue.
>>>>
>>>> Which prompted me to test --no-check-sections with CS 2009q1.
>>>> Adding it to LDFLAGS_u-boot does reduce the annoyance from errors to
>>>> a warning,
>>>> but there is no way to completely make it disappear.
>>>
>>> So the conclusion is still a tool chain bug, right?
>>> Probably it doesn't hurt besides the annoying warning.
>>
>> Sorry for being slow.
>>
>> Yes, it seems to be a toolchain bug and limited to a spurious warning.
>> However, spurious warnings are always a pain when doing a MAKEALL arm,
>> because the board is listed among the unclean and failed builds and then
>> one must go through these individually and remember that this specific
>> warning is both unimportant and unavoidable.
>
> Have we gone and bugged any toolchain folks about what caused / fixed
> this problem so that perhaps we can option around it or otherwise squash
> this?

I haven't, as I work mostly with ELDK42 and occasionally recent (2010 
and later) CS and current Ubuntu-provided Linaro toolchains, and don't 
see the warning -- and I don't think toolchain providers will bother 
about warnings from old releases that do not still exist in current 
ones, just like we don't bother with issues present only in older 
version of U-Boot.

But if somebody on the list is using the current version of some 
toolchain that does emit this warning, then yes, they should indeed 
bring the issue to the toolchain provider, or at least raise their hand 
here.

Amicalement,
Igor Grinberg Feb. 21, 2012, 1:44 p.m. UTC | #8
On 02/17/12 11:50, Albert ARIBAUD wrote:
> Hi Igor,
> 
> Le 07/02/2012 09:01, Igor Grinberg a écrit :
>> Hi Albert,
>>
>> On 02/07/12 00:56, Albert ARIBAUD wrote:
>>> Le 09/01/2012 09:30, Nikita Kiryanov a écrit :
>>>> On 01/05/2012 04:56 PM, Wolfgang Denk wrote:
>>>>> Dear Igor Grinberg,
>>>>>
>>>>> In message<1325764937-7342-1-git-send-email-grinberg@compulab.co.il>
>>>>> you wrote:
>>>>>> From: Nikita Kiryanov<nikita@compulab.co.il>
>>>>>>
>>>>>> Add board specific EEPROM handling module,
>>>>>> read the serial number from the EEPROM and pass it to Linux.
>>>>> ...
>>>>>
>>>>>> * Fix strange linker warning: ".bss section overlaps previous sections"
>>>>>> by changing the type of the eeprom_layout static global variable to int
>>>>>> (probably this is a compiler bug).
>>>>> Probably it is now. Did you inspect the linke rmap?
>>>>
>>>> u-boot.map shows the bss section aligning perfectly with the start of
>>>> rel.dyn.
>>>>
>>>> The difference between the original "working" version and the version
>>>> with the warning
>>>> was an additional byte added by uchar eeprom_layout to the size of
>>>> libcm_t35.o.
>>>> This shouldn't be a problem because the bss section is followed by an
>>>> ALIGN(4), but
>>>> we decided to try changing eeprom_layout to an int and the problem went
>>>> away.
>>>> When we tried to define 4 uchars the problem reappeared.
>>>>
>>>> This suggests that this might be a compiler bug.
>>>>
>>>> There's been some discussion about this in the following threads:
>>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114646
>>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/90723
>>>> and we're not aware of any fix to the issue.
>>>
>>> Which prompted me to test --no-check-sections with CS 2009q1.
>>> Adding it to LDFLAGS_u-boot does reduce the annoyance from errors to a warning,
>>> but there is no way to completely make it disappear.
>>
>> So the conclusion is still a tool chain bug, right?
>> Probably it doesn't hurt besides the annoying warning.
> 
> Sorry for being slow.

This is fine, I was also out  for a week or so...

> 
> Yes, it seems to be a toolchain bug and limited to a spurious warning. However, spurious warnings are always a pain when doing a MAKEALL arm, because the board is listed among the unclean and failed builds and then one must go through these individually and remember that this specific warning is both unimportant and unavoidable.

That is exactly the reason why we tried (and succeeded) to workaround the issue...
diff mbox

Patch

diff --git a/board/cm_t35/Makefile b/board/cm_t35/Makefile
index 27693f0..894fa09 100644
--- a/board/cm_t35/Makefile
+++ b/board/cm_t35/Makefile
@@ -25,7 +25,9 @@  include $(TOPDIR)/config.mk
 
 LIB	= $(obj)lib$(BOARD).o
 
-COBJS	:= cm_t35.o leds.o
+COBJS-$(CONFIG_DRIVER_OMAP34XX_I2C) += eeprom.o
+
+COBJS	:= cm_t35.o leds.o $(COBJS-y)
 
 SRCS	:= $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS))
diff --git a/board/cm_t35/eeprom.c b/board/cm_t35/eeprom.c
new file mode 100644
index 0000000..93ed6cb
--- /dev/null
+++ b/board/cm_t35/eeprom.c
@@ -0,0 +1,78 @@ 
+/*
+ * (C) Copyright 2011 CompuLab, Ltd. <www.compulab.co.il>
+ *
+ * Authors: Nikita Kiryanov <nikita@compulab.co.il>
+ *	    Igor Grinberg <grinberg@compulab.co.il>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc.
+ */
+
+#include <common.h>
+#include <i2c.h>
+
+#define EEPROM_LAYOUT_VER_OFFSET	44
+#define BOARD_SERIAL_OFFSET		20
+#define BOARD_SERIAL_OFFSET_LEGACY	8
+
+#define LAYOUT_INVALID	0
+#define LAYOUT_LEGACY	0xff
+
+static int eeprom_layout; /* Implicitly LAYOUT_INVALID */
+
+static int cm_t3x_eeprom_read(uint offset, uchar *buf, int len)
+{
+	return i2c_read(CONFIG_SYS_I2C_EEPROM_ADDR, offset,
+			CONFIG_SYS_I2C_EEPROM_ADDR_LEN, buf, len);
+}
+
+static int eeprom_setup_layout(void)
+{
+	int res;
+
+	if (eeprom_layout != LAYOUT_INVALID)
+		return 0;
+
+	res = cm_t3x_eeprom_read(EEPROM_LAYOUT_VER_OFFSET,
+						(uchar *)&eeprom_layout, 1);
+	if (res) {
+		eeprom_layout = LAYOUT_INVALID;
+		return res;
+	}
+
+	if (eeprom_layout == 0 || eeprom_layout >= 0x20)
+		eeprom_layout = LAYOUT_LEGACY;
+
+	return 0;
+}
+
+void get_board_serial(struct tag_serialnr *serialnr)
+{
+	u32 serial[2];
+	uint offset;
+
+	memset(serialnr, 0, sizeof(*serialnr));
+	if (eeprom_setup_layout())
+		return;
+
+	offset = (eeprom_layout != LAYOUT_LEGACY) ?
+			BOARD_SERIAL_OFFSET : BOARD_SERIAL_OFFSET_LEGACY;
+	if (cm_t3x_eeprom_read(offset, (uchar *)serial, 8))
+		return;
+
+	if (serial[0] != 0xffffffff && serial[1] != 0xffffffff) {
+		serialnr->low = serial[0];
+		serialnr->high = serial[1];
+	}
+}
diff --git a/board/cm_t35/eeprom.h b/board/cm_t35/eeprom.h
new file mode 100644
index 0000000..97a002a
--- /dev/null
+++ b/board/cm_t35/eeprom.h
@@ -0,0 +1,38 @@ 
+/*
+ * (C) Copyright 2011 CompuLab, Ltd. <www.compulab.co.il>
+ *
+ * Authors: Nikita Kiryanov <nikita@compulab.co.il>
+ *	    Igor Grinberg <grinberg@compulab.co.il>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc.
+ */
+#ifndef _EEPROM_
+#define _EEPROM_
+
+#include <common.h>
+
+#ifndef CONFIG_DRIVER_OMAP34XX_I2C
+void get_board_serial(struct tag_serialnr *serialnr)
+{
+	/*
+	 * This corresponds to what happens when we can communicate with the
+	 * eeprom but don't get a valid board serial value.
+	 */
+	serialnr->low = 0;
+	serialnr->high = 0;
+};
+#endif /* CONFIG_DRIVER_OMAP34XX_I2C */
+
+#endif
diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
index 61226d5..fe91c10 100644
--- a/include/configs/cm_t35.h
+++ b/include/configs/cm_t35.h
@@ -72,6 +72,7 @@ 
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_INITRD_TAG
 #define CONFIG_REVISION_TAG
+#define CONFIG_SERIAL_TAG
 
 /*
  * Size of malloc() pool
@@ -153,6 +154,8 @@ 
 #define CONFIG_SYS_I2C_BUS		0
 #define CONFIG_SYS_I2C_BUS_SELECT	1
 #define CONFIG_DRIVER_OMAP34XX_I2C
+#define CONFIG_SYS_I2C_EEPROM_ADDR	0x50
+#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
 
 /*
  * TWL4030