[v2,16/17] net/dp8393x: repair can_receive() method

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 <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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 <hpoussin@reactos.org>
> ---
>  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?

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 <hpoussin@reactos.org>
>> ---
>>   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é