From: Andri Yngvason <andri.yngvason@marel.com>
To: "Ahmed S. Darwish" <darwish.07@gmail.com>,
Wolfgang Grandegger <wg@grandegger.com>
Cc: Olivier Sobrie <olivier@sobrie.be>,
Oliver Hartkopp <socketcan@hartkopp.net>,
Marc Kleine-Budde <mkl@pengutronix.de>,
Linux-CAN <linux-can@vger.kernel.org>,
netdev <netdev@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
Date: Wed, 21 Jan 2015 15:00:15 +0000 [thread overview]
Message-ID: <20150121150015.31351.6605@shannon> (raw)
In-Reply-To: <20150121144323.GA17070@linux>
Quoting Ahmed S. Darwish (2015-01-21 14:43:23)
> Hi!
>
> On Wed, Jan 21, 2015 at 12:53:58PM +0100, Wolfgang Grandegger wrote:
> > On Wed, 21 Jan 2015 10:33:19 +0000, Andri Yngvason
> > <andri.yngvason@marel.com> wrote:
> > > Quoting Ahmed S. Darwish (2015-01-20 21:45:37)
> > >> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> > >>
> > >> Replace most of the can interface's state and error counters
> > >> handling with the new can-dev can_change_state() mechanism.
> > >>
> > >> Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
> > >> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> > >> ---
> > >> drivers/net/can/usb/kvaser_usb.c | 114
> > >> +++++++++++++++++++--------------------
> > >> 1 file changed, 55 insertions(+), 59 deletions(-)
> > >>
> > >> diff --git a/drivers/net/can/usb/kvaser_usb.c
> > >> b/drivers/net/can/usb/kvaser_usb.c
> > >> index 971c5f9..0386d3f 100644
> > >> --- a/drivers/net/can/usb/kvaser_usb.c
> > >> +++ b/drivers/net/can/usb/kvaser_usb.c
> >
> > ...
> > >
> > > Looks good.
> >
> > Would be nice to see some "candump" traces as well.
>
> Sure. The USBCan-II device trace below is generated after applying
> all patches in the series, especially patch #3, which fixes some
> some invalid CAN state transitions logic in the original driver.
[...]
>
> Afterwards, candump on a PC, Kvaser USB device on the sending end:
>
> ...
> (000.008784) can0 60A [1] C1
> (000.011341) can0 2A8 [8] C2 0A 00 00 00 00 00 00
> (000.009873) can0 03D [7] C3 0A 00 00 00 00 00
> (000.010394) can0 55C [8] C4 0A 00 00 00 00 00 00
> (000.009979) can0 45A [8] C5 0A 00 00 00 00 00 00
> (000.010125) can0 6E8 [8] C6 0A 00 00 00 00 00 00
> (000.010149) can0 4EE [8] C7 0A 00 00 00 00 00 00
> (000.010102) can0 5D2 [8] C8 0A 00 00 00 00 00 00
> (000.010000) can0 61F [8] C9 0A 00 00 00 00 00 00
> (000.010271) can0 5F8 [8] CA 0A 00 00 00 00 00 00
>
> <-- Unplug the cable -->
>
> (000.009106) can0 20000080 [8] 00 00 00 00 00 00 08 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{8}{0}}
> (000.001872) can0 20000080 [8] 00 00 00 00 00 00 10 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{16}{0}}
[...]
> error-counter-tx-rx{{80}{0}}
> (000.001910) can0 20000080 [8] 00 00 00 00 00 00 58 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{88}{0}}
> (000.001753) can0 20000084 [8] 00 08 00 00 00 00 60 00 ERRORFRAME
> controller-problem{tx-error-warning}
Good.
> bus-error
> error-counter-tx-rx{{96}{0}}
> (000.001720) can0 20000080 [8] 00 00 00 00 00 00 68 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{104}{0}}
> (000.001876) can0 20000080 [8] 00 00 00 00 00 00 70 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{112}{0}}
> (000.001749) can0 20000080 [8] 00 00 00 00 00 00 78 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{120}{0}}
> (000.001771) can0 20000084 [8] 00 20 00 00 00 00 80 00 ERRORFRAME
> controller-problem{tx-error-passive}
Also good.
> bus-error
> error-counter-tx-rx{{128}{0}}
> (000.001868) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{128}{0}}
> (000.001982) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{128}{0}}
>
> (( Then a continous flood, exactly similar to the above packet, appears.
> Unfortunately this flooding is a firmware problem. ))
>
> <-- Replug the cable, after a good amount of time -->
>
Where are the reverse state transitions?
>
> (000.520485) can0 33D [4] CB 0A 00 00
> (000.002693) can0 42E [8] CC 0A 00 00 00 00 00 00
> (000.001795) can0 319 [4] CD 0A 00 00
> (000.002705) can0 3B1 [8] CE 0A 00 00 00 00 00 00
> (000.001295) can0 4CC [2] CF 0A
> (000.002205) can0 42B [6] D0 0A 00 00 00 00
> (000.001620) can0 5A2 [3] D1 0A 00
> (000.002636) can0 691 [8] D2 0A 00 00 00 00 00 00
> (000.002615) can0 36A [8] D3 0A 00 00 00 00 00 00
> (000.001729) can0 068 [4] D4 0A 00 00
> (000.001195) can0 4C8 [1] D5
> ...
>
> ##########################################################################
>
> Bus-off Testing:
>
> candump on a PC, Kvaser device on the sending end. An i.mx6 ARM
> board with flexcan is on the receiving end:
>
> (000.010319) can0 5CC [8] 90 02 00 00 00 00 00 00
> (000.008747) can0 679 [1] 91
> (000.011442) can0 011 [8] 92 02 00 00 00 00 00 00
> (000.008991) can0 631 [2] 93 02
> (000.011097) can0 532 [7] 94 02 00 00 00 00 00
> (000.009781) can0 0A9 [5] 95 02 00 00 00
> (000.010792) can0 1DD [8] 96 02 00 00 00 00 00 00
> (000.010026) can0 44E [8] 97 02 00 00 00 00 00 00
> (000.010181) can0 76A [8] 98 02 00 00 00 00 00 00
> (000.008867) can0 1A5 [1] 99
> (000.011322) can0 2B4 [8] 9A 02 00 00 00 00 00 00
>
> <-- Unplug the can low and high wires from the board -->
>
> (000.009688) can0 20000080 [8] 00 00 00 00 00 00 08 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{8}{0}}
> (000.002246) can0 20000080 [8] 00 00 00 00 00 00 10 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{16}{0}}
> (000.002124) can0 20000080 [8] 00 00 00 00 00 00 18 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{24}{0}}
> (000.002115) can0 20000080 [8] 00 00 00 00 00 00 20 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{32}{0}}
> (000.002132) can0 20000080 [8] 00 00 00 00 00 00 28 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{40}{0}}
> (000.002266) can0 20000080 [8] 00 00 00 00 00 00 30 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{48}{0}}
> (000.002187) can0 20000080 [8] 00 00 00 00 00 00 38 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{56}{0}}
> (000.002046) can0 20000080 [8] 00 00 00 00 00 00 40 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{64}{0}}
> (000.002076) can0 20000080 [8] 00 00 00 00 00 00 48 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{72}{0}}
> (000.002406) can0 20000080 [8] 00 00 00 00 00 00 50 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{80}{0}}
> (000.001969) can0 20000080 [8] 00 00 00 00 00 00 58 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{88}{0}}
> (000.002388) can0 20000084 [8] 00 08 00 00 00 00 60 00 ERRORFRAME
> controller-problem{tx-error-warning}
> bus-error
> error-counter-tx-rx{{96}{0}}
> (000.002021) can0 20000080 [8] 00 00 00 00 00 00 68 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{104}{0}}
> (000.002110) can0 20000080 [8] 00 00 00 00 00 00 70 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{112}{0}}
> (000.002155) can0 20000080 [8] 00 00 00 00 00 00 78 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{120}{0}}
> (000.002140) can0 20000084 [8] 00 20 00 00 00 00 80 00 ERRORFRAME
> controller-problem{tx-error-passive}
> bus-error
> error-counter-tx-rx{{128}{0}}
> (000.002324) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{128}{0}}
> (000.002416) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{128}{0}}
> (000.002237) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{128}{0}}
>
> (( Then a continous flood, exactly similar to the above packet, appears ))
>
> <-- Short-circuit the can high and low wires -->
>
> (000.002364) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{128}{0}}
> (000.002108) can0 20000080 [8] 00 00 00 00 00 00 88 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{136}{0}}
> (000.000494) can0 20000080 [8] 00 00 00 00 00 00 90 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{144}{0}}
> (000.000523) can0 20000080 [8] 00 00 00 00 00 00 98 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{152}{0}}
> (000.000661) can0 20000080 [8] 00 00 00 00 00 00 A0 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{160}{0}}
> (000.000464) can0 20000080 [8] 00 00 00 00 00 00 A8 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{168}{0}}
> (000.000534) can0 20000080 [8] 00 00 00 00 00 00 B0 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{176}{0}}
> (000.000499) can0 20000080 [8] 00 00 00 00 00 00 B8 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{184}{0}}
> (000.000626) can0 20000080 [8] 00 00 00 00 00 00 C0 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{192}{0}}
> (000.000373) can0 20000080 [8] 00 00 00 00 00 00 C8 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{200}{0}}
> (000.000627) can0 20000080 [8] 00 00 00 00 00 00 D0 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{208}{0}}
> (000.000507) can0 20000080 [8] 00 00 00 00 00 00 D8 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{216}{0}}
> (000.000501) can0 20000080 [8] 00 00 00 00 00 00 E0 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{224}{0}}
> (000.000459) can0 20000080 [8] 00 00 00 00 00 00 E8 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{232}{0}}
> (000.000606) can0 20000080 [8] 00 00 00 00 00 00 F0 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{240}{0}}
> (000.000454) can0 20000080 [8] 00 00 00 00 00 00 F8 00 ERRORFRAME
> bus-error
> error-counter-tx-rx{{248}{0}}
> (000.000664) can0 200000C0 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> bus-off
> bus-error
>
> (( No further bus activity ))
>
Bus-off seems OK. You could have just short circuited them without
disconnecting.
Reverse state transitions are missing from the logs. See comments above.
--
Andri
next prev parent reply other threads:[~2015-01-21 15:00 UTC|newest]
Thread overview: 98+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-23 15:46 [PATCH] can: kvaser_usb: Don't free packets when tight on URBs Ahmed S. Darwish
2014-12-23 15:53 ` [PATCH] can: kvaser_usb: Add support for the Usbcan-II family Ahmed S. Darwish
2014-12-24 12:36 ` Olivier Sobrie
2014-12-24 15:04 ` Ahmed S. Darwish
2014-12-28 21:51 ` Olivier Sobrie
2014-12-30 15:33 ` Ahmed S. Darwish
2014-12-31 12:13 ` Olivier Sobrie
2014-12-24 12:31 ` [PATCH] can: kvaser_usb: Don't free packets when tight on URBs Olivier Sobrie
2014-12-24 15:52 ` Ahmed S. Darwish
2014-12-24 23:56 ` [PATCH v2 1/4] " Ahmed S. Darwish
2014-12-24 23:59 ` [PATCH v2 2/4] can: kvaser_usb: Reset all URB tx contexts upon channel close Ahmed S. Darwish
2014-12-25 0:02 ` [PATCH v2 3/4] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels Ahmed S. Darwish
2014-12-25 0:04 ` [PATCH v2 4/4] can: kvaser_usb: Add support for the Usbcan-II family Ahmed S. Darwish
2014-12-28 21:54 ` [PATCH v2 3/4] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels Olivier Sobrie
2014-12-25 2:50 ` [PATCH v2 1/4] can: kvaser_usb: Don't free packets when tight on URBs Greg KH
2014-12-25 9:38 ` Ahmed S. Darwish
2014-12-28 21:52 ` Olivier Sobrie
2015-01-01 21:59 ` [PATCH] " Stephen Hemminger
2015-01-03 14:34 ` Ahmed S. Darwish
2015-01-05 17:49 ` [PATCH v3 1/4] " Ahmed S. Darwish
2015-01-05 17:52 ` [PATCH v3 2/4] can: kvaser_usb: Reset all URB tx contexts upon channel close Ahmed S. Darwish
2015-01-05 17:57 ` [PATCH v3 3/4] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels Ahmed S. Darwish
2015-01-05 18:31 ` [PATCH v3 4/4] can: kvaser_usb: Add support for the Usbcan-II family Ahmed S. Darwish
2015-01-08 11:53 ` Marc Kleine-Budde
2015-01-08 15:19 ` Ahmed S. Darwish
2015-01-12 11:51 ` Marc Kleine-Budde
2015-01-12 12:26 ` Ahmed S. Darwish
2015-01-12 12:34 ` Marc Kleine-Budde
2015-01-09 3:06 ` Ahmed S. Darwish
2015-01-09 14:05 ` Marc Kleine-Budde
2015-01-09 16:23 ` Oliver Hartkopp
2015-01-08 9:59 ` [PATCH v3 1/4] can: kvaser_usb: Don't free packets when tight on URBs Marc Kleine-Budde
2015-01-11 20:05 ` [PATCH v4 00/04] can: Introduce support for Kvaser USBCAN-II devices Ahmed S. Darwish
2015-01-11 20:11 ` [PATCH v4 01/04] can: kvaser_usb: Don't dereference skb after a netif_rx() devices Ahmed S. Darwish
2015-01-11 20:15 ` [PATCH v4 2/4] can: kvaser_usb: Update error counters before exiting on OOM Ahmed S. Darwish
2015-01-11 20:36 ` [PATCH v4 3/4] can: kvaser_usb: Add support for the Usbcan-II family Ahmed S. Darwish
2015-01-11 20:45 ` [PATCH v4 4/4] can: kvaser_usb: Retry first bulk transfer on -ETIMEDOUT Ahmed S. Darwish
2015-01-11 20:51 ` Marc Kleine-Budde
2015-01-12 10:14 ` Ahmed S. Darwish
2015-01-12 10:25 ` Marc Kleine-Budde
2015-01-12 13:33 ` Olivier Sobrie
2015-01-12 13:50 ` Ahmed S. Darwish
2015-01-12 11:20 ` [PATCH v4 3/4] can: kvaser_usb: Add support for the Usbcan-II family Ahmed S. Darwish
2015-01-12 11:43 ` Marc Kleine-Budde
2015-01-12 12:07 ` Ahmed S. Darwish
2015-01-12 12:36 ` Ahmed S. Darwish
2015-01-12 13:53 ` Olivier Sobrie
2015-01-18 20:12 ` Ahmed S. Darwish
2015-01-18 20:13 ` Marc Kleine-Budde
2015-01-12 11:09 ` [PATCH v4 2/4] can: kvaser_usb: Update error counters before exiting on OOM Marc Kleine-Budde
2015-01-12 20:36 ` Ahmed S. Darwish
2015-01-16 14:39 ` Marc Kleine-Budde
2015-01-16 15:50 ` Andri Yngvason
2015-01-18 20:33 ` Ahmed S. Darwish
2015-01-11 20:49 ` [PATCH v4 01/04] can: kvaser_usb: Don't dereference skb after a netif_rx() Ahmed S. Darwish
2015-01-12 11:05 ` Marc Kleine-Budde
2015-01-20 21:44 ` [PATCH v5 1/5] can: kvaser_usb: Update net interface state before exiting on OOM Ahmed S. Darwish
2015-01-20 21:45 ` [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling Ahmed S. Darwish
2015-01-20 21:47 ` [PATCH v5 3/5] can: kvaser_usb: Fix state handling upon BUS_ERROR events Ahmed S. Darwish
2015-01-20 21:48 ` [PATCH v5 4/5] can: kvaser_usb: Retry the first bulk transfer on -ETIMEDOUT Ahmed S. Darwish
2015-01-20 21:50 ` [PATCH v5 5/5] can: kvaser_usb: Add support for the USBcan-II family Ahmed S. Darwish
2015-01-21 12:24 ` [PATCH v5 4/5] can: kvaser_usb: Retry the first bulk transfer on -ETIMEDOUT Sergei Shtylyov
2015-01-25 11:59 ` Ahmed S. Darwish
2015-01-21 10:33 ` [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling Andri Yngvason
2015-01-21 10:44 ` Marc Kleine-Budde
2015-01-21 11:00 ` Andri Yngvason
2015-01-21 11:53 ` Wolfgang Grandegger
2015-01-21 14:43 ` Ahmed S. Darwish
2015-01-21 15:00 ` Andri Yngvason [this message]
2015-01-21 15:36 ` Ahmed S. Darwish
2015-01-21 16:13 ` Wolfgang Grandegger
2015-01-23 6:07 ` Ahmed S. Darwish
2015-01-23 10:32 ` Andri Yngvason
2015-01-25 2:43 ` Ahmed S. Darwish
2015-01-26 10:21 ` Andri Yngvason
2015-01-21 16:37 ` Andri Yngvason
2015-01-21 16:20 ` Andri Yngvason
2015-01-21 22:59 ` Marc Kleine-Budde
2015-01-22 10:14 ` Andri Yngvason
2015-01-25 2:49 ` Ahmed S. Darwish
2015-01-25 3:21 ` Ahmed S. Darwish
2015-01-26 5:17 ` [PATCH v6 0/7] can: kvaser_usb: Leaf bugfixes and USBCan-II support Ahmed S. Darwish
2015-01-26 5:20 ` [PATCH v6 1/7] can: kvaser_usb: Do not sleep in atomic context Ahmed S. Darwish
2015-01-26 5:22 ` [PATCH v6 2/7] can: kvaser_usb: Send correct context to URB completion Ahmed S. Darwish
2015-01-26 5:24 ` [PATCH v6 3/7] can: kvaser_usb: Retry the first bulk transfer on -ETIMEDOUT Ahmed S. Darwish
2015-01-26 5:25 ` [PATCH v6 4/7] can: kvaser_usb: Fix state handling upon BUS_ERROR events Ahmed S. Darwish
2015-01-26 5:27 ` [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM Ahmed S. Darwish
2015-01-26 5:29 ` [PATCH v6 6/7] can: kvaser_usb: Consolidate and unify state change handling Ahmed S. Darwish
2015-01-26 5:33 ` [PATCH v6 7/7] can: kvaser_usb: Add support for the USBcan-II family Ahmed S. Darwish
2015-01-26 10:34 ` Andri Yngvason
2015-01-26 10:26 ` [PATCH v6 6/7] can: kvaser_usb: Consolidate and unify state change handling Andri Yngvason
2015-01-26 10:28 ` [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM Andri Yngvason
2015-01-26 10:50 ` Marc Kleine-Budde
2015-01-26 10:52 ` Andri Yngvason
2015-01-26 10:53 ` Marc Kleine-Budde
2015-01-26 9:55 ` [PATCH v6 0/7] can: kvaser_usb: Leaf bugfixes and USBCan-II support Marc Kleine-Budde
2015-01-26 10:07 ` Marc Kleine-Budde
2015-01-26 10:56 ` Marc Kleine-Budde
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150121150015.31351.6605@shannon \
--to=andri.yngvason@marel.com \
--cc=darwish.07@gmail.com \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.org \
--cc=olivier@sobrie.be \
--cc=socketcan@hartkopp.net \
--cc=wg@grandegger.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.