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]
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot

