On Tuesday, March 01, 2011 04:29:21 Michal Simek wrote:
Mike Frysinger wrote:
On Tuesday, March 01, 2011 03:34:38 Michal Simek wrote:
How does it look like phy lib u-boot support?
i dont know what you mean ... how is phylib relevant to this ? or are you just asking in general ?
Ben wanted to create general phy lib support and remove all phy specific things from net drivers. I hope you see that connection because there is also phy part. If phy lib support is in progress (which probably not) then I would change the driver to support it.
yes, but that isnt relevant to any of the feedback ive given for this patch
also, you should change the "hang()" to "return 0" in the init func.
Are you sure return 0 which should mean success. Anything different from 0 seems to me relevant.
as i said, the initialize function is not returning "success" or "failure". it is returning "# of devices registered". if you cannot register any, you should return 0. having the boot process fail because of network issues doesnt make much sense when u-boot can do quite a lot without the network. including updating itself via other means.
Interesting.
- you talked about hang() in initialize function(not dev->init) and for me
xilinx_axiemac_initialize Initialize function is called from board_eth_init which is called from eth_initialize(eth.c)
There is this part of code if (board_eth_init != __def_eth_init) { if (board_eth_init(bis) < 0) printf("Board Net Initialization Failed\n");
If initialization failed the return value is < 0. That's why hang() should be changed to return -1 and doesn't matter how many device there are.
this detail isnt currently ironed out, so if you wanted to change the "hang()" into "return -1" or "return 0", that is fine.
If you write:
also, you should change the "hang()" to "return 0" in the init func.
(hang is only in xilinx_axiemac_initialize) and should be changed which I agree.
If you propose any change which I should do, I expect that if you are focus on blackfin that you have done that changes in all blackfin eth drivers. For example in bfin_mac.c where hang is also used.
incorrect code in other drivers (including bfin_mac) is not justification for adding incorrect code to new drivers. bfin_mac.c's call to hang() is wrong too in the current code base.
I maintain emaclite driver and none tell me this that's why the process is so slow. I believe if you release that documentation, which you are talking about, then others will clean/test their drivers.
the behavior i describe isnt a decision i made. it was made by the previous net maintainer and agreed upon by others in the discussion.
Ok. If that decision was made than I expect that should be written somewhere in doc. I know it is boring to write any documentation but I expect that if any decision was made then is common that general code will be changed.
obviously that is true, but docs only get written/updated when someone is so inclined to do the work. the existing doc only exists because i felt like writing at the time. ive never been the net maintainer and thus "obligated" to write net documentation. i just got tired of people doing it wrong and no one knowing what should be going on.
- hang() -> return -1
ok
- driver initialize function (setup dev functions, driver name, priv, etc)
return -1 - if initialize failed return 0 - on success
no, "return 1" when the device has successfully registered
- dev->init
return -1 - if init failed return 0 - on success
ok
(here you are saying should be return # of devices)
no, i think you confused "initialize" with "init" in my feedback
- dev->recv
return -1 - failed return 0 - packet not received return >0 - success - packet lenght
"return 0" is still "success" in the sense that there is nothing to do, but that nuance doesnt matter
- dev->send
return -1 - failed return 0 - success
- dev->write_hwaddr
return -1 - failed return 0 - success
ok -mike