Re: [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method

[Hervé Poussineau]

Le 02/06/2015 13:05, Aurelien Jarno a écrit :
> On 2015-05-27 14:19, Hervé Poussineau wrote:
> > Datasheet clearly says that RXDIS flag prevents reception, but says
> > nothing about a required presence of RXEN flag.
> >
> > While at it, fix the style of the following if statement.
> >
> > Signed-off-by: Hervé Poussineau <address@hidden>
> > ---
> >   hw/net/dp8393x.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> > index 95a4d3d..b81036a 100644
> > --- a/hw/net/dp8393x.c
> > +++ b/hw/net/dp8393x.c
> > @@ -622,10 +622,12 @@ static int dp8393x_can_receive(NetClientState *nc)
> >   {
> >       dp8393xState *s = qemu_get_nic_opaque(nc);
> >
> > -    if (!(s->regs[SONIC_CR] & SONIC_CR_RXEN))
> > +    if (s->regs[SONIC_CR] & SONIC_CR_RXDIS) {
> >           return 0;
> > -    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE)
> > +    }
> > +    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE) {
> >           return 0;
> > +    }
> >       return 1;
> >   }
>
> I don't have the datasheet at hand, but what would be the point of such
> a RXEN flag if it is ignored? Are you sure it's not because it's enabled
> by default?

After double checking the datasheet:
- RXEN "enables the receive buffer management [...]. Setting this bit resets the 
RXDIS bit"
- "Setting [RXDIS] disables the receiver from buffering data to memory or the 
Receive FIFO. [...] The RXEN bit is reset when the receiver is disabled."

So RXEN and RXDIS are mutually exclusive, except in transition phases when a 
packet is currently received and you're setting RXEN/RXDIS.
As QEMU doesn't emulate the period of reception of a packet (packet is received 
at once), both flag checks are equivalent.

So, I'll withdraw this patch in v3.

Hervé