[U-Boot] Net driver questions
Hello,
Please forgive me if answers to my questions are obvious, I'm new to U-Boot.
I'm trying to port the Ethernet driver for Intel IXP4xx CPU from Linux (I know there is Intel's code already ported). I've read the doc/README.drivers.eth. I came across some problems: - dev->halt() seems to be called before the first call to dev->init() (i.e., before the hardware is initialized). Is it on purpose?
- dev->recv() seems to be called recursively, for example while doing "dhcp" or "bootp" (ping is ok). dev->recv() in my driver calls NetReceive(), which in turn (without returning to the caller, i.e., dev->recv(), first) reinitializes the driver on error (calls dev->halt() and dev->init()). This makes a lot of mess in the driver, should it stay this way? Perhaps we should queue the received packets and process them on return from dev->recv()? Or maybe return all those packets together?
- dev->recv() is provided with RX packet buffers. IXP4xx can only receive to already allocated memory so the driver has to provide it's own buffers prior to dev->recv(). I assume it's common situation with all hardware recent enough. Does the driver have to copy data to NetRxPackets[], or is it ok to simply call NetReceive using driver's buffers?
On Sunday 01 November 2009 18:07:43 Krzysztof Halasa wrote:
I'm trying to port the Ethernet driver for Intel IXP4xx CPU from Linux (I know there is Intel's code already ported). I've read the doc/README.drivers.eth. I came across some problems:
- dev->halt() seems to be called before the first call to dev->init() (i.e., before the hardware is initialized). Is it on purpose?
it's on purpose because it makes the code simpler -- no need to maintain state. drivers have to be able to handle halt() irregardless of init(). i dont see this being a problem for anyone.
- dev->recv() seems to be called recursively, for example while doing "dhcp" or "bootp" (ping is ok). dev->recv() in my driver calls NetReceive(), which in turn (without returning to the caller, i.e., dev->recv(), first) reinitializes the driver on error (calls dev->halt() and dev->init()). This makes a lot of mess in the driver, should it stay this way? Perhaps we should queue the received packets and process them on return from dev->recv()? Or maybe return all those packets together?
where exactly do you see that call path ? i dont see it anywhere ...
NetReceive() may call eth_send(), but that only expands into dev->send()
- dev->recv() is provided with RX packet buffers. IXP4xx can only receive to already allocated memory so the driver has to provide it's own buffers prior to dev->recv(). I assume it's common situation with all hardware recent enough. Does the driver have to copy data to NetRxPackets[], or is it ok to simply call NetReceive using driver's buffers?
the NetRxPackets[] are set up for you by default and are merely a convenience. you can use them or not, it doesnt really matter. after all, your driver is what calls NetReceive() and the first argument there is the buffer that you're receiving. none of the internal network code relies on these pointers.
i'll send out a patch to cover some of the finer details in the drivers doc. -mike
Mike Frysinger vapier@gentoo.org writes:
it's on purpose because it makes the code simpler -- no need to maintain state. drivers have to be able to handle halt() irregardless of init(). i dont see this being a problem for anyone.
Ok. Sure, that's not a problem for me, I just noted the README doesn't talk about this.
- dev->recv() seems to be called recursively, for example while doing "dhcp" or "bootp" (ping is ok). dev->recv() in my driver calls NetReceive(), which in turn (without returning to the caller, i.e., dev->recv(), first) reinitializes the driver on error (calls dev->halt() and dev->init()). This makes a lot of mess in the driver, should it stay this way? Perhaps we should queue the received packets and process them on return from dev->recv()? Or maybe return all those packets together?
where exactly do you see that call path ? i dont see it anywhere ...
NetReceive() may call eth_send(), but that only expands into dev->send()
Let's look... The code does NetSetHandler(TftpHandler). I think NetReceive() calls (*packetHandler)() = TftpHandler and this one may call NetStartAgain().
the NetRxPackets[] are set up for you by default and are merely a convenience. you can use them or not, it doesnt really matter. after all, your driver is what calls NetReceive() and the first argument there is the buffer that you're receiving. none of the internal network code relies on these pointers.
I see. Thanks for your mail.
On Monday 02 November 2009 10:08:00 Krzysztof Halasa wrote:
Mike Frysinger vapier@gentoo.org writes:
it's on purpose because it makes the code simpler -- no need to maintain state. drivers have to be able to handle halt() irregardless of init(). i dont see this being a problem for anyone.
Ok. Sure, that's not a problem for me, I just noted the README doesn't talk about this.
that's because the README covered whatever i could think of at the time, and i basically brain dumped what little i knew of the net framework. getting more people to read & question edge cases is good so those can be filled out.
(i already posted a patch for your first and last point)
- dev->recv() seems to be called recursively, for example while doing "dhcp" or "bootp" (ping is ok). dev->recv() in my driver calls NetReceive(), which in turn (without returning to the caller, i.e., dev->recv(), first) reinitializes the driver on error (calls dev->halt() and dev->init()). This makes a lot of mess in the driver, should it stay this way? Perhaps we should queue the received packets and process them on return from dev->recv()? Or maybe return all those packets together?
where exactly do you see that call path ? i dont see it anywhere ...
NetReceive() may call eth_send(), but that only expands into dev->send()
Let's look... The code does NetSetHandler(TftpHandler). I think NetReceive() calls (*packetHandler)() = TftpHandler and this one may call NetStartAgain().
but this doesnt call recv(), and NetStartAgain() changes the handler, so i still dont see recursion with the recv() function. but perhaps your definition of recursive is different from mine. recv() does not turn around and call recv(), but recv() may turn around and call other driver functions.
the tftp error behavior is to basically discard all pending packets, so in practice, it shouldnt be a big deal. it would probably be cleaner though if the packet handler returned an integer indicating errors that NetReceive() would pass up so that the driver recv knew to stop processing packets right away. Ben would have to comment here though as this is beyond the extent of my networking knowledge. -mike
Mike Frysinger vapier@gentoo.org writes:
Let's look... The code does NetSetHandler(TftpHandler). I think NetReceive() calls (*packetHandler)() = TftpHandler and this one may call NetStartAgain().
but this doesnt call recv(), and NetStartAgain() changes the handler, so i still dont see recursion with the recv() function. but perhaps your definition of recursive is different from mine. recv() does not turn around and call recv(), but recv() may turn around and call other driver functions.
I haven't debugged it yet (is there a way to print a backtrace?) but the first recv() doesn't return from NetReceive().
the tftp error behavior is to basically discard all pending packets, so in practice, it shouldnt be a big deal. it would probably be cleaner though if the packet handler returned an integer indicating errors that NetReceive() would pass up so that the driver recv knew to stop processing packets right away.
I think so. I'll look at it too.
On Monday 02 November 2009 18:38:59 Krzysztof Halasa wrote:
Mike Frysinger writes:
Let's look... The code does NetSetHandler(TftpHandler). I think NetReceive() calls (*packetHandler)() = TftpHandler and this one may call NetStartAgain().
but this doesnt call recv(), and NetStartAgain() changes the handler, so i still dont see recursion with the recv() function. but perhaps your definition of recursive is different from mine. recv() does not turn around and call recv(), but recv() may turn around and call other driver functions.
I haven't debugged it yet (is there a way to print a backtrace?) but the first recv() doesn't return from NetReceive().
the Blackfin port has a way of dumping a backtrace, but i dont know about any other arch -mike
participants (2)
-
Krzysztof Halasa -
Mike Frysinger