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

[Simon Glass]

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)
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 :-)

>
>> >         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