On Sun, 14 Sep 2008 19:07:52 +0200 Wolfgang Denk wd@denx.de wrote:

In message 20080914114924.0090d95c@peedub.jennejohn.org you wrote:

...

Signed-off-by: Gary Jennejohn garyj@denx.de

Sorry, but I don't understand what you're doing here, or why.

Why would there be any machine checks in scc_init()? Such problems have never been repoorted for any systems. Actuallym this part of the code is one of the oldest in U-Boot - if there were machine check issues in any configruation, these should be known.

...

--- a/cpu/mpc8xx/scc.c +++ b/cpu/mpc8xx/scc.c @@ -70,6 +70,9 @@ static int scc_recv(struct eth_device* dev); static int scc_init (struct eth_device* dev, bd_t * bd); static void scc_halt(struct eth_device* dev);

+/* avoid unnecessary reinitialization of the SCC */ +static int scc_init_completed;

int scc_initialize(bd_t *bis) { struct eth_device* dev; @@ -193,6 +196,19 @@ static int scc_init (struct eth_device *dev, bd_t * bis)

volatile immap_t *immr = (immap_t *) CFG_IMMR;

• /*

• * This routine is called again and again from eth_init(),
  

• * especially when CONFIG_NETCONSOLE is defined and
  

• * stdout=nc.
  

• * Avoid unneccessary flailing, otherwise we can get a panic here.
  

• */
  

• if (scc_init_completed) {

• immr->im_ioport.iop_pcso |= (PC_ENET_CLSN | PC_ENET_RENA);
  

• immr->im_cpm.cp_scc[SCC_ENET].scc_gsmrl |=
  

• 	(SCC_GSMRL_ENR | SCC_GSMRL_ENT);
  

If the initialization has already been done, why would we repeat these two steps here?

And why exactle these two, and not the others that are performed for a regular init sequence?

...

• return 1;
  

• }

#if defined(CONFIG_LWMON) reset_phy(); #endif

I'm not sure if your early return here is actually valid. As you can see, other actions follow - for example here on the LWMON baord the PHY reset; I suspect that your change might eventually work on your current system, but break many others.

Also, I'm not sure if you are4 aware that the eth_init() part may be called from a interface switching sequence, i. e. when cycling between several ethenret interfaces. These will have been shut down before by eth_halt() - skipping the init sequence here seems the wrong thing to me.

Maybe you could explain what problem you are actually truing to fix, and especially under which situations you observe machine checks.

The first call to scc_init() executes all the code. Only subsequent calls are short-circuited.

OK, just think about it a little.

When nc is active as a console it is often invoked for every byte.

The network stack code calls eth_init() prior to sending the byte and eth_halt() after sending the byte.

scc_init() happily tries to reallocate dpram at every call when CFG_ALLOC_DPRAM is defined, as it was in my case. Eventually dpram is exhausted which evidently results in a machine check.

Also note that scc_halt() merely clears some bits in scc_gsmrl and iop_pcso. It really doesn't seem necessary to reinitialize every register at every call to scc_init().

Most network drivers behave this way, which works in the general use case, but not when nc is used.

I doubt that nc is used very often, otherwise this problem would have reared its ugly head a lot sooner. The general use case is also to merely invoke tftp to load the kernel and maybe fdt. This succeeds because it takes numerous (in this case about 14) calls to the init routine before the machine check happens.

--- Gary Jennejohn ********************************************************************* 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: office@denx.de *********************************************************************