linux-kernel - Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

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]
Date:	Tue, 05 Aug 2014 11:52:30 +0200
From:	Marc Kleine-Budde <mkl@...gutronix.de>
To:	Stefan Agner <stefan@...er.ch>
CC:	shawn.guo@...escale.com, kernel@...gutronix.de,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-can@...r.kernel.org
Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

On 08/04/2014 06:01 PM, Stefan Agner wrote:
> Am 2014-08-04 16:27, schrieb Marc Kleine-Budde:
>> On 08/04/2014 03:43 PM, Stefan Agner wrote:
>> [...]
>>
>>>> Thanks for the test, so far looks promising :) With this setup the other
>>>> CAN node repeats the CAN frame until it's ACKed. Because there is no
>>>> node with a compatible bitrate, there is no ACking CAN node.
>>>>
>>>> Can you add a third CAN node to the network. The second and third node
>>>> should use the same bitrate, while your vf610 uses a different one. With
>>>> the new setup it should take more than one frame until the vf610 goes
>>>> into error warning and even more frames to error passive. This way we
>>>> can see it the error warning interrupt is connected or not. The error
>>>> counters should increase with each "wrong" bitrate frame it sees, you
>>>> can check with:
>>>>
>>>>     ip -details link show can0
>>>>
>>>> The output looks like this:
>>>>
>>>>> 4: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT qlen 10
>>>>>     link/can
>>>>>     can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
>>>>                               ^^^^^^^^^^^^^^^^^^^^^^
>>>>>     bitrate 1000000 sample-point 0.750
>>>>>     tq 125 prop-seg 2 phase-seg1 3 phase-seg2 2 sjw 1
>>>>>     sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
>>>>>     clock 8000000
>>>>
>>>> When one of the berr-counter crosses 96 (and stays below 128) a warning
>>>> interrupt should be generated.
>>>
>>> Ok, created this setup, could successfully communicate with all three
>>> nodes. I then set the Vybrid to half of the bitrate. When I send a frame
>>> from Vybrid, the berr-counter tx immediately ends up at 128 and the
>>> device is in ERROR-PASSIVE:
>>
>> This is expected.
>>
>>> root@...ibri-vf:~# ip -details link show can1
>>> 3: can1: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
>>> mode DEFAULT group default qlen 10
>>>     link/can  promiscuity 0
>>>     can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
>>>     bitrate 124990 sample-point 0.739
>>>     tq 347 prop-seg 8 phase-seg1 8 phase-seg2 6 sjw 1
>>>     flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>>>     clock 83368421
>>             ^^^^^^^^
>>
>> BTW: the can core has a really weird clock rate, have a look at the
>> datasheet if you manage to route a 24 MHz clock (or another multiple of
>> 8MHz) to the flexcan core. I had a quick glance at the datasheet, if I
>> understand it correctly the Fast OSC clock runs with 24 MHz.
>>
> 
> This pinmux is actually part of the IPs CTRL register (see CLKSRC).
> Currently this is set unconditionally to peripheral clock, which is on
> my device 83.3 MHz (500MHz/6). This would be a bit bigger change. Just
> thinking how to implement this.

Sigh! I've missed the fact, that this bit is _inside_ the CAN core
(again). On the mx53 and imx6 this bit was obsolete and the clock was
selected outside of the CAN core.

> Maybe we have to reference a third clock
> "osc" and a device tree property fsl,can-clock-osc which one can choose
> between "osc" and "per" what do you think?

Yes, but that's a different patch. I thought it would be as easy as on
the mx53, where you just had to reconfigure the clock tree.

>>> root@...ibri-vf:~# cansend can1 1F334455#1122334455667788
>>> interface = can1, family = 29, type = 3, proto = 1
>>> root@...ibri-vf:~# [  818.886664] flexcan_irq, esr=00062242
>>> [  818.890365] flexcan_irq, ctrl=1c3dac57
>>>
>>> root@...ibri-vf:~# ip -details link show can1
>>> 3: can1: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
>>> mode DEFAULT group default qlen 10
>>>     link/can  promiscuity 0
>>>     can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
>>>     bitrate 124990 sample-point 0.739
>>>     tq 347 prop-seg 8 phase-seg1 8 phase-seg2 6 sjw 1
>>>     flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>>>     clock 83368421
>>>
>>>
>>> When I send the frames from another device on the bus, I can see the rx
>>> count incrementing by one on each frame I send. As you expected, the
>>> device changes to ERROR-WARNING when crossing the 96 frame boundary:
>>
>> This is correct
>>
> 
> Just found out when using too high bitrate (500000 vs 125000), the error
> counter immediately goes up to 128 (or even beyond) and ends up in
> ERROR-PASSIVE:

Maybe there is a off-by-one error in the flexcan core, so that a rx
error of 127 doesn't trigger an error passive interrupt.

> # ip -details link show can1
> [  292.164820] flexcan_get_berr_counter, esr=00000000
> [  292.169715] flexcan_get_berr_counter, esr=00040190
> 3: can1: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> mode DEFAULT group default qlen 10
>     link/can  promiscuity 0
>     can state ERROR-PASSIVE (berr-counter tx 0 rx 135) restart-ms 0
>     bitrate 298811 sample-point 0.777
>     tq 371 prop-seg 3 phase-seg1 3 phase-seg2 2 sjw 1
>     flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>     clock 83368421

[...]

>>> However, once reaching 128, I don't get another interrupt and the device
>>> stays in ERROR-WARNING:
>>
>> The contents of the esr reg would be interesting, especially the
>> FLT_CONF part.

> root@...ridvf61-v11:~# [  222.221651] flexcan_irq, esr=0005050a
> [  222.225349] flexcan_irq, ctrl=1c3dac57

> ip -details link show can1
> [  223.791082] flexcan_get_berr_counter, esr=00000000
> [  223.796246] flexcan_get_berr_counter, esr=00040180
> 3: can1: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> mode DEFAULT group default qlen 
> 10
>     link/can  promiscuity 0
>     can state ERROR-WARNING (berr-counter tx 0 rx 96) restart-ms 0
>     bitrate 124990 sample-point 0.739
>     tq 347 prop-seg 8 phase-seg1 8 phase-seg2 6 sjw 1
>     flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>     clock 83368421

> root@...ridvf61-v11:~# ip -details link show can1
> [  243.571175] flexcan_get_berr_counter, esr=00000000
> [  243.576343] flexcan_get_berr_counter, esr=00040582
> 3: can1: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> mode DEFAULT group default qlen 
> 10
>     link/can  promiscuity 0
>     can state ERROR-WARNING (berr-counter tx 0 rx 104) restart-ms 0
>     bitrate 124990 sample-point 0.739
>     tq 347 prop-seg 8 phase-seg1 8 phase-seg2 6 sjw 1
>     flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>     clock 83368421
> 
> ...
> 
> root@...ridvf61-v11:~# ip -details link show can1
> [  299.831192] flexcan_get_berr_counter, esr=00000000
> [  299.836358] flexcan_get_berr_counter, esr=00040582
> 3: can1: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> mode DEFAULT group default qlen 10
>     link/can  promiscuity 0 
>     can state ERROR-WARNING (berr-counter tx 0 rx 126) restart-ms 0 
>     bitrate 124990 sample-point 0.739 
>     tq 347 prop-seg 8 phase-seg1 8 phase-seg2 6 sjw 1
>     flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>     clock 83368421
> root@...ridvf61-v11:~# ip -details link show can1
> [  302.411025] flexcan_get_berr_counter, esr=00000000
> [  302.416195] flexcan_get_berr_counter, esr=00040592
> 3: can1: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> mode DEFAULT group default qlen 10
>     link/can  promiscuity 0 
>     can state ERROR-WARNING (berr-counter tx 0 rx 128) restart-ms 0 
>     bitrate 124990 sample-point 0.739 
>     tq 347 prop-seg 8 phase-seg1 8 phase-seg2 6 sjw 1
>     flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>     clock 83368421
> 
> The FLTCONF is in Error Passive, however the stack did not received that
> information. I still have the printk in the interrupt, however I did not
> get an interrupt here (while I get one when it switched to
> ERROR-WARNING... But it looks like the ERRINT is still pending...

According to the datasheet the ERRINT indicates a bit error (bit 10...15
is set), not the error passive state.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


Download attachment "signature.asc" of type "application/pgp-signature" (243 bytes)