[U-Boot] [PATCH] dm: eth: Provide a way for drivers to manage packet buffers

[Joe Hershberger]

Hi Simon,

On Tue, Mar 31, 2015 at 10:32 PM, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Joe,
>
> On 30 March 2015 at 14:44, Joe Hershberger <joe.hershberger at ni.com> wrote:
> > Some drivers need a chance to manage their receive buffers after the
> > packet has been handled by the network stack. Add an operation that
> > will allow the driver to be called in that case.
> >
> > Reported-by: Simon Glass <sjg at chromium.org>
> > Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> > ---
> > This patch depends on dm/next
> >
> >  include/net.h | 4 ++++
> >  net/eth.c     | 8 ++++++--
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/net.h b/include/net.h
> > index e7f28d7..f9df532 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -98,6 +98,9 @@ struct eth_pdata {
> >   * 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
> > + * free_pkt: Give the driver an opportunity to manage its packet
buffer memory
> > + *          when the network stack is finished processing it. This
will only be
> > + *          called when a packet was successfully returned from recv -
optional
> >   * stop: Stop the hardware from looking for packets - may be called
even if
> >   *      state == PASSIVE
> >   * mcast: Join or leave a multicast group (for TFTP) - optional
> > @@ -113,6 +116,7 @@ struct eth_ops {
> >         int (*start)(struct udevice *dev);
> >         int (*send)(struct udevice *dev, void *packet, int length);
> >         int (*recv)(struct udevice *dev, uchar **packetp);
> > +       int (*free_pkt)(struct udevice *dev, uchar *packet, int length);
> >         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 13b7723..889ad8f 100644
> > --- a/net/eth.c
> > +++ b/net/eth.c
> > @@ -342,10 +342,14 @@ int eth_rx(void)
> >         /* 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)
> > +               if (ret > 0) {
>
> To match the old net stack behaviour I wonder if we should process the
> packet when it is length 0, and require recv() to return -EAGAIN when
> there is no packet? At least with designware, it processes a 0-length
> packet for some reason, and we need to call free_pkt() in that case.

I pretty much assumed that since the driver is not expecting the network
stack to do anything with the buffer in the retval == 0 case, the driver
would handle its buffer management before returning from recv().

I'm not sure which is more clear to the driver writer... to expect the
free_pkt() call when returning 0 or to not expect it.  I guess my initial
instinct is that you would not expect it.

> >                         net_process_received_packet(packet, ret);
> > -               else
> > +                       if (eth_get_ops(current)->free_pkt)
> > +                               eth_get_ops(current)->free_pkt(current,
packet,
> > +                                                              ret);
> > +               } else {
> >                         break;
> > +               }
> >         }
> >         if (ret == -EAGAIN)
> >                 ret = 0;
> > --
> > 1.7.11.5
> >
>
> Tested on pcduino3:
>
> Tested-by: Simon Glass <sjg at chromium.org>
> Acked-by: Simon Glass <sjg at chromium.org>
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot