All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ppc4xx: Fix problem with board_eth_init() vs cpu_eth_init() on AMCC boards
Date: Wed, 11 Feb 2009 14:02:59 +0100	[thread overview]
Message-ID: <20090211130259.5AFAD832E893@gemini.denx.de> (raw)
In-Reply-To: <200902111352.04598.sr@denx.de>

Dear Stefan,

In message <200902111352.04598.sr@denx.de> you wrote:
>
> > > +	cpu_eth_init(bis);
> > > +	pci_eth_init(bis);
> > > +
> > > +	/*
> > > +	 * Return 0 so that cpu_eth_init() won't get executed again
> > > +	 */
> > > +	return 0;
> >
> > What happens in case of errors? This looks broken to me, or I
> > misinderstand the comment.
> 
> This is the code calling board_eth_init() from net/eth.c:
> 
>         /* Try board-specific initialization first.  If it fails or isn't
>          * present, try the cpu-specific initialization */
>         if (board_eth_init(bis) < 0)
>                 cpu_eth_init(bis);
> 
> So if we return with an error in board_eth_init(), cpu_eth_init() will get 
> called again. Another way to fix this problem would be this implementation:

I consider this a buggy design that should be fixed. It should be
possible to handle the situation that pci_eth_init() returns an error
code.

> board_eth_init()
> {
> 	pci_eth_init(bis);
> 
> 	/*
> 	 * Return -1 so that cpu_eth_init() will get executed in net/eth.c
> 	 */
> 	return -1;
> }
> 
> But I like the former implementation better, since the cpu internal ethernet 
> interfaces are added first to the network devices list.

That would be as bad as the previous solution, or actually worse as it
looks as if board_eth_init() was always failing.

I think the key problems is here:

>         /* Try board-specific initialization first.  If it fails or isn't
>          * present, try the cpu-specific initialization */
>         if (board_eth_init(bis) < 0)
>                 cpu_eth_init(bis);

I think we must differentiate between board_eth_init() not existing
and a failure in board_eth_init(); these are two very different
situations.

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
"Well I don't see why I have to make one man  miserable  when  I  can
make so many men happy."              - Ellyn Mustard, about marriage

  reply	other threads:[~2009-02-11 13:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-11  8:52 [U-Boot] [PATCH] ppc4xx: Fix problem with board_eth_init() vs cpu_eth_init() on AMCC boards Stefan Roese
2009-02-11 12:19 ` Wolfgang Denk
2009-02-11 12:52   ` Stefan Roese
2009-02-11 13:02     ` Wolfgang Denk [this message]
2009-02-11 13:36       ` Stefan Roese
2009-02-11 16:22         ` Wolfgang Denk
2009-02-11 14:53       ` Ben Warren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090211130259.5AFAD832E893@gemini.denx.de \
    --to=wd@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.