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.be...@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?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
