From mboxrd@z Thu Jan  1 00:00:00 1970
From: Holger Brunck <holger.brunck@keymile.com>
Date: Mon, 02 May 2011 09:32:11 +0200
Subject: [U-Boot] [PATCH 22/30] km/common: implement setboardid as a
	command
In-Reply-To: <20110430082521.5DD29D5270E@gemini.denx.de>
References: <cover.1302272395.git.valentin.longchamp@keymile.com>
	<cover.1302272395.git.valentin.longchamp@keymile.com>
	<30d7413306aba80ca4540c3353d884bdffe9a643.1302272395.git.valentin.longchamp@keymile.com>
	<20110430082521.5DD29D5270E@gemini.denx.de>
Message-ID: <4DBE5DFB.5000501@keymile.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi,

On 04/30/2011 10:25 AM, Wolfgang Denk wrote:
> Dear Valentin Longchamp,
> 
> In message <30d7413306aba80ca4540c3353d884bdffe9a643.1302272395.git.valentin.longchamp@keymile.com> you wrote:
>> From: Holger Brunck <holger.brunck@keymile.com>
>>
>> Read out board id and HW key from the IVM eeprom and set
>> these values as an environment variable was now done inside the
>> code as a command.
> 
> This begs to ask: and how has it been done so far?  Should this patch
> not remove some old code, then?
> 

Yes it does, we did it previously in the default environment. But this was an
out of tree patch so far. Maybe I should adapt the commit message.

>> +{
>> +	unsigned char buf[32];
>> +	char *p;
>> +
>> +	p = get_local_var("IVM_BoardId");
>> +	sprintf((char *)buf, "%s", p);
>> +	setenv("boardid", (char *)buf);
>> +	p = get_local_var("IVM_HWKey");
>> +	sprintf((char *)buf, "%s", p);
>> +	setenv("hwkey", (char *)buf);
> 
> get_local_var() will return NULL in some cases. Error handling
> missing.
> 

Ok.

>> +	return 0;
> 
> As is, the return 0 makes no sense, but I guess this will change when
> you add error handling.
>

Yes.

Thanks.

Best regards
Holger Brunck