On Tue, Mar 10, 2015 at 6:31 PM, Simon Glass <s...@chromium.org> wrote:
>
> Hi Joe,
>
> On 10 March 2015 at 16:28, Joe Hershberger <joe.hershber...@gmail.com>
wrote:
> > On Wed, Mar 4, 2015 at 12:35 PM, Simon Glass <s...@chromium.org> wrote:
> >>
> >> Hi Joe,
> >>
> >> On 3 March 2015 at 19:41, Joe Hershberger <joe.hershber...@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.hershber...@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:
>
> 1. 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.
> 2. 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);
> >> > + else
> >> > + break;
> >> > + }
> >> > + return ret;
> >> > }
> >> >
> >> > static int eth_write_hwaddr(struct udevice *dev)
> >> > --
>
> Regards,
> Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
