Re: [U-Boot] [PATCH] ppc4xx: Fix problem with board_eth_init() vs cpu_eth_init() on AMCC boards

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