Re: [PATCH] WAN: bit and/or confusion
From: Krzysztof Halasa <khc@pm.waw.pl>
Date: 2009-08-20 14:25:14
Roel Kluin [off-list ref] writes:
quoted
Perhaps something like the following should be better? u8 status = ~skb->data[pkt_len]; if (status == 0) looks_good...; else { if (status & FrameRab) ... if (status & FrameVfr) ... etc. rx_errors++; }I don't understand your suggestion - why status == 0? doesn't the patch below do what you want instead?
Because I think (didn't read the manual) that these (inverted) bits represent specific errors. So I suggested inverting them, then treating as separate error bits, it should be easier to read. That's only a suggestion of course (unless someone sends me the/a card - non-returnable "donations" only please - and I can work on it personally). IOW all (inverted) bits = 1 => all bits zero after inversion => no errors. This changes functionality a bit, and would need to be checked. Otherwise, you could test: if ((status & FrameOk) == 0) then looks_good().
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c index 8face5d..88534b6 100644 --- a/drivers/net/wan/dscc4.c +++ b/drivers/net/wan/dscc4.c@@ -663,12 +663,12 @@ 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++; - else if (!(skb->data[pkt_len] | ~(FrameVfr | FrameRab))) + else if ((skb->data[pkt_len] & (FrameVfr | FrameRab)) != + FrameVfr | FrameRab) dev->stats.rx_length_errors++; - else - dev->stats.rx_errors++; + dev->stats.rx_errors++; dev_kfree_skb_irq(skb); } refill:
Guess it would do. -- Krzysztof Halasa