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

Pali Rohár
2011-10-08T23:37:04Z

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