From: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public•gmane.org>
To: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public•gmane.org>
Cc: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public•gmane.org>,
linux-can-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
Subject: Re: [PATCH v2] can: kvaser_usb: Add support for Kvaser CAN/USB devices
Date: Wed, 08 Aug 2012 17:02:04 +0200 [thread overview]
Message-ID: <50227F6C.8010501@grandegger.com> (raw)
In-Reply-To: <20120808133005.GA14300@hposo>
On 08/08/2012 03:30 PM, Olivier Sobrie wrote:
> On Wed, Aug 08, 2012 at 10:25:35AM +0200, Wolfgang Grandegger wrote:
>> Hi Oliver,
>>
>> On 08/08/2012 08:14 AM, Olivier Sobrie wrote:
>>> Hi Wolfgang,
>>>
>>> On Tue, Aug 07, 2012 at 08:26:38AM +0200, Wolfgang Grandegger wrote:
>>>> On 08/06/2012 07:21 AM, Olivier Sobrie wrote:
>>>>> This driver provides support for several Kvaser CAN/USB devices.
>>>>> Such kind of devices supports up to three can network interfaces.
>>>>
>>>> s/can/CAN/
>>>>
>>>>> It has been tested with a Kvaser USB Leaf Light (one network interface)
>>>>> connected to a pch_can interface.
>>>>> The firmware version of the Kvaser device was 2.5.205.
>>>>>
>>>>> List of Kvaser devices supported by the driver:
>>>>> - Kvaser Leaf prototype (P010v2 and v3)
>>>>> - Kvaser Leaf Light (P010v3)
>>>>> - Kvaser Leaf Professional HS
>>>>> - Kvaser Leaf SemiPro HS
>>>>> - Kvaser Leaf Professional LS
>>>>> - Kvaser Leaf Professional SWC
>>>>> - Kvaser Leaf Professional LIN
>>>>> - Kvaser Leaf SemiPro LS
>>>>> - Kvaser Leaf SemiPro SWC
>>>>> - Kvaser Memorator II, Prototype
>>>>> - Kvaser Memorator II HS/HS
>>>>> - Kvaser USBcan Professional HS/HS
>>>>> - Kvaser Leaf Light GI
>>>>> - Kvaser Leaf Professional HS (OBD-II connector)
>>>>> - Kvaser Memorator Professional HS/LS
>>>>> - Kvaser Leaf Light "China"
>>>>> - Kvaser BlackBird SemiPro
>>>>> - Kvaser OEM Mercury
>>>>> - Kvaser OEM Leaf
>>>>> - Kvaser USBcan R
>>>>
>>>> Impressive list! What CAN controllers are used inside the devices? SJA1000?
>>>
>>> I took this list from the Kvaser driver. However I only have a Kvaser
>>> Leaf Light device thus I'm not sure it will work with other ones.
>>> If you prefer I can only let Kvaser Leaf Light instead of the full list.
>>> In my device it looks to be a Renesas M16C controller.
>>
>> OK. Checking the manual, if available, could help to understand how the
>> firmware handles bus errors and state changes.
>
> Ok I'll try to find it.
>
>>
>>>>> Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public•gmane.org>
>>>>> ---
>>>>> Changes since v1:
>>>>> - added copyrights
>>>>> - kvaser_usb.h merged into kvaser.c
>>>>> - added kvaser_usb_get_endpoints to find eindpoints instead of
>>>>> hardcoding their address
>>>>> - some cleanup and comestic changes
>>>>> - fixed issues with errors handling
>>>>> - fixed restart-ms == 0 case
>>>>> - removed do_get_berr_counter method since the hardware doens't return
>>>>> good values for txerr and rxerr.
>>>>>
>>>>> If someone in the linux-usb mailing can review it, it would be nice.
>>>>>
>>>>> Concerning the errors, it behaves like that now:
>>>>>
>>>>> 1) Short-circuit CAN-H and CAN-L and restart-ms = 0
>>>>>
>>>>> t0: short-circuit + 'cansend can1 123#112233'
>>>>>
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-off
>>>>> bus-error
>>>>>
>>>>> t1: remove short-circuit + 'ip link set can1 type can restart'
>>>>>
>>>>> can1 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>>>> restarted-after-bus-off
>>>>> can1 20000004 [8] 00 0C 00 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>
>>>> Why do we get the last error message? Maybe the firmware does it that
>>>> way (going down passive->warning->active).
>>>
>>> It goes in that order: warning -> passive -> bus off -> warning
>>> -> passive -> ...
>>
>> Just for curiosity? You don't see back to "error active"?
>
> No but that's maybe because of my misunderstanding of the
> M16C_STATE_BUS_ERROR flag.
> What I see is:
> t1: M16C_STATE_BUS_ERROR
> t2: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_PASSIVE
> t3: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_OFF
> and then again
> t4: M16C_STATE_BUS_ERROR
> t2: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_PASSIVE
> t3: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_OFF
>
> Thus as you suggested below, the flag M16C_STATE_BUS_ERROR might not mean
> CAN_STATE_ERROR_WARNING...
Do you see bus error bits set? If not, I could mean "error active",
otherwise "error warning". Meaning the device sends such messages
containing bus error information plus the current state.
>>>>> 2) Short-circuit CAN-H and CAN-L and restart-ms = 100
>>>>>
>>>>> t0: short-circuit + cansend can1 123#112233
>>>>>
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-off
>>>>> bus-error
>>>>> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> restarted-after-bus-off
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-off
>>>>> bus-error
>>>>> ...
>>>>>
>>>>> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> restarted-after-bus-off
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>>
>>>>> t1: remove short-circuit
>>>>>
>>>>> can1 123 [3] 11 22 33
>>>>> can1 20000008 [8] 00 00 40 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{back-to-error-active}{}}
>>>>
>>>> The order is still inverted but likely the firmware is doing it that way.
>>>
>>> Indeed the firmware does it that way: it sends the acknwledge of the
>>> frame beofre the state change. I can avoid that behavior by checking the
>>> state in the acknowledge frame and send the restart frame if the bus was
>>> off.
>>
>> Well, if the firmware does it wrong, I would not really care. Also,
>> could you use timestamping to see if they come close together.
>
> candump can1,0:0,#FFFFFFFF -td -e:
> (001.369850) can1 123 [3] 11 22 33
> (004.716034) can1 20000008 [8] 00 00 40 00 00 00 00 00 ERRORFRAME
> protocol-violation{{back-to-error-active}{}}
>
> Not so close... I'll add something in the tx acknowledge.
More than three seconds, wired. I would add netdev_dbg() to the state
change and error handling code to better understand what's going on.
....
>>> What error should I report in case of M16C_STATE_BUS_ERROR?
>>
>> To make that clear, I have added an (old) output from the SJA1000, which
>> is the defacto reference. Bus error reporting is enabled and no cable is
>> connected. Watch the TX error count increasing and how the state changes:
>
> With my hardware I do not get txerr and rxerr... I only get
> M16C_STATE_BUS_ERROR. Thus in fact I can only pass from ACTIVE to PASSIVE
> without reaching the WARNING state.
No problem, I just wanted to illustrate how the hardware does usually do
error and state change reporting.
Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-08-08 15:02 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-30 5:32 [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices Olivier Sobrie
2012-07-30 11:11 ` Marc Kleine-Budde
2012-07-30 13:33 ` Olivier Sobrie
2012-07-31 9:56 ` Marc Kleine-Budde
2012-07-31 13:06 ` Olivier Sobrie
2012-07-31 13:27 ` Marc Kleine-Budde
2012-08-02 10:53 ` Olivier Sobrie
2012-08-02 11:56 ` Marc Kleine-Budde
2012-08-02 12:16 ` Olivier Sobrie
2012-08-02 12:33 ` Marc Kleine-Budde
2012-08-02 13:20 ` Olivier Sobrie
2012-08-05 20:43 ` Wolfgang Grandegger
2012-08-06 5:27 ` Olivier Sobrie
2012-07-31 13:43 ` Wolfgang Grandegger
2012-08-05 20:41 ` Wolfgang Grandegger
2012-08-06 5:21 ` [PATCH v2] " Olivier Sobrie
2012-08-06 8:10 ` Oliver Neukum
2012-08-08 5:28 ` Olivier Sobrie
2012-08-07 6:26 ` Wolfgang Grandegger
2012-08-08 6:14 ` Olivier Sobrie
2012-08-08 8:25 ` Wolfgang Grandegger
[not found] ` <5022227F.60800-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2012-08-08 13:30 ` Olivier Sobrie
2012-08-08 15:02 ` Wolfgang Grandegger [this message]
2012-08-13 13:51 ` [PATCH v3] " Olivier Sobrie
2012-09-20 5:06 ` [PATCH v4] " Olivier Sobrie
[not found] ` <1348117587-2905-1-git-send-email-olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
2012-09-21 9:54 ` Marc Kleine-Budde
2012-09-22 16:02 ` Wolfgang Grandegger
[not found] ` <505DE106.6060405-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2012-10-02 7:14 ` Olivier Sobrie
2012-10-02 7:16 ` [PATCH v5] " Olivier Sobrie
2012-11-07 20:29 ` Marc Kleine-Budde
2012-11-20 8:46 ` Olivier Sobrie
2012-11-20 10:59 ` Marc Kleine-Budde
[not found] ` <1343626352-24760-1-git-send-email-olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
2012-11-21 7:11 ` [PATCH v6] " Olivier Sobrie
2012-11-22 11:01 ` Marc Kleine-Budde
2012-11-22 15:01 ` Olivier Sobrie
2012-11-22 21:30 ` Greg KH
2012-11-23 8:48 ` Marc Kleine-Budde
2012-11-23 13:30 ` [PATCH] kvaser_usb: fix "dma on the stack" errors Olivier Sobrie
2012-11-23 13:40 ` Olivier Sobrie
2012-11-23 13:48 ` Marc Kleine-Budde
2012-11-23 13:54 ` [PATCH v2] " Olivier Sobrie
2012-11-23 14:08 ` Marc Kleine-Budde
2012-11-23 14:16 ` Olivier Sobrie
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=50227F6C.8010501@grandegger.com \
--to=wg-5yr1bzd7o62+xt7jha+gda@public$(echo .)gmane.org \
--cc=linux-can-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public$(echo .)gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public$(echo .)gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox