[U-Boot] [PATCH 5/7] OMAP3: Add serial number based on die ID

[Dirk Behme <dirk.behme@googlemail.com>]

Dear Jean-Christophe,

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 06:53 Mon 02 Feb     , Dirk Behme wrote:
>> Add serial number based on OMAP3 die ID.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
>>
>> ---
>>  board/omap3/beagle/beagle.c            |    2 ++
>>  board/omap3/evm/evm.c                  |    2 ++
>>  board/omap3/overo/overo.c              |    2 ++
>>  board/omap3/pandora/pandora.c          |    2 ++
>>  board/omap3/zoom1/zoom1.c              |    1 +
>>  cpu/arm_cortexa8/omap3/sys_info.c      |   27 +++++++++++++++++++++++++++
>>  include/asm-arm/arch-omap3/cpu.h       |   13 +++++++++++++
>>  include/asm-arm/arch-omap3/omap3.h     |    1 +
>>  include/asm-arm/arch-omap3/sys_proto.h |    1 +
>>  9 files changed, 51 insertions(+)
>>
>> Index: u-boot-main/cpu/arm_cortexa8/omap3/sys_info.c
>> ===================================================================
>> --- u-boot-main.orig/cpu/arm_cortexa8/omap3/sys_info.c
>> +++ u-boot-main/cpu/arm_cortexa8/omap3/sys_info.c
>> @@ -36,6 +36,33 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_
>>  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>>  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>>  
>> +/*****************************************************************
>> + * serial_num_r(void) - read and set serial number based on DIE ID
>> + *****************************************************************/
>> +void serial_num_r(void)
>> +{
>> +	ctrl_id_t *id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
>> +	char *uid_s, die_id[34];
>> +	u32 id[4];
>> +
>> +	memset(die_id, 0, sizeof(die_id));
>> +
>> +	uid_s = getenv("serial");
> why not call a cat a cat?

Why not give the cat a fitting name?

The user gets a serial number, so we name it serial. How and where we 
get it from is hidden from user. It wouldn't help to name it "die ID" 
for user, nobody would know what this is. But serial number is clear 
to every body.

> IMHO why don't you report the die id?
>> +
>> +	if (uid_s == NULL) {
>> +		id[3] = readl(&id_base->die_id_0);
>> +		id[2] = readl(&id_base->die_id_1);
>> +		id[1] = readl(&id_base->die_id_2);
>> +		id[0] = readl(&id_base->die_id_3);
>> +		sprintf(die_id, "%08x%08x%08x%08x", id[0], id[1], id[2], id[3]);
>> +		setenv("serial", die_id);
>> +	} else {
>> +		strncpy(die_id, uid_s, 34);
> why copy it? why not simply print it

Same concept as in beagle revision patch: Touch hardware only once if 
not done already, store the value and deal with value read from HW 
instead of touching HW again and again.

Then we use one central output location for the printf below, not 
repeating same printf at different lines.

>> +	}
>> +
>> +	printf("Serial #%s\n", die_id);
>> +}

Dirk

Btw.: I hope that the other OMAP3 patch sent independently

http://lists.denx.de/pipermail/u-boot/2009-February/046691.html

will not be forgotten?