On Tue, Mar 10, 2015 at 6:31 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 10 March 2015 at 16:28, Joe Hershberger joe.hershberger@gmail.com
wrote:
On Wed, Mar 4, 2015 at 12:35 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 3 March 2015 at 19:41, Joe Hershberger joe.hershberger@ni.com
wrote:
Stop forcing drivers to call net_process_received_packet() - formerly called NetReceive(). Now the uclass will handle calling the driver
for
each packet until the driver errors or has nothing to return. The
uclass
will then pass the good packets off to the network stack by calling net_process_received_packet().
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Changes in v5: -New to v5
Changes in v4: None Changes in v3: None Changes in v2: None
include/net.h | 2 +- net/eth.c | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/net.h b/include/net.h index 4e44832..37d1f36 100644 --- a/include/net.h +++ b/include/net.h @@ -110,7 +110,7 @@ struct eth_pdata { struct eth_ops { int (*start)(struct udevice *dev); int (*send)(struct udevice *dev, void *packet, int length);
int (*recv)(struct udevice *dev);
int (*recv)(struct udevice *dev, uchar **packetp);Need to update docs above. With serial we return -EAGAIN when there is nothing more to receive. So you might want to swallow that error instead of returning it from eth_rx().
I was doing this filtering in the sandbox-raw driver, but I can easily
to it
here instead (or too) so that it doesn't leak through from other drivers that do not do this check in the future.
OK. My main concerns are:
- Avoid wait loops in drivers (if needed they should be in the uclass
triggered by -EAGAIN)
Drivers are expected to return and not wait. I've made this more clear in the function comment.
- Ensure that uclass ops methods are clearly documented as to
purpose, parameters, return value, etc, so people don't have to resort to archaeology to do the right thing :-)
The new function comment looks like this:
+ * recv: Check if the hardware received a packet. If so, set the pointer to the + * packet buffer in the packetp parameter. If not, return an error or 0 to + * indicate that the hardware receive FIFO is empty
void (*stop)(struct udevice *dev);#ifdef CONFIG_MCAST_TFTP int (*mcast)(struct udevice *dev, const u8 *enetaddr, int
join);
diff --git a/net/eth.c b/net/eth.c index 1abf027..b66d253 100644 --- a/net/eth.c +++ b/net/eth.c @@ -259,6 +259,9 @@ int eth_send(void *packet, int length) int eth_rx(void) { struct udevice *current;
uchar *packet;int ret;int i; current = eth_get_dev(); if (!current)@@ -267,7 +270,15 @@ int eth_rx(void) if (!device_active(current)) return -EINVAL;
return eth_get_ops(current)->recv(current);
/* Process up to 32 packets at one time */for (i = 0; i < 32; i++) {ret = eth_get_ops(current)->recv(current, &packet);if (ret > 0)net_process_received_packet(packet, ret);elsebreak;}return ret;}
static int eth_write_hwaddr(struct udevice *dev)
Regards, Simon