WAN: bit and/or confusion

Fix the tests that check whether Frame* bits are not set

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
// vi drivers/net/wan/dscc4.c +307
#define FrameVfr	0x80
#define FrameRdo        0x40
#define FrameCrc	0x20
#define FrameRab	0x10

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Roel Kluin <roel.kluin@gmail.com>
Date: Fri, 14 Aug 2009 14:51:46 +0200

> Fix the tests that check whether Frame* bits are not set
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, 14 Aug 2009 14:51:46 +0200
Roel Kluin <roel.kluin@gmail.com> wrote:

> Fix the tests that check whether Frame* bits are not set
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> // vi drivers/net/wan/dscc4.c +307
> #define FrameVfr	0x80
> #define FrameRdo        0x40
> #define FrameCrc	0x20
> #define FrameRab	0x10
> 
> diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
> index 8face5d..dd3c64a 100644
> --- a/drivers/net/wan/dscc4.c
> +++ b/drivers/net/wan/dscc4.c
> @@ -663,9 +663,9 @@ static inline void dscc4_rx_skb(struct dscc4_dev_priv *dpriv,
>  	} else {
>  		if (skb->data[pkt_len] & FrameRdo)
>  			dev->stats.rx_fifo_errors++;
> -		else if (!(skb->data[pkt_len] | ~FrameCrc))
> +		else if (!(skb->data[pkt_len] & ~FrameCrc))
>  			dev->stats.rx_crc_errors++;

that's

	if (!(x & 0xffffffdf))

which seems peculiar.  Should it have been

	else if (skb->data[pkt_len] & FrameCrc)

or

	else if (!(skb->data[pkt_len] & FrameCrc))


> -		else if (!(skb->data[pkt_len] | ~(FrameVfr | FrameRab)))
> +		else if (!(skb->data[pkt_len] & ~(FrameVfr | FrameRab)))
>  			dev->stats.rx_length_errors++;
>  		else
>  			dev->stats.rx_errors++;


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Andrew Morton <akpm@linux-foundation.org>
Date: Fri, 14 Aug 2009 16:36:44 -0700

> On Fri, 14 Aug 2009 14:51:46 +0200
> Roel Kluin <roel.kluin@gmail.com> wrote:
> 
>> @@ -663,9 +663,9 @@ static inline void dscc4_rx_skb(struct dscc4_dev_priv *dpriv,
>>  	} else {
>>  		if (skb->data[pkt_len] & FrameRdo)
>>  			dev->stats.rx_fifo_errors++;
>> -		else if (!(skb->data[pkt_len] | ~FrameCrc))
>> +		else if (!(skb->data[pkt_len] & ~FrameCrc))
>>  			dev->stats.rx_crc_errors++;
> 
> that's
> 
> 	if (!(x & 0xffffffdf))
> 
> which seems peculiar.  Should it have been
> 
> 	else if (skb->data[pkt_len] & FrameCrc)
> 
> or
> 
> 	else if (!(skb->data[pkt_len] & FrameCrc))

Indeed, I can't tell which variant would be correct.

I'm reverting until someone with a datasheet for this chip speaks up
:-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, 14 Aug 2009 16:41:23 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Fri, 14 Aug 2009 16:36:44 -0700
> 
> > On Fri, 14 Aug 2009 14:51:46 +0200
> > Roel Kluin <roel.kluin@gmail.com> wrote:
> > 
> >> @@ -663,9 +663,9 @@ static inline void dscc4_rx_skb(struct dscc4_dev_priv *dpriv,
> >>  	} else {
> >>  		if (skb->data[pkt_len] & FrameRdo)
> >>  			dev->stats.rx_fifo_errors++;
> >> -		else if (!(skb->data[pkt_len] | ~FrameCrc))
> >> +		else if (!(skb->data[pkt_len] & ~FrameCrc))
> >>  			dev->stats.rx_crc_errors++;
> > 
> > that's
> > 
> > 	if (!(x & 0xffffffdf))
> > 
> > which seems peculiar.  Should it have been
> > 
> > 	else if (skb->data[pkt_len] & FrameCrc)
> > 
> > or
> > 
> > 	else if (!(skb->data[pkt_len] & FrameCrc))
> 
> Indeed, I can't tell which variant would be correct.
> 
> I'm reverting until someone with a datasheet for this chip speaks up
> :-)

http://www.datasheet.in/download.php?id=39415

Page 383 and 384 say that bit 5 (CRC) is zero if the rx frame contained
errors.


So we need

	else if (!(skb->data[pkt_len] & FrameCrc))

> -		else if (!(skb->data[pkt_len] | ~(FrameVfr | FrameRab)))
> +		else if (!(skb->data[pkt_len] & ~(FrameVfr | FrameRab)))


vfr is "valid frame".  0 is invalid.

rab is "receive message aborted".  The data sheet doesn't actually say
if the bit is active-high or active-low (grr).  

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html