From mboxrd@z Thu Jan  1 00:00:00 1970
From: Pali =?ISO-8859-1?Q?Roh=E1r?= <pali.rohar@gmail.com>
Date: Sun, 09 Oct 2011 01:37:04 +0200
Subject: [U-Boot] [PATCH 01/15] Make bootm optionally use pre-existing
	atags for Linux kernel boot.
In-Reply-To: <201109010952.09122.vapier@gentoo.org>
References: <201109011304.46581.marek.vasut@gmail.com>
	<1314876881-9669-1-git-send-email-pali.rohar@gmail.com>
	<201109010952.09122.vapier@gentoo.org>
Message-ID: <57311380.ftk7ejamt1@pali-elitebook>
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

On Thursday 01 September 2011 09:52:08 you wrote:
> > +#ifdef CONFIG_CHAINLOADER
> > +	uint	params;
> > +	#define PARAMS params
> > +#else
> > +	#define PARAMS (bd->bi_boot_params)
> > +#endif
> 
> do not indent the "#" with preprocessors

ok

> 
> also, this can be rewritten a bit.  always declare uint params regardless of
> the define, and then when in the chainloader logic, set it to the env
> value, otherwise set it to bd->bi_boot_params.  then you can delete this
> dedicated and ugly "PARAMS" define.

ok, PARAMS was deleted.

> 
> > +#ifdef CONFIG_CHAINLOADER
> 
> this is kind of a crappy define.  how about "CONFIG_ATAGSADDR".

I think that here CONFIG_CHAINLOADER is not needed. I think that bootm can use 
other atag without needed defined CONFIG_CHAINLOADER.

> 
> also, you'll need to update toplevel README to document the new option.

ok, I update toplevel README file.

> 
> > +	s = getenv ("atags");
> 
> "atagsaddr" is probably a better name as a plain "atags" can be interpreted
> multiple ways

renamed to atagaddr.

> 
> > +		params = simple_strtoul (s, NULL, 16);
> 
> no spaces before the open paren in func calls.
> wrong: foo (a);
> right: foo(a);

fixed

> 
> > +		printf ("Using existing atags at 0x%x\n", params);
> 
> 0x%x -> %#x

fixed

> -mike

-- 
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111009/784cf424/attachment.pgp