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
[email protected]

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to