Re: [Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Greg Ungerer <gerg@uclinux.org>]

Hi Stefan,

On 26/06/15 20:12, Stefan Hajnoczi wrote:
> On Fri, Jun 26, 2015 at 03:27:12PM +1000, gerg@uclinux.org wrote:
>> The following set of patches fixes the emulated ColdFire ethernet fec
>> driver. There is primarily two problems that need to be fixed.
>>
>> 1. The emulated driver needs to support probing of an attached phy.
>>     It is strait forward to emulate an attached phy, but to avoid using
>>     magic numbers I have factored out the common MII register and value
>>     definitions into their own mii.h file first.
>>
>> 2. Fix the fec driver receiver to return the correct value.
>>
>> With these changes in place the qemu m5208evb board emulation can probe,
>> identify and use the fec ethernet running a Linux guest.
>>
>>
>>   hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
>>   include/hw/net/allwinner_emac.h |   40 ---------------------
>>   include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 128 insertions(+), 42 deletions(-)
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> If this series has no maintainer to go through I'll merge it via my net
> tree.  Please let me know if you'd like that.

That would be great. Please do merge via your tree.
Thanks for the review.

> mcf_fec is not fully correct yet, it needs to call
> qemu_flush_queued_packets() when rx_enabled transitions from 0 to 1.
> This will restart rx by sending any queued packets from the net
> subsystem.
>
> In order to get flow control between NetClientState peers working the
> following is missing:
>
> 1. mcf_fec_receive()'s !s->rx_enabled case should return -1 to drop
>     unexpected packets.
>
> 2. mcf_fec_receive() should return 0 in the (bd.flags & FEC_BD_E) == 0
>     case where we've run out of rx buffers.  That way the net subsystem
>     queues the packet and waits until the next RDAR write transitions
>     rx_enabled from 0 to 1.

Ok, thanks for the pointers. I'll work up some additional changes
that handle these cases.

Regards
Greg