* [U-Boot] Net driver questions
@ 2009-11-01 23:07 Krzysztof Halasa
2009-11-02 3:32 ` Mike Frysinger
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Halasa @ 2009-11-01 23:07 UTC (permalink / raw)
To: u-boot
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?
--
Krzysztof Halasa
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Net driver questions
2009-11-01 23:07 [U-Boot] Net driver questions Krzysztof Halasa
@ 2009-11-02 3:32 ` Mike Frysinger
2009-11-02 15:08 ` Krzysztof Halasa
0 siblings, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2009-11-02 3:32 UTC (permalink / raw)
To: u-boot
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
-------------- 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/20091101/d705474f/attachment.pgp
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Net driver questions
2009-11-02 3:32 ` Mike Frysinger
@ 2009-11-02 15:08 ` Krzysztof Halasa
2009-11-02 22:53 ` Mike Frysinger
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Halasa @ 2009-11-02 15:08 UTC (permalink / raw)
To: u-boot
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.
--
Krzysztof Halasa
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Net driver questions
2009-11-02 15:08 ` Krzysztof Halasa
@ 2009-11-02 22:53 ` Mike Frysinger
2009-11-02 23:38 ` Krzysztof Halasa
0 siblings, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2009-11-02 22:53 UTC (permalink / raw)
To: u-boot
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
-------------- 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/20091102/f23f0903/attachment.pgp
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Net driver questions
2009-11-02 22:53 ` Mike Frysinger
@ 2009-11-02 23:38 ` Krzysztof Halasa
2009-11-02 23:54 ` Mike Frysinger
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Halasa @ 2009-11-02 23:38 UTC (permalink / raw)
To: u-boot
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.
--
Krzysztof Halasa
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Net driver questions
2009-11-02 23:38 ` Krzysztof Halasa
@ 2009-11-02 23:54 ` Mike Frysinger
0 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2009-11-02 23:54 UTC (permalink / raw)
To: u-boot
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
-------------- 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/20091102/e7a305fb/attachment.pgp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-11-02 23:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-01 23:07 [U-Boot] Net driver questions Krzysztof Halasa
2009-11-02 3:32 ` Mike Frysinger
2009-11-02 15:08 ` Krzysztof Halasa
2009-11-02 22:53 ` Mike Frysinger
2009-11-02 23:38 ` Krzysztof Halasa
2009-11-02 23:54 ` Mike Frysinger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.