linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Max S." <max@schneidersoft.net>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: linux-can <linux-can@vger.kernel.org>
Subject: Re: GS_USB
Date: Mon, 11 Nov 2013 02:10:35 +0000	[thread overview]
Message-ID: <1384135835.3749.31.camel@blackbox> (raw)
In-Reply-To: <527EC310.5010003@grandegger.com>

Hi Wolfgang,

On Sun, 2013-11-10 at 00:19 +0100, Wolfgang Grandegger wrote:

> You redefine the CAN flags and mask here but in many places you
> silentely assume that they are identical, e.g. via:
> 
>    cf->can_id = hf->can_id;
> 
> Not sure how to handle this transprently. I think these defines will
> never change. Either use the CAN flags and mask directly (like you
> already do for the error frames) or properly remap/copy all flags and
> values. Other opinions?

I have removed the GS_CAN_* flags. They are the same as those in the
Linux can headers. The idea was not to have to repackage everything. But
instead get by with some copies. like cf->can_id = hf->can_id; Byte
order is handled by the device.

> In case of bus-off. What does the hardware do? How does it recover? Is
> recovery triggered by software possible? Is it possible to support
> manual recovery?

Manual recovery is not supported. The node can leave bus off state if
it detects 128 occurrences of 11 consecutive recessive bits on the bus.
Maybe I can trick the controller out of bus off by doing a software
reset. That's probably not going to be a clean solution though.

> 
> > +	} else if (cf->can_id & CAN_ERR_CRTL) {
> > +		dev->can.state = CAN_STATE_ERROR_ACTIVE;
> 
> 
> This means back to error active, right?
Yes, if none of the following conditions apply.
I guess i could use an if else if else instead of conditionally
overwriting the state...
> 
> > +		if ((cf->data[1] & CAN_ERR_CRTL_TX_WARNING) ||
> > +			(cf->data[1] & CAN_ERR_CRTL_RX_WARNING))
> > +			dev->can.state = CAN_STATE_ERROR_WARNING;
> > +
> > +		if ((cf->data[1] & CAN_ERR_CRTL_TX_PASSIVE) ||
> > +			(cf->data[1]&CAN_ERR_CRTL_RX_PASSIVE))
> 
> s/&/ & /
> 
> > +			dev->can.state = CAN_STATE_ERROR_PASSIVE;
> > +	}
> 
> Also please update the stats accordingly:
These are simple counts for how often the conditions occurred? They
count when the respective state is entered? When a node recovers from
bus off does that count as entering error passive?
> 
> 	can_stats.bus_off++;
> 	can_stats.error_warning++
> 	can_stats.error_passive++
> 
> Apart from that, the state handling is rather basic but totaly
> sufficient. Would be nice if you could show the state handling
> and bus-off recovery by reporting the output of "$ candump -td -e
> any,0:0,#FFFFFFFF" while sending messages to the device ...
> 
> 1. without cable connected (entering error passive)
> 2. with short-circuited CAN high and low (entering bus-off)

$ sudo ip link set can0 up type can bitrate 250000
$ cansequence can0 -p
$ candump -td -e can0,0:0,#FFFFFFFF 1>can.log

# CAN node transmitting normally
 (000.000000)  can0  002   [1]  00
...
 (000.000006)  can0  002   [1]  68
# CAN node is disconnected from bus
 (000.040973)  can0  20000004   [8]  00 20 00 00 00 00 80 00
ERRORFRAME
	controller-problem{tx-error-passive}
	error-counter-tx-rx{{128}{0}}
 (007.437011)  can0  002   [1]  69
...
 (000.000991)  can0  002   [1]  B3
# CAN node is reconnected to bus
 (000.000008)  can0  20000004   [8]  00 00 00 00 00 00 00 00
ERRORFRAME
	controller-problem{}
 (000.000004)  can0  002   [1]  B4
...
 (000.000003)  can0  002   [1]  D3
# CAN node is disconnected from bus
 (000.062975)  can0  20000004   [8]  00 20 00 00 00 00 80 00
ERRORFRAME
	controller-problem{tx-error-passive}
	error-counter-tx-rx{{128}{0}}
# short added between CAN-High to CAN-Low
 (005.593023)  can0  20000040   [8]  00 00 00 00 00 00 00 0E
ERRORFRAME
	bus-off
	error-counter-tx-rx{{0}{14}}
 (000.260982)  can0  20000100   [8]  00 00 00 00 00 00 E8 00
ERRORFRAME
	restarted-after-bus-off
	error-counter-tx-rx{{232}{0}}
 (000.526024)  can0  20000040   [8]  00 00 00 00 00 00 00 7D
ERRORFRAME
	bus-off
	error-counter-tx-rx{{0}{125}}
 (001.834994)  can0  20000100   [8]  00 00 00 00 00 00 F0 00
ERRORFRAME
	restarted-after-bus-off
	error-counter-tx-rx{{240}{0}}
 (000.523992)  can0  20000040   [8]  00 00 00 00 00 00 00 80
ERRORFRAME
	bus-off
	error-counter-tx-rx{{0}{128}}
 (001.835013)  can0  20000100   [8]  00 00 00 00 00 00 F8 00
ERRORFRAME
	restarted-after-bus-off
	error-counter-tx-rx{{248}{0}}
 (000.524981)  can0  20000004   [8]  00 00 00 00 00 00 28 00
ERRORFRAME
	controller-problem{}
	error-counter-tx-rx{{40}{0}}
 (000.086006)  can0  20000040   [8]  00 00 00 00 00 00 00 7C
ERRORFRAME
	bus-off
	error-counter-tx-rx{{0}{124}}
# The short between CAN-High and CAN-Low is removed
 (000.525000)  can0  20000100   [8]  00 00 00 00 00 00 80 00
ERRORFRAME
	restarted-after-bus-off
	error-counter-tx-rx{{128}{0}}
 (002.702998)  can0  002   [1]  D4
...
 (000.000003)  can0  002   [1]  ED
# CAN node is reconnected to bus
 (000.000007)  can0  20000004   [8]  00 08 00 00 00 00 6E 00
ERRORFRAME
	controller-problem{tx-error-warning}
	error-counter-tx-rx{{110}{0}}
 (000.000003)  can0  002   [1]  EE
...
 (000.000990)  can0  002   [1]  60
 (000.000008)  can0  20000004   [8]  00 00 00 00 00 00 00 00
ERRORFRAME
	controller-problem{}
 (000.000004)  can0  002   [1]  61
 (000.000004)  can0  002   [1]  62
...

Thank you for your time,
Max Schneider.



  reply	other threads:[~2013-11-11  1:57 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-04 12:31 GS_USB Max S.
2013-10-04 13:23 ` GS_USB Marc Kleine-Budde
2013-10-05 20:36 ` GS_USB Wolfgang Grandegger
2013-10-07 14:22   ` GS_USB Max S.
2013-10-07 14:37     ` GS_USB Wolfgang Grandegger
2013-10-07 19:52       ` GS_USB Max S.
2013-10-07 20:30         ` GS_USB Wolfgang Grandegger
2013-11-03 17:12           ` GS_USB Max S.
2013-11-03 19:42             ` GS_USB Wolfgang Grandegger
2013-11-09 23:19             ` GS_USB Wolfgang Grandegger
2013-11-11  2:10               ` Max S. [this message]
2013-11-11  8:05                 ` GS_USB Wolfgang Grandegger
2013-11-11 15:41                   ` GS_USB Oliver Hartkopp
     [not found]                     ` <1384199350.3483.20.camel@blackbox>
2013-11-11 21:49                       ` GS_USB Oliver Hartkopp
2013-11-15 10:39                         ` GS_USB Max S.
2013-11-23 16:05                           ` GS_USB Max S.
2013-12-04 21:17                             ` GS_USB Max S.
2013-12-05 19:05                               ` GS_USB Oliver Hartkopp
2013-12-05 19:07                                 ` GS_USB Oliver Hartkopp
2013-12-09 17:53                                   ` GS_USB Max S.
2013-12-05 20:18                               ` GS_USB Wolfgang Grandegger
2013-12-05 20:42                                 ` GS_USB Wolfgang Grandegger
2013-12-07 10:06                             ` GS_USB Wolfgang Grandegger
2013-12-09 10:52                               ` GS_USB Marc Kleine-Budde
2013-12-09 13:15                                 ` GS_USB Wolfgang Grandegger

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=1384135835.3749.31.camel@blackbox \
    --to=max@schneidersoft.net \
    --cc=linux-can@vger.kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).