Re: [PATCH] can: ti_hecc: WIP: fix out-of-order problem - CANMIN Version

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]

On 11/07/2012 01:10 PM, AnilKumar, Chimata wrote:
> On Tue, Nov 06, 2012 at 01:23:52, Marc Kleine-Budde wrote:
>> This is Work-In-Process patch which fixes several problem:
>> - ti_hecc_xmit: modify CANMIM under spin_lock
>> - ti_hecc_rx_pkt: don't re-enable current mailbox,
>>   next CAN frame might do into same mailbox
>> - ti_hecc_interrupt: modify CANMIM under spin_lock
>> - ti_hecc_rx_poll: rework polling loop, wrap-around-handling and
>>   reactivation of mailboxes.
>>
>> Before acknowledging the received CAN frames in CANRMP wait for CAN core to
>> finish current rx, otherwise next CAN frame goes into undefined mailbox. Idea
>> lifted shamelessly from AnilKumar Ch's patch.
>>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>> Hello AnilKumar,
>>
>> here's the CANMIN version. With this patch the rx-path doesn't change the CANME
>> register, the wait-for-end-of-reception is done before writing to the CANRMP
>> register. So there isn't any spin_lock after the busy wait loop.
>>
>> please test,
> 
> +Anant
> 
> Hi Marc,
> 
> Note: I am not using the standard utilities, I have the utilities
> from http://git.pengutronix.de/?p=tools/canutils.git

I use them as well.

> 1. I am seeing the issue with this patch as well.
> 
> @500KBPS
> <0x002> [1] 13
> <0x002> [1] 16
> <0x002> [1] 18
> <0x002> [1] 1d
> <0x002> [1] 20
> <0x002> [1] 23
> <0x002> [1] 26
> <0x002> [1] 29
> 
> @1MBPS
> <0x002> [1] 22
> <0x002> [1] 37
> <0x002> [1] 44
> <0x002> [1] 54
> <0x002> [1] 6a
> <0x002> [1] 80
> <0x002> [1] 94
> <0x002> [1] a6
> 
> Steps I have followed:-
>     One side AM335x sending packets through D_CAN ip
>       $ cansequence can0 -p
>     AM3517 receiving the packets through HECC ip
>       $ candump can0
> 
> 2. But if I use this command in the reception side @500KBPS I am not
> seeing any out of seq message "received wrong sequence count....".
> This might be due to load on the system is less because no
> UART/console prints
> 
> $ cansequence -r -v | grep wrong 

I'm using $(cansequence -rvv | grep wrong) and it works for me even
under heavy ping -f load  at 500Kbit/s. I think without the grep the
receiving process spends too much time printing and the receive buffer
in the socket will overflow.

> 3. @1MBPS I have seen the wrong sequence message.

Can you give the the exact error messages. There is a difference between
"lost" messages and out-of-order messages. Lost messages simply print a
single wrong sequence number detected message. An out-of-order reception
prints a characteristic 3 line error message.

BTW: We should check if a lost message on the CAN core level leads to a
receive buffer overflow CAN error frame.

My hardware is not working properly at 1Mbit/s. Can you check if the
patch I sent first makes a difference?

> With the standard utilities can we test the can-sequence? I have not
> used those.

As Wolfgang pointed out, you might use canfdtest [1]. But I haven't used
it, yet. Maybe I should as cansequence doesn't trigger the problem at
500 kbit/s any more.

Marc

[1] https://gitorious.org/linux-can/can-utils/blobs/master/canfdtest.c

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]