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

[Michal Simek]

Mike Frysinger wrote:
> On Tuesday, March 01, 2011 03:34:38 Michal Simek wrote:
>> Mike Frysinger wrote:
>>> On Tuesday, March 01, 2011 03:00:47 Michal Simek wrote:
>>>> If both functions should return 0 then any code should check it and all
>>>> others drivers should be fixed.
>>> i agree, but that doesnt mean new code should knowingly be left broken
>> I agree that make no sense do not fix it right now.
> 
> err, your new driver should return the correct values.  what higher levels do 
> or do not check does not matter, and whether other drivers do it correctly 
> does not matter.

Comment below.

> 
>>>> ep93xx_eth.c returns also 1.
>>>> Anyway if is number of registered devices, "1" should means one
>>>> registered device. If zero means one registered device then please
>>>> point me to that documentation.
>>> the change hasnt been ported to all drivers yet.  but new drivers should
>>> be doing it as i described.
>> 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.

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

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.


> 
>> 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. The changes can be from top to down. If general eth code 
checks return values then driver which returns wrong return codes won't work.

I am not arguing that my driver shouldn't return correct values!!! That's not my point.

If you start arguing that my driver has some faults, I am open to fix it and I really appreciate 
your comments.

I think it is good time to summarize all that changes we discuss: (Please correct it if you think 
that it is wrong). (Our communication is getting messy a little bit)

1. hang() -> return -1

2. driver initialize function (setup dev functions, driver name, priv, etc)
return -1 - if initialize failed
return 0 - on success

3. dev->init
return -1 - if init failed
return 0 - on success
(here you are saying should be return # of devices)

4. dev->recv
return -1 - failed
return 0 - packet not received
return >0 - success - packet lenght

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

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

Thanks,
Michal



Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian