[U-Boot] [PATCH] net: axi_ethernet: Add driver to u-boot

[Mike Frysinger]

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.
> 1. 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.

> 1. hang() -> return -1

ok

> 2. 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

> 3. 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

> 4. 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

> 5. dev->send
> return -1 - failed
> return 0 - success
> 
> 6. dev->write_hwaddr
> return -1 - failed
> return 0 - success

ok
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110303/0f1f9735/attachment.pgp