From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ira W. Snyder" Subject: Re: [PATCH 3/6] can: janz-ican3: fix error and byte counters Date: Thu, 19 Jul 2012 13:17:09 -0700 Message-ID: <20120719201709.GK25905@ovro.caltech.edu> References: <20120719155026.GG25905@ovro.caltech.edu> <1342713258-14822-1-git-send-email-iws@ovro.caltech.edu> <500860C7.7080603@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from ovro.ovro.caltech.edu ([192.100.16.2]:55681 "EHLO ovro.ovro.caltech.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751739Ab2GSURN (ORCPT ); Thu, 19 Jul 2012 16:17:13 -0400 Content-Disposition: inline In-Reply-To: <500860C7.7080603@pengutronix.de> Sender: linux-can-owner@vger.kernel.org List-ID: To: Marc Kleine-Budde Cc: linux-can@vger.kernel.org On Thu, Jul 19, 2012 at 09:32:23PM +0200, Marc Kleine-Budde wrote: > On 07/19/2012 05:54 PM, Ira W. Snyder wrote: > > From: "Ira W. Snyder" > > > > The error and byte counter statistics were being incremented > > incorrectly. For example, a TX error would be counted both in tx_errors > > and rx_errors. > > > > This corrects the problem so that tx_errors and rx_errors are only > > incremented for errors caused by packets sent to the bus. Error packets > > generated by the driver are not counted. > > > > The byte counters are only increased for packets which are actually > > transmitted or received from the bus. Error packets generated by the > > driver are not counted. > > > > Signed-off-by: Ira W. Snyder > > --- > > > > Changes for this version: > > - for RX overflow errors, both rx_over_errors and rx_errors are incremented > > > > drivers/net/can/janz-ican3.c | 11 ++++++----- > > 1 files changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c > > index ea1919e..14d5abc 100644 > > --- a/drivers/net/can/janz-ican3.c > > +++ b/drivers/net/can/janz-ican3.c > > @@ -907,8 +907,8 @@ static void ican3_handle_msglost(struct ican3_dev *mod, struct ican3_msg *msg) > > if (skb) { > > cf->can_id |= CAN_ERR_CRTL; > > cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; > > + stats->rx_over_errors++; > > stats->rx_errors++; > > - stats->rx_bytes += cf->can_dlc; > > netif_rx(skb); > > } > > } > > @@ -982,7 +982,6 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) > > > > dev_dbg(mod->dev, "bus error interrupt\n"); > > mod->can.can_stats.bus_error++; > > - stats->rx_errors++; > ^^^^^^^^^^^^^^^^^^ > > What about this one? Accidentally removed? > No, it is correct. You snipped the extra parts of the patch that were relevant. I've re-added them, along with my explanation. > @@ -1001,8 +1000,12 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) > break; > } > > - if ((ecc & ECC_DIR) == 0) > + if (!(ecc & ECC_DIR)) { > cf->data[2] |= CAN_ERR_PROT_TX; > + stats->tx_errors++; > + } else { > + stats->rx_errors++; > + } > The error counter you mention above moved down here. We are now distinguishing between RX bus errors and TX bus errors. > cf->data[6] = txerr; > cf->data[7] = rxerr; > @@ -1028,8 +1031,6 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) > } > > mod->can.state = state; > - stats->rx_errors++; > - stats->rx_bytes += cf->can_dlc; > netif_rx(skb); > return 0; > } And this was just bogus. Before this fix, I was incrementing rx_errors twice when a bus error interrupt was received. Ira > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions | Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | >