[U-Boot] [PATCH 01/15] Make bootm optionally use pre-existing atags for Linux kernel boot.

[Pali Rohár]

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-0002.pgp