[U-Boot] [PATCH 4/5] NAND: Make page, erase, oob size available via cmd_nand

Wolfgang Denk
2011-09-21T19:52:54Z

Dear Scott Wood,

In message <4E7A332C.5000705@freescale.com> you wrote:
>
> > +#ifdef CONFIG_SYS_HUSH_PARSER
> > +#include <hush.h>
> > +#endif
>
> I don't think you need this ifdef.

It should be omitted alltogether.

> > +#ifdef CONFIG_SYS_HUSH_PARSER
> > + memset(buf, 0, bufsz);
> > + sprintf(buf, "nand_writesize=%x", nand->writesize);
> > + set_local_var(buf, 0);
...
>
> Is there no existing abstraction to add a variable to both the shell (if
> present) and the environment, without needing such an ifdef in the
> caller? If not, should one be added?

While it's a good idea to keep such dynamic variables out of the
(persistently stored) environment, I don't want to see such a solitary
approach. If we start thinking about this it should be implemented
differently, and for all dynamic variables (including things like
filesize etc.).

Marek, please get rid of the hush special case and always use plain
env variables like we do elsewhere, too.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"He was so narrow minded he could see through a keyhole with both
eyes ..."