[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 08 Aug 2012 17:02:04 +0200
From: Wolfgang Grandegger <wg@...ndegger.com>
To: Olivier Sobrie <olivier@...rie.be>
CC: Marc Kleine-Budde <mkl@...gutronix.de>, linux-can@...r.kernel.org,
linux-usb@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v2] can: kvaser_usb: Add support for Kvaser CAN/USB devices
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@...rie.be>
>>>>> ---
>>>>> 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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists