[U-Boot] [PATCH v5 15/27] dm: eth: Pass the packet pointer as a parameter to recv

[Joe Hershberger]

On Tue, Mar 10, 2015 at 6:31 PM, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Joe,
>
> On 10 March 2015 at 16:28, Joe Hershberger <joe.hershberger at gmail.com>
wrote:
> > On Wed, Mar 4, 2015 at 12:35 PM, Simon Glass <sjg at chromium.org> wrote:
> >>
> >> Hi Joe,
> >>
> >> On 3 March 2015 at 19:41, Joe Hershberger <joe.hershberger at 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 at 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