[U-Boot-Users] What if eth_init() fails?

[Upakul Barkakaty]

Hi Wolfgang/Detlev,

Thanks for writing in. Yes, I do agree with your suggestion that
changing the code in eth.c would be, logically, the best way to handle
this. 

The reason why I had suggested changing the code in Netloop() in net.c,
was because I had observed the Linux open source file dev.c, where
positive error codes had been handled in the dev_init() function, so I
thought that would not be a bad idea. Here is the code snippet from the
Linux file dev.c

	/* Init, if this function is available */
	if (dev->init) {
		ret = dev->init(dev);
		if (ret) {
			if (ret > 0)
				ret = -EIO;
			goto out_err;
		}
	}


But on second thoughts, yes, logically, it would be best to leave the
Netloop() code as it is:

if (eth_init(bd) < 0) ... /* error handling */

and modify the code in eth.c file to make it return a negative code in
case of errors, to somewhat like this:

int eth_init(bd_t *bis)
{
	struct eth_device* old_current;

	if (!eth_current)
		return -1;

	old_current = eth_current;
	do {
		debug ("Trying %s\n", eth_current->name);

		if (!eth_current->init(eth_current,bis)) 
		{
			eth_current->state = ETH_STATE_ACTIVE;
			return 0;
		}
		debug  ("FAIL\n");

		eth_try_another(0);
	} while (old_current != eth_current);

	return -1;
}


Regards,
 
Upakul Barkakaty
Conexant Systems, Inc.



On 11/16/07, Wolfgang Denk <wd at denx.de> wrote:

    Dear Upakul,

    in message
<bb58ac4d0711152213y4e4520ay44ae0b9d5302b9c2 at mail.gmail.com> you wrote:
    >
    > Thanks for the replies. I am attaching herewith, the patch which I
suppose
    > should fix the issue in NetLoop().

    I'm sorry, but I think this is actually not a good idea.

    Tradition is that a function returns <0 (typical -1) in case of
    problems, and a return code >=0 indicates success (eventually
    including a useful return value).

    It seems that the original call was based on that expectation, too:

    --- u-boot-1.2.0_orig/net/net.c 2007-01-07 04:43:11.000000000 +0530
    +++ u-boot-1.2.0/net/net.c      2007-11-14 18:03:03.000000000 +0530
    @@ -305,7 +305,7 @@
    #ifdef CONFIG_NET_MULTI
            eth_set_current();
    #endif
    -       if (eth_init(bd) < 0) {

    The test for  "< 0" reads to me: "if there was an error"...

    +       if (eth_init(bd) > 0) {

    Now this is completely misleading.


    Assuming that eth_init() can only result in sucess or failure, the
    test should be written either as

            if (eth_init(bd) != 0) ... /* error handling */

    or be left as is:

            if (eth_init(bd) < 0) ... /* error handling */

    Actually I strongly prefer the second form which perfectly matches
my
    internal C parser :-)

    So the real fix for this problem is to change the code of eth_init()
    instead and to make it return -1 in case of errors (instead of 1).

    Note that all places where eth_init() is called should be checked.


    And BTW: please stop posting HTML!

    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
    There is an order of things in this universe.
            -- Apollo, "Who Mourns for Adonais?" stardate 3468.1



Conexant E-mail Firewall (Conexant.Com) made the following annotations
---------------------------------------------------------------------
********************** Legal Disclaimer **************************** 

"This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you." 

********************************************************************** 

---------------------------------------------------------------------