[U-Boot] [PATCH] 8xx: prevent a machine check in scc_init().

[Wolfgang Denk]

Dear Gary Jennejohn,

In message <20080914114924.0090d95c at peedub.jennejohn.org> you wrote:
> 
> Signed-off-by: Gary Jennejohn <garyj at 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.

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
The reasonable man adapts himself to the world; the unreasonable  one
persists  in  trying  to  adapt  the  world to himself. Therefore all
progress depends on the unreasonable man."      - George Bernard Shaw