Message ID | 4A8D57F8.1050106@gmail.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Roel Kluin <roel.kluin@gmail.com> writes: >> 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(). > 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.
From: Roel Kluin <roel.kluin@gmail.com> Date: Thu, 20 Aug 2009 16:04:40 +0200 > Fix the tests that check whether Frame* bits are not set > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> Applied to net-next-2.6, thanks. -- 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
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: