| 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
| ||
|
Date: Wed, 15 Mar 2017 10:42:03 +0100
From: Wolfgang Grandegger <wg@...ndegger.com>
To: Akshay Bhat <nodeax@...il.com>
Cc: Akshay Bhat <akshay.bhat@...esys.com>, mkl@...gutronix.de,
linux-can@...r.kernel.org, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver
Hello Akshay,
Am 15.03.2017 um 05:44 schrieb Akshay Bhat:
> Hi Wolfgang,
>
> On Tue, Mar 14, 2017 at 2:08 PM, Wolfgang Grandegger <wg@...ndegger.com> 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.
I have little hope! Some revision of the flexcan controller have the
same problem
>
> So here is my plan:
> - Have the bus error interrupt always enabled
> - If berr-reporting off, then have the isr checks/reports state changes
Error state change messages should always be there. These are the
important one.
> - 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?)
If berr-reporting is off, simply do not create an error message for bus
errors, and only if the state changed. If it's "on" create an additional
bus error message.
http://lxr.free-electrons.com/source/drivers/net/can/flexcan.c#L334
>>> write: No buffer space available
>>> root@...6qrom5420b1:~# 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.
Seems OK according to:
http://electronics.stackexchange.com/questions/220596/can-error-counters-behaviour
>>
>>>
>>> root@...6qrom5420b1:~# 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.
Well, the SJA1000 is the reference concerning error reporting. It's very
detailed. Most of the error cases from
http://lxr.free-electrons.com/source/include/uapi/linux/can/error.h
are SJA1000 related. Most other CAN controllers report much less. And
the Flexcan driver handles a lot of different chip revisions and uses
mail boxes. It's not a good base for the Hi-311x.
Wolfgang.
Wolfgang.
Powered by blists - more mailing lists