Hi Joe,
On 1 April 2015 at 10:03, Joe Hershberger <joe.hershber...@gmail.com> wrote:
> Hi Simon,
>
>
> On Tue, Mar 31, 2015 at 10:32 PM, Simon Glass <s...@chromium.org> wrote:
>>
>> Hi Joe,
>>
>> On 30 March 2015 at 14:44, Joe Hershberger <joe.hershber...@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 <s...@chromium.org>
>> > Signed-off-by: Joe Hershberger <joe.hershber...@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.
Fair enough - should be documented one way or the other in the uclass
header net.h. I think a case can be made that a 0-length packet should
be handled differently in the uclass if there is any special behaviour
required, i.e. that the uclass should still call free_pkt() but may
skip processing the packet. But I'm really not sure why this happens
at all.
>
>> > 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 <s...@chromium.org>
>> Acked-by: Simon Glass <s...@chromium.org>
>>
>> Regards,
>> Simon
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
