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:

quoted

Hi Oliver,

On 08/08/2012 08:14 AM, Olivier Sobrie wrote:

quoted

Hi Wolfgang,

On Tue, Aug 07, 2012 at 08:26:38AM +0200, Wolfgang Grandegger wrote:

quoted

On 08/06/2012 07:21 AM, Olivier Sobrie wrote:

quoted

This driver provides support for several Kvaser CAN/USB devices.
Such kind of devices supports up to three can network interfaces.

s/can/CAN/

quoted

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.

quoted

quoted

quoted

quoted

Signed-off-by: Olivier Sobrie <redacted>
---
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.

quoted

quoted

quoted

quoted

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.
....

quoted

quoted

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