All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikita Kiryanov <nikita@compulab.co.il>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 3/6] cm-t35: add EEPROM module and pass Linux a serial number
Date: Mon, 09 Jan 2012 10:30:19 +0200	[thread overview]
Message-ID: <4F0AA59B.2080603@compulab.co.il> (raw)
In-Reply-To: <20120105145611.47A9082518@gemini.denx.de>

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
>

  reply	other threads:[~2012-01-09  8:30 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-02 14:01 [U-Boot] [PATCH 0/6] Cleanups and updates for cm-t35/3730 Igor Grinberg
2012-01-02 14:01 ` [U-Boot] [PATCH 1/6] cm-t35: cleanup the config file Igor Grinberg
2012-01-03  6:35   ` Igor Grinberg
2012-01-02 14:01 ` [U-Boot] [PATCH 2/6] cm-t35: various cleanups Igor Grinberg
2012-01-02 14:01 ` [U-Boot] [PATCH 3/6] cm-t35: add EEPROM module and pass Linux a serial number Igor Grinberg
2012-01-03 14:52   ` Tom Rini
2012-01-05 12:02     ` [U-Boot] [PATCH v2 " Igor Grinberg
2012-01-05 14:56       ` Wolfgang Denk
2012-01-09  8:30         ` Nikita Kiryanov [this message]
2012-02-06 22:56           ` Albert ARIBAUD
2012-02-07  8:01             ` Igor Grinberg
2012-02-17  9:50               ` Albert ARIBAUD
2012-02-17 17:10                 ` Tom Rini
2012-02-18 10:30                   ` Albert ARIBAUD
2012-02-21 13:44                 ` Igor Grinberg
2012-01-12 13:26         ` [U-Boot] [PATCH v3 " Igor Grinberg
2012-01-02 14:01 ` [U-Boot] [PATCH 4/6] omap3: make get_board_rev() function weak Igor Grinberg
2012-01-03 14:36   ` Tom Rini
2012-01-05 12:03     ` [U-Boot] [PATCH v2 " Igor Grinberg
2012-01-02 14:01 ` [U-Boot] [PATCH 5/6] cm-t35: pass correct revision information to Linux Igor Grinberg
2012-01-02 14:01 ` [U-Boot] [PATCH 6/6] cm-t35: use the new EEPROM module to read the MAC address Igor Grinberg
2012-01-12 13:28   ` [U-Boot] [PATCH v2 " Igor Grinberg
2012-01-03 15:15 ` [U-Boot] [PATCH 0/6] Cleanups and updates for cm-t35/3730 Tom Rini
2012-01-13 17:21   ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F0AA59B.2080603@compulab.co.il \
    --to=nikita@compulab.co.il \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.