Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver
From: Akshay Bhat
Date: Wed Mar 15 2017 - 00:45:18 EST
Hi Wolfgang,
On Tue, Mar 14, 2017 at 2:08 PM, Wolfgang Grandegger <wg@xxxxxxxxxxxxxx> wrote:
...snip....
>> /////disconnect cable
>> can0 20000088 [8] 00 00 00 19 00 00 28 00 ERRORFRAME
>> protocol-violation{{}{acknowledge-slot}}
>> bus-error
>> error-counter-tx-rx{{40}{0}}
>> can0 20000088 [8] 00 00 00 19 00 00 58 00 ERRORFRAME
>> protocol-violation{{}{acknowledge-slot}}
>> bus-error
>> error-counter-tx-rx{{88}{0}}
>> can0 20000088 [8] 00 00 00 19 00 00 80 00 ERRORFRAME
>> protocol-violation{{}{acknowledge-slot}}
>> bus-error
>> error-counter-tx-rx{{128}{0}}
>
>
> TX error warning is missing.
>
This support was missing in the driver, added in V4 patch.
>> can0 2000008C [8] 00 20 00 19 00 00 80 00 ERRORFRAME
>> controller-problem{tx-error-passive}
>> protocol-violation{{}{acknowledge-slot}}
>> bus-error
>> error-counter-tx-rx{{128}{0}}
>
>
> Here "tx-error-passiv" is packed with a bus error. What I'm looking for are
> state change messages similar to:
>
> can0 20000204 [8] 00 08 00 00 00 00 60 00 ERRORFRAME
> controller-problem{tx-error-warning}
> state-change{tx-error-warning}
> error-counter-tx-rx{{96}{0}}
> can0 20000204 [8] 00 30 00 00 00 00 80 00 ERRORFRAME
> controller-problem{tx-error-passive}
> state-change{tx-error-passive}
> error-counter-tx-rx{{128}{0}
>
> They should always come, even with "berr-reporting off".
>
HI-3110 has only 1 bus error interrupt. There is no dedicated state
change interrupts like other controllers.
So here is my plan:
- Have the bus error interrupt always enabled
- If berr-reporting off, then have the isr checks/reports state changes
- if berr-reporting on, then have the isr checks/reports bus errors
and state changes (Does it make sense packing the error message, if
the ISR finds both bus and state changes?)
>> write: No buffer space available
>> root@imx6qrom5420b1:~# ip -s -d link show can0
>> 4: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
>> mode DEFAULT group default qlen 10
>> link/can promiscuity 0
>> can <BERR-REPORTING> state ERROR-PASSIVE (berr-counter tx 128 rx 0)
>> restart-ms 0
>> bitrate 1000000 sample-point 0.750
>> tq 62 prop-seg 5 phase-seg1 6 phase-seg2 4 sjw 1
>> hi3110: tseg1 2..16 tseg2 2..8 sjw 1..4 brp 1..64 brp-inc 1
>> clock 16000000
>> re-started bus-errors arbit-lost error-warn error-pass bus-off
>> 0 6 0 1 1 0
>
>
> The error warning and passive counter increased , though. Also the bus error
> should come in at a rather hight rate. Looking to the code, maybe
> you need to test STATF to check for state changes (and not ERR).
>
Apologize, just realized In the above case some error packets were
lost, because I forgot to set the CPU frequency to max. Will resend
the log.
..snip...
>
> After some more messages there should be also:
>
> can0 20000200 [8] 00 40 00 00 00 00 5F 00 ERRORFRAME
> state-change{back-to-error-active}
> error-counter-tx-rx{{95}{0}}
>
> For each message sent, the error counter decreases by 8.
>
The HI-3110 controller decrements the error counter by 1 for every message sent.
The error count increments by 8 when there is an error.
>
>>
>> root@imx6qrom5420b1:~# ip -s -d link show can0
>> 4: can0: <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 117 rx 0) restart-ms 0
>> bitrate 1000000 sample-point 0.750
>> tq 62 prop-seg 5 phase-seg1 6 phase-seg2 4 sjw 1
>> hi3110: tseg1 2..16 tseg2 2..8 sjw 1..4 brp 1..64 brp-inc 1
>> clock 16000000
>> re-started bus-errors arbit-lost error-warn error-pass bus-off
>> 0 1 0 0 0 0
>
>
> Strange, some counters got lost.
>
This was a bug introduced when adding berr-reporting, have fixed in v4 patch.
>>
>> I have not been able to check the bus-off condition by (short-circuiting
>> CAN low and high). The tec error count remains at 128 when I short the
>> CAN low and high pins and the status never goes BUSOFF.
>
>
> You also need to send a message and the short-circuit should be at the
> connector of the sending host. What tranceiver is used? Do you know?
>
ADM3054 transceiver is used with HI-3111. I connected the
HI-3111/ADM3054 board to kvaser leaf and ran "cangen -i can0" and
"candump -e any,0:0,#FFFFFFF" on the board. Removed the cable and
shorted the CAN_H/L pins coming out of ADM3054. I will try your
suggestion of using a different bit-rate on the Kvaser leaf instead.
I appreciate your continued feedback, it has helped significantly
improve the error handling of the driver. Looking back I should have
based it on sja1000 or flexcan driver.
Thanks,
Akshay