[U-Boot] Driver model Ethernet issue

[Simon Glass]

+mailing list which got dropped off at some point

Hi Joe,

On 30 March 2015 at 11:50, Joe Hershberger <joe.hershberger at gmail.com> wrote:
> Hi Simon,
>
> On Thu, Mar 26, 2015 at 3:11 AM, Joe Hershberger <joe.hershberger at gmail.com>
> wrote:
>>
>> Hi Simon,
>>
>> On Wed, Mar 25, 2015 at 10:10 PM, Simon Glass <sjg at chromium.org> wrote:
>> >
>> > Hi Joe,
>> >
>> > I think moving the packet processing into the uclass was a good idea,
>> > but I did not think it through.
>> >
>> > If you look at the designware driver, when it receives a packet it
>> > processes it and then retires that buffer for later reuse. The few
>> > others I have looked at are similar.
>> >
>> > We can't retire it in the recv() method since the packet is still
>> > needed at that point, and may be overwritten by a new packet if we
>> > retire its buffer space.
>> >
>> > I am wondering if we should have a new method, like:
>> >
>> > int free_pkt(struct udevice *dev, uchar *packet, int len)
>> >
>> > which is called by the uclass after the packet is processed (assuming
>> > that recv() returns a packet). It would pass the packet pointer and
>> > length to the driver.
>>
>> In light of this behavior that I also did not realize some drivers were
>> exhibiting, it is not simpler to revert the change? It seems that we are
>> adding complexity overall by having two functions to be implemented bu
>> drivers instead of one function to be called by the driver. Perhaps you can
>> convince me that the two functions are a better design, but I'd like to hear
>> the argument.
>
> What is your preference here? I can push the change either way. I can
> certainly see how it's a bit unusual to have a driver call back into the
> stack. Do you think two ops makes the driver cleaner than having the drivers
> call one function to process a packet?

Sorry for not coming back on this earlier. I was planning on doing
another spin of the sunxi Ethernet series but I have not got to it.

I do have a preference for keeping the control flow out of the driver.
It worries me that the driver calls a packet reception function, and
then the driver sits in the call stack for a while while it is
processed. Seems wrong to me.

Also you can argue that buffer management is a reasonable thing to
provide a method for. Using a buffer to receive a packet, sending that
packet for processing, and retiring the buffer are all conceptually
separate things. We could even consider (in the future!) supporting a
buffer ring in the uclass since all of the drivers I have looked at
have one.

So yes I think a new method is the best way to handle this situation.

>
>> > This would allow the driver to do what it needs to do.
>> >
>> > What do you think? I'd like to figure this out and send v2 of the
>> > sunxi Ethernet conversion.
>>
>> One comment about new driver-model network drivers is I'd like to see new
>> driver header files (if needed) live in include/net. I figure DM is a good
>> excuse to enforce that these headers get organized a bit.

Sounds good.

>
> Thanks,
> -Joe

Regards,
Simon