Hi Detlev,
Detlev Zundel <d...@denx.de> wrote:
> > Signed-off-by: Michael Weiss michael.we...@ifm.com
>
> Please fix the e-mail format.
Ok.
> > @@ -820,6 +820,9 @@ aria_config: unconfig
> > mecp5123_config: unconfig
> > @$(MKCONFIG) -a mecp5123 ppc mpc512x mecp5123 esd
> >
> > +pdm360ng_config: unconfig
> > + @$(MKCONFIG) -a pdm360ng ppc mpc512x pdm360ng
> > +
> > mpc5121ads_config \
> > mpc5121ads_rev2_config \
> > : unconfig
>
> Keep the list of targets sorted in the Makefile please.
Ok!
> > +/*
> > + * Co-Processor communication POST
> > + */
> > +#if defined(CONFIG_POST) && defined(CONFIG_SERIAL_MULTI)
>
> Why is this depending on SERIAL_MULTI? The POST system as such is
> orthogonal, no?
Currently there is only Coprocessor POST only which depends
on SERIAL_MULTI. I think I should better do something like:
#if defined(CONFIG_POST)
#if defined(CONFIG_SERIAL_MULTI)
/* Co-Processor POST code */
...
#endif
#endif
Even better would be to move POST code to separate file, it seems.
> > +
> > +#if defined(CONFIG_SYS_POST_WORD_ADDR)
> > +# define _POST_ADDR (CONFIG_SYS_POST_WORD_ADDR)
> > +#else
> > +#error echo "No POST word address defined"
> > +#endif
> > +
> > +void post_word_store(ulong a)
> > +{
> > + volatile void *save_addr = (volatile void *)(_POST_ADDR);
> > +
> > + out_be32(save_addr, a);
> > +}
> > +
> > +ulong post_word_load(void)
> > +{
> > + volatile void *save_addr = (volatile void *)(_POST_ADDR);
> > +
> > + return in_be32(save_addr);
> > +}
>
> Can't we put this into 512x shared code? I really dislike putting this
> into board code. I know that 5200 set a precedent here, but they all
> have identical code. One should rather fix 5200 also.
I'll move it to cpu/mpc512x/commproc.c.
...
> > + ret = read_port(cop_port, buf, sizeof(buf));
> > + close_port(1);
> > + if (ret <= 0) {
> > + post_log("Error: Can't read WD Coprocessor port.\n");
> > + return -1;
> > + }
> > +
> > + if (strcmp(buf, alive)) {
> > + post_log("Error: WD-Cop. resp.: %s\n", buf);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +#endif /* CONFIG_POST */
>
> Wouldn't it be a good idea to split the POST stuff into a separate file?
I'll move it to a separate file.
> > +/* Use SRAM for initial stack */
> > +#define CONFIG_SYS_INIT_RAM_ADDR CONFIG_SYS_SRAM_BASE /* Initial RAM
> > address */
> > +#define CONFIG_SYS_INIT_RAM_END CONFIG_SYS_SRAM_SIZE /* End
> > of used area in RAM */
> > +
> > +#define CONFIG_SYS_GBL_DATA_SIZE 0x100 /* num bytes
> > initial data */
> > +#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_INIT_RAM_END -
> > CONFIG_SYS_GBL_DATA_SIZE)
> > +#define CONFIG_SYS_POST_WORD_ADDR (CONFIG_SYS_GBL_DATA_OFFSET - 0x4)
> > +#define CONFIG_SYS_INIT_SP_OFFSET CONFIG_SYS_POST_WORD_ADDR
>
> Isn't this shared among the 512x implementations so we can put it in a
> common file?
Yes, we should put it in a common file.
Thanks,
Anatolij
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
