Re: [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for Bosch C_CAN controller

Hi Bhupesh,

On 02/01/2011 05:29 AM, Bhupesh SHARMA wrote:

Hello Wolfgang,

quoted

...

quoted

quoted

quoted

+		/* handle error on the bus */
+		lec_type = c_can_has_and_handle_berr(priv);
+		if (lec_type && (error_type != C_CAN_NO_ERROR))
+			work_done += c_can_err(dev, error_type, lec_type);

State changes are only reported if berr_reporting is enabled and a

bus

quoted

quoted

error occured. This needs to be fixed.

As I mentioned earlier in a response to a review comment, the Bus

Error

quoted

reporting for C_CAN seems different from sja1000 and flexcan

approaches.

quoted

Do you think it will be useful to drop CAN_CTRLMODE_BERR_REPORTING

from

quoted

priv->can.ctrlmode_supported as done by *pch* driver? Or do you have
a better idea..

You bus error reporting is OK. The problem is that it does not only
affect bus errors but also state changes. State change messages should
alway be send independent of priv->can.ctrlmode. It's just a matter of
moving code to the right location. E.g. the code snippet above inside
c_can_err() before you check for bus errors.

quoted

quoted

Feel free to send the output of "candump any,0:0,#FFFFFFFF" when
sending
messages without cable connected and with a bus error provocuted.

OK. I will try to cross-compile candump for my arm-v7 architecture
and will send the output.

I did some changes to the code to ensure that the state change and lec
handling are handled separately and properly.

Great.

Please find the candump any,0:0,#FFFFFFFF output below:

1. With No-Cable connected, I keep getting:
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME
  can0  20000004  [8] 00 28 00 00 00 00 00 00   ERRORFRAME

The SJA1000 reports:

---- Error-active -> error-warning -> error-passive
---- cable disconnected
-bash-3.2# ./candump -t d any,0:0,#FFFFFFFF

 (0.000000)  can0  20000004  [8] 00 08 00 00 00 00 60 00   ERROR-WARNING
 (0.002588)  can0  20000004  [8] 00 20 00 00 00 00 80 00   ERROR-PASSIVE

I wonder why you get data[1]=28 immediately. Also it's nice to have the
txerr and rxerr in data[6..7].

2. With Tx and Rx shorted to simulate bus-error, I get:
  can0  20000044  [8] 00 20 00 00 00 00 00 00   ERRORFRAME

Also here, you should see:

---- Error-active -> error-warning -> error-passive -> bus-off

-bash-3.2# ./candump -t d any,0:0,#FFFFFFFF
 (0.000000)  can0  20000004  [8] 00 08 00 00 00 00 88 00   ERROR-WARNING
 (0.001362)  can0  20000004  [8] 00 20 00 00 00 00 88 00   ERROR-PASSIVE
 (0.001560)  can0  20000044  [8] 00 00 00 00 00 00 7F 00   BUS-OFF

In case 2, when I enable debug messages I get the correct state change sequence:
entered error warning state
entered error passive state
entered bus off state

As the log confirms.

Does this result seem fine to you?

Not yet. Also the output with "berr-reporting on" should be checked. And
the output when you recover from bus-off manually (using "ip link set
type can restart")

Thanks,

Wolfgang.