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@denx.de wrote:
Dear Upakul,
in message bb58ac4d0711152213y4e4520ay44ae0b9d5302b9c2@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@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."
**********************************************************************
---------------------------------------------------------------------