netdev - RE: [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for Bosch C

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <D5ECB3C7A6F99444980976A8C6D896384DEE0832F8@EAPEX1MAIL1.st.com>
Date:	Tue, 1 Feb 2011 12:29:35 +0800
From:	Bhupesh SHARMA <bhupesh.sharma@...com>
To:	Wolfgang Grandegger <wg@...ndegger.com>
Cc:	"Socketcan-core@...ts.berlios.de" <Socketcan-core@...ts.berlios.de>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Marc Kleine-Budde <mkl@...gutronix.de>,
	David Miller <davem@...emloft.net>
Subject: RE: [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for Bosch
 C_CAN controller

Hello Wolfgang,

> ...
> >>> +		/* 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
> >> error occured. This needs to be fixed.
> >
> > As I mentioned earlier in a response to a review comment, the Bus
> Error
> > reporting for C_CAN seems different from sja1000 and flexcan
> approaches.
> > Do you think it will be useful to drop CAN_CTRLMODE_BERR_REPORTING
> from
> > 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.
> 
> >> 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.
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

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

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

Does this result seem fine to you?

Regards,
Bhupesh
--
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