From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Kleine-Budde Subject: Re: [PATCH 3/6] can: janz-ican3: fix error and byte counters Date: Thu, 19 Jul 2012 10:11:38 +0200 Message-ID: <5007C13A.3070809@pengutronix.de> References: <1342650798-2991-1-git-send-email-iws@ovro.caltech.edu> <1342650798-2991-4-git-send-email-iws@ovro.caltech.edu> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigADF4FF0C41768C4847A521C1" Return-path: Received: from metis.ext.pengutronix.de ([92.198.50.35]:38157 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753723Ab2GSILs (ORCPT ); Thu, 19 Jul 2012 04:11:48 -0400 In-Reply-To: <1342650798-2991-4-git-send-email-iws@ovro.caltech.edu> Sender: linux-can-owner@vger.kernel.org List-ID: To: "Ira W. Snyder" Cc: linux-can@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigADF4FF0C41768C4847A521C1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 07/19/2012 12:33 AM, Ira W. Snyder wrote: > From: "Ira W. Snyder" >=20 > 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. >=20 > 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. >=20 > 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. >=20 > Signed-off-by: Ira W. Snyder Looking at the patch (and not the complete driver) I have some questions. See inline. > --- > drivers/net/can/janz-ican3.c | 13 ++++++------- > 1 files changed, 6 insertions(+), 7 deletions(-) >=20 > diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.= c > index ea1919e..d9d4ed1 100644 > --- a/drivers/net/can/janz-ican3.c > +++ b/drivers/net/can/janz-ican3.c > @@ -907,8 +907,7 @@ static void ican3_handle_msglost(struct ican3_dev *= mod, struct ican3_msg *msg) > if (skb) { > cf->can_id |=3D CAN_ERR_CRTL; > cf->data[1] =3D CAN_ERR_CRTL_RX_OVERFLOW; > - stats->rx_errors++; ^^^^^^^^^^^^^^^^^^^ > - stats->rx_bytes +=3D cf->can_dlc; > + stats->rx_over_errors++; I've checked the sja1000 and the mscan driver, on an rx overflow error they increment both rx_errors and rx_over_errors. > netif_rx(skb); > } > } > @@ -956,7 +955,6 @@ static int ican3_handle_cevtind(struct ican3_dev *m= od, struct ican3_msg *msg) > cf->can_id |=3D CAN_ERR_CRTL; > cf->data[1] =3D CAN_ERR_CRTL_RX_OVERFLOW; > stats->rx_over_errors++; > - stats->rx_errors++; dito > } > =20 > /* error warning + passive interrupt */ > @@ -982,7 +980,6 @@ static int ican3_handle_cevtind(struct ican3_dev *m= od, struct ican3_msg *msg) > =20 > dev_dbg(mod->dev, "bus error interrupt\n"); > mod->can.can_stats.bus_error++; > - stats->rx_errors++; this rx_errors probably moved to the one below ... > cf->can_id |=3D CAN_ERR_PROT | CAN_ERR_BUSERROR; > =20 > switch (ecc & ECC_MASK) { > @@ -1001,8 +998,12 @@ static int ican3_handle_cevtind(struct ican3_dev = *mod, struct ican3_msg *msg) > break; > } > =20 > - if ((ecc & ECC_DIR) =3D=3D 0) > + if (!(ecc & ECC_DIR)) { > cf->data[2] |=3D CAN_ERR_PROT_TX; > + stats->tx_errors++; > + } else { > + stats->rx_errors++; =2E..here. right? > + } > =20 > cf->data[6] =3D txerr; > cf->data[7] =3D rxerr; > @@ -1028,8 +1029,6 @@ static int ican3_handle_cevtind(struct ican3_dev = *mod, struct ican3_msg *msg) > } > =20 > mod->can.state =3D state; > - stats->rx_errors++; > - stats->rx_bytes +=3D cf->can_dlc; > netif_rx(skb); > return 0; > } Marc --=20 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 | --------------enigADF4FF0C41768C4847A521C1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlAHwT4ACgkQjTAFq1RaXHMrUQCdHTOIWjfdli9uM/j+afFB5wbd KLIAnjfWWCayXAyLFdreFzkxyd6Hl2Qp =OA3f -----END PGP SIGNATURE----- --------------enigADF4FF0C41768C4847A521C1--