Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1600276 > unrolled thread

Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver

Started byWolfgang Grandegger <wg@grandegger.com>
First post2017-03-14 13:20 +0100
Last post2017-03-18 13:40 +0100
Articles 16 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-14 13:20 +0100
    Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Akshay Bhat <akshay.bhat@timesys.com> - 2017-03-14 17:30 +0100
      Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-14 19:10 +0100
        Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-14 22:30 +0100
        Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Akshay Bhat <nodeax@gmail.com> - 2017-03-15 05:50 +0100
          Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-15 08:30 +0100
          Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-15 10:50 +0100
            Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Akshay Bhat <akshay.bhat@timesys.com> - 2017-03-16 18:10 +0100
              Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-16 21:10 +0100
                Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Akshay Bhat <akshay.bhat@timesys.com> - 2017-03-16 23:40 +0100
                  Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-17 08:40 +0100
                    Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-17 09:20 +0100
                    Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Akshay Bhat <akshay.bhat@timesys.com> - 2017-03-17 17:10 +0100
                      Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-17 18:40 +0100
                        Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Akshay Bhat <akshay.bhat@timesys.com> - 2017-03-17 19:40 +0100
                          Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver Wolfgang Grandegger <wg@grandegger.com> - 2017-03-18 13:40 +0100

#1600276 — Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-14 13:20 +0100
SubjectRe: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver
Message-ID<tkTAK-8qq-1@gated-at.bofh.it>
Hallo Akshay,

Am 13.03.2017 um 16:38 schrieb Akshay Bhat:
> Hi Wolfgang,
>
> On 03/09/2017 12:36 PM, Wolfgang Grandegger wrote:
>> Hello,
>>
>> doing a quick review... I realized a few issues...
>>
>> Am 17.01.2017 um 20:22 schrieb Akshay Bhat:
... snip ...
>> A few other things to check:
>>
>> Run "cangen" and monitor the message with "candump -e any,0:0,#FFFFFFF".
>> Then 1) disconnect the cable or 2) short-circuit CAN low and high at the
>> connector. You should see error messages. After reconnection or removing
>> the short-circuit (and bus-off recovery) the state should go back to
>> "active".
>>
>
> With the above sequence, candump reports "ERRORFRAME" with
> protocol-violation{{}{acknowledge-slot}}, bus-error. On re-connecting
> the cable the can state goes back to ACTIVE and I see the messages that
> were in the queue being sent.

Do you get the ACK error also with berr-reporting off? Would be nice if 
you could show a candump log here.

Also, any error message should show the bus error counts in data[7,8]:

http://lxr.free-electrons.com/source/drivers/net/can/sja1000/sja1000.c#L408

And please check bus-off as well (short-circuiting CAN low and high).

Wolfgang.

[toc] | [next] | [standalone]


#1600663

FromAkshay Bhat <akshay.bhat@timesys.com>
Date2017-03-14 17:30 +0100
Message-ID<tkXuG-2Iv-3@gated-at.bofh.it>
In reply to#1600276
Hi Wolfgang,

On 03/14/2017 08:11 AM, Wolfgang Grandegger wrote:
> ... snip ...
>>> A few other things to check:
>>>
>>> Run "cangen" and monitor the message with "candump -e any,0:0,#FFFFFFF".
>>> Then 1) disconnect the cable or 2) short-circuit CAN low and high at the
>>> connector. You should see error messages. After reconnection or removing
>>> the short-circuit (and bus-off recovery) the state should go back to
>>> "active".
>>>
>>
>> With the above sequence, candump reports "ERRORFRAME" with
>> protocol-violation{{}{acknowledge-slot}}, bus-error. On re-connecting
>> the cable the can state goes back to ACTIVE and I see the messages that
>> were in the queue being sent.
> 
> Do you get the ACK error also with berr-reporting off? Would be nice if
> you could show a candump log here.
> 

Below is a log for disconnecting and re-connecting CAN cable scenario:
(Note this is on a 4.1.18 kernel with RT patch)

root@imx6qrom5420b1:~# ip link set can0 up type can bitrate 1000000
berr-reporting on
root@imx6qrom5420b1:~# candump -e any,0:0,#FFFFFFF &
[1] 768
root@imx6qrom5420b1:~# cangen can0
  can0  21C   [8]  35 98 C0 7A 95 03 E6 2A
  can0  6E6   [1]  F2
  can0  5C7   [2]  42 50
  can0  57C   [8]  83 7A E4 0C 03 8B 90 45
  can0  55C   [8]  B9 74 87 52 D8 F4 64 04
  can0  014   [8]  28 CB 96 57 3B 80 67 4F
  can0  6AF   [1]  35
  can0  51E   [8]  B6 C8 6C 1D 3A 87 ED 2E
  can0  527   [8]  D0 8A D3 59 0E 34 40 78
  can0  30C   [2]  6A 12
  can0  145   [8]  CB 6E FF 55 C1 BE C3 22
  can0  5A5   [8]  C4 49 54 68 02 63 F9 35
  can0  0BA   [8]  DA 57 5E 3A CE 88 20 1C
  can0  516   [2]  09 09
  can0  743   [8]  7C 4D 25 47 61 4C 56 3D
  can0  31D   [2]  9C D3
  can0  71E   [8]  53 7C 97 2A 2A F2 9F 56
  can0  52E   [8]  FE DA 2D 51 73 96 DF 79
/////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}}
  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}}
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
    RX: bytes  packets  errors  dropped overrun mcast
    0          0        6       0       0       0
    TX: bytes  packets  errors  dropped carrier collsns
    106        18       0       0       0       0
root@imx6qrom5420b1:~#
/////re-connect cable
  can0  169   [8]  35 55 A3 1C 0F 47 2E 5B
  can0  318   [8]  11 AA 27 11 D2 1B CE 34
  can0  577   [8]  A0 A4 EE 50 8D A2 E1 3E
  can0  4ED   [8]  52 96 17 7E 31 FC 7D 7C
  can0  2E7   [8]  92 48 D4 39 05 1E 9F 50
  can0  200   [8]  4A 66 F6 02 1E 71 8E 26
  can0  29A   [8]  49 63 2E 7D C9 77 85 7A
  can0  15A   [7]  3C 0E 65 74 C3 62 80
  can0  011   [1]  D2
  can0  26B   [3]  FC D6 68
  can0  5CE   [8]  6F 02 B5 14 BC 7A D7 02

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-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          7          0          1          1          0
    RX: bytes  packets  errors  dropped overrun mcast
    0          0        7       0       0       0
    TX: bytes  packets  errors  dropped carrier collsns
    181        29       0       0       0       0


//Reboot the board and test with bus error reporting off

root@imx6qrom5420b1:~# ip link set can0 up type can bitrate 1000000
berr-reporting off
root@imx6qrom5420b1:~# candump -e any,0:0,#FFFFFFF &
[1] 782
root@imx6qrom5420b1:~# cangen can0
  can0  1FA   [3]  C9 FE C2
  can0  3E2   [5]  85 37 03 5B 6F
  can0  289   [8]  A4 F6 BF 4A 3F 70 65 1B
  can0  12D   [8]  B2 72 10 33 AB B4 68 64
  can0  054   [2]  01 D7
  can0  4A6   [8]  29 7D 76 56 CA C1 60 00
  can0  768   [8]  97 3D 92 08 61 C1 D9 03
  can0  098   [6]  A4 A8 5A 60 92 1A
  can0  3C9   [8]  71 78 0D 25 AB 27 8B 51
/////disconnect cable
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 state ERROR-ACTIVE (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          0          0          0          0          0
    RX: bytes  packets  errors  dropped overrun mcast
    0          0        0       0       0       0
    TX: bytes  packets  errors  dropped carrier collsns
    56         9        0       0       0       0
root@imx6qrom5420b1:~#
/////re-connect cable
can0  20000088   [8]  00 00 00 19 00 00 7F 00   ERRORFRAME
	protocol-violation{{}{acknowledge-slot}}
	bus-error
	error-counter-tx-rx{{127}{0}}
  can0  553   [6]  1A E4 60 6B DC 07
  can0  7E3   [8]  1C 78 95 6E 10 81 AA 40
  can0  20C   [8]  BB 35 13 25 60 0A 56 57
  can0  1D0   [8]  48 4A 39 64 76 E6 57 08
  can0  43A   [1]  40
  can0  2CF   [7]  03 45 5E 0F 67 33 4C
  can0  1CD   [8]  F9 4D AB 1D 96 A5 67 0E
  can0  515   [8]  41 CD F2 5F 68 92 43 16
  can0  661   [8]  45 9A 73 69 45 EE 8B 42
  can0  41B   [1]  55
  can0  52F   [1]  87

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
    RX: bytes  packets  errors  dropped overrun mcast
    0          0        1       0       0       0
    TX: bytes  packets  errors  dropped carrier collsns
    120        20       0       0       0       0


> Also, any error message should show the bus error counts in data[7,8]:
>
> http://lxr.free-electrons.com/source/drivers/net/can/sja1000/sja1000.c#L408
>

I can add this in v4 version of the patch (Above log has this patch
applied).

> And please check bus-off as well (short-circuiting CAN low and high).
> 

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.

If I short the CAN high pin to ground, then the TEC goes to 136 but
thats the highest I have been able to get the TEC to go.

The setup I have is:
HI-3111 can chip -> ADM3054 can transceiver

//CAN_H and CAN_L shorted
<snip>
  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}}

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          7          0          1          1          0
    RX: bytes  packets  errors  dropped overrun mcast
    0          0        7       0       0       0
    TX: bytes  packets  errors  dropped carrier collsns
    626        109      0       0       0       0


//CAN_H shorted to GND
<snip>
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}}

can0  2000008C   [8]  00 20 01 00 00 00 88 00   ERRORFRAME
	controller-problem{tx-error-passive}
	protocol-violation{{single-bit-error}{}}
	bus-error
	error-counter-tx-rx{{136}{0}}

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 136 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          9          0          1          1          0
    RX: bytes  packets  errors  dropped overrun mcast
    0          0        9       0       0       0
    TX: bytes  packets  errors  dropped carrier collsns
    626        109      0       0       0       0

Thanks,
Akshay

[toc] | [prev] | [next] | [standalone]


#1600783

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-14 19:10 +0100
Message-ID<tkZ3s-3WN-29@gated-at.bofh.it>
In reply to#1600663
Hello Akshay,

Am 14.03.2017 um 17:20 schrieb Akshay Bhat:
>
> Hi Wolfgang,
>
> On 03/14/2017 08:11 AM, Wolfgang Grandegger wrote:
>> ... snip ...
>>>> A few other things to check:
>>>>
>>>> Run "cangen" and monitor the message with "candump -e any,0:0,#FFFFFFF".
>>>> Then 1) disconnect the cable or 2) short-circuit CAN low and high at the
>>>> connector. You should see error messages. After reconnection or removing
>>>> the short-circuit (and bus-off recovery) the state should go back to
>>>> "active".
>>>>
>>>
>>> With the above sequence, candump reports "ERRORFRAME" with
>>> protocol-violation{{}{acknowledge-slot}}, bus-error. On re-connecting
>>> the cable the can state goes back to ACTIVE and I see the messages that
>>> were in the queue being sent.
>>
>> Do you get the ACK error also with berr-reporting off? Would be nice if
>> you could show a candump log here.
>>
>
> Below is a log for disconnecting and re-connecting CAN cable scenario:
> (Note this is on a 4.1.18 kernel with RT patch)
>
> root@imx6qrom5420b1:~# ip link set can0 up type can bitrate 1000000
> berr-reporting on
> root@imx6qrom5420b1:~# candump -e any,0:0,#FFFFFFF &

Please add "-td" ...

> [1] 768
> root@imx6qrom5420b1:~# cangen can0

and "-i" here.

>   can0  21C   [8]  35 98 C0 7A 95 03 E6 2A
>   can0  6E6   [1]  F2
>   can0  5C7   [2]  42 50
>   can0  57C   [8]  83 7A E4 0C 03 8B 90 45
>   can0  55C   [8]  B9 74 87 52 D8 F4 64 04
>   can0  014   [8]  28 CB 96 57 3B 80 67 4F
>   can0  6AF   [1]  35
>   can0  51E   [8]  B6 C8 6C 1D 3A 87 ED 2E
>   can0  527   [8]  D0 8A D3 59 0E 34 40 78
>   can0  30C   [2]  6A 12
>   can0  145   [8]  CB 6E FF 55 C1 BE C3 22
>   can0  5A5   [8]  C4 49 54 68 02 63 F9 35
>   can0  0BA   [8]  DA 57 5E 3A CE 88 20 1C
>   can0  516   [2]  09 09
>   can0  743   [8]  7C 4D 25 47 61 4C 56 3D
>   can0  31D   [2]  9C D3
>   can0  71E   [8]  53 7C 97 2A 2A F2 9F 56
>   can0  52E   [8]  FE DA 2D 51 73 96 DF 79
> /////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.

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

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

>     RX: bytes  packets  errors  dropped overrun mcast
>     0          0        6       0       0       0
>     TX: bytes  packets  errors  dropped carrier collsns
>     106        18       0       0       0       0
> root@imx6qrom5420b1:~#
> /////re-connect cable
>   can0  169   [8]  35 55 A3 1C 0F 47 2E 5B
>   can0  318   [8]  11 AA 27 11 D2 1B CE 34
>   can0  577   [8]  A0 A4 EE 50 8D A2 E1 3E
>   can0  4ED   [8]  52 96 17 7E 31 FC 7D 7C
>   can0  2E7   [8]  92 48 D4 39 05 1E 9F 50
>   can0  200   [8]  4A 66 F6 02 1E 71 8E 26
>   can0  29A   [8]  49 63 2E 7D C9 77 85 7A
>   can0  15A   [7]  3C 0E 65 74 C3 62 80
>   can0  011   [1]  D2
>   can0  26B   [3]  FC D6 68
>   can0  5CE   [8]  6F 02 B5 14 BC 7A D7 02
>
> 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-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          7          0          1          1          0
>     RX: bytes  packets  errors  dropped overrun mcast
>     0          0        7       0       0       0
>     TX: bytes  packets  errors  dropped carrier collsns
>     181        29       0       0       0       0
>
>
> //Reboot the board and test with bus error reporting off
>
> root@imx6qrom5420b1:~# ip link set can0 up type can bitrate 1000000
> berr-reporting off
> root@imx6qrom5420b1:~# candump -e any,0:0,#FFFFFFF &
> [1] 782
> root@imx6qrom5420b1:~# cangen can0
>   can0  1FA   [3]  C9 FE C2
>   can0  3E2   [5]  85 37 03 5B 6F
>   can0  289   [8]  A4 F6 BF 4A 3F 70 65 1B
>   can0  12D   [8]  B2 72 10 33 AB B4 68 64
>   can0  054   [2]  01 D7
>   can0  4A6   [8]  29 7D 76 56 CA C1 60 00
>   can0  768   [8]  97 3D 92 08 61 C1 D9 03
>   can0  098   [6]  A4 A8 5A 60 92 1A
>   can0  3C9   [8]  71 78 0D 25 AB 27 8B 51
> /////disconnect cable
> 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 state ERROR-ACTIVE (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          0          0          0          0          0
>     RX: bytes  packets  errors  dropped overrun mcast
>     0          0        0       0       0       0
>     TX: bytes  packets  errors  dropped carrier collsns
>     56         9        0       0       0       0
> root@imx6qrom5420b1:~#
> /////re-connect cable
> can0  20000088   [8]  00 00 00 19 00 00 7F 00   ERRORFRAME
> 	protocol-violation{{}{acknowledge-slot}}
> 	bus-error
> 	error-counter-tx-rx{{127}{0}}
>   can0  553   [6]  1A E4 60 6B DC 07
>   can0  7E3   [8]  1C 78 95 6E 10 81 AA 40
>   can0  20C   [8]  BB 35 13 25 60 0A 56 57
>   can0  1D0   [8]  48 4A 39 64 76 E6 57 08
>   can0  43A   [1]  40
>   can0  2CF   [7]  03 45 5E 0F 67 33 4C
>   can0  1CD   [8]  F9 4D AB 1D 96 A5 67 0E
>   can0  515   [8]  41 CD F2 5F 68 92 43 16
>   can0  661   [8]  45 9A 73 69 45 EE 8B 42
>   can0  41B   [1]  55
>   can0  52F   [1]  87

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.


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

>     RX: bytes  packets  errors  dropped overrun mcast
>     0          0        1       0       0       0
>     TX: bytes  packets  errors  dropped carrier collsns
>     120        20       0       0       0       0
>
>
>> Also, any error message should show the bus error counts in data[7,8]:
>>
>> http://lxr.free-electrons.com/source/drivers/net/can/sja1000/sja1000.c#L408
>>
>
> I can add this in v4 version of the patch (Above log has this patch
> applied).

Looks good.

>> And please check bus-off as well (short-circuiting CAN low and high).
>>
>
> 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?

Wolfgang.

[toc] | [prev] | [next] | [standalone]


#1600915

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-14 22:30 +0100
Message-ID<tl2aZ-66f-7@gated-at.bofh.it>
In reply to#1600783
Am 14.03.2017 um 19:08 schrieb Wolfgang Grandegger:
> Hello Akshay,
>
> Am 14.03.2017 um 17:20 schrieb Akshay Bhat:
>>
>> Hi Wolfgang,
>>
>> On 03/14/2017 08:11 AM, Wolfgang Grandegger wrote:
>>> ... snip ...
>>>>> A few other things to check:
>>>>>
>>>>> Run "cangen" and monitor the message with "candump -e
>>>>> any,0:0,#FFFFFFF".
>>>>> Then 1) disconnect the cable or 2) short-circuit CAN low and high
>>>>> at the
>>>>> connector. You should see error messages. After reconnection or
>>>>> removing
>>>>> the short-circuit (and bus-off recovery) the state should go back to
>>>>> "active".
>>>>>
>>>>
>>>> With the above sequence, candump reports "ERRORFRAME" with
>>>> protocol-violation{{}{acknowledge-slot}}, bus-error. On re-connecting
>>>> the cable the can state goes back to ACTIVE and I see the messages that
>>>> were in the queue being sent.
>>>
>>> Do you get the ACK error also with berr-reporting off? Would be nice if
>>> you could show a candump log here.
>>>
>>
>> Below is a log for disconnecting and re-connecting CAN cable scenario:
>> (Note this is on a 4.1.18 kernel with RT patch)
>>
>> root@imx6qrom5420b1:~# ip link set can0 up type can bitrate 1000000
>> berr-reporting on
>> root@imx6qrom5420b1:~# candump -e any,0:0,#FFFFFFF &
>
> Please add "-td" ...
>
>> [1] 768
>> root@imx6qrom5420b1:~# cangen can0
>
> and "-i" here.
>
>>   can0  21C   [8]  35 98 C0 7A 95 03 E6 2A
>>   can0  6E6   [1]  F2
>>   can0  5C7   [2]  42 50
>>   can0  57C   [8]  83 7A E4 0C 03 8B 90 45
>>   can0  55C   [8]  B9 74 87 52 D8 F4 64 04
>>   can0  014   [8]  28 CB 96 57 3B 80 67 4F
>>   can0  6AF   [1]  35
>>   can0  51E   [8]  B6 C8 6C 1D 3A 87 ED 2E
>>   can0  527   [8]  D0 8A D3 59 0E 34 40 78
>>   can0  30C   [2]  6A 12
>>   can0  145   [8]  CB 6E FF 55 C1 BE C3 22
>>   can0  5A5   [8]  C4 49 54 68 02 63 F9 35
>>   can0  0BA   [8]  DA 57 5E 3A CE 88 20 1C
>>   can0  516   [2]  09 09
>>   can0  743   [8]  7C 4D 25 47 61 4C 56 3D
>>   can0  31D   [2]  9C D3
>>   can0  71E   [8]  53 7C 97 2A 2A F2 9F 56
>>   can0  52E   [8]  FE DA 2D 51 73 96 DF 79
>> /////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.
>
>>   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".
>
>> 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).

Likely the ERR bits are only valid if the BUSERR bit in INTF is set.

>>     RX: bytes  packets  errors  dropped overrun mcast
>>     0          0        6       0       0       0
>>     TX: bytes  packets  errors  dropped carrier collsns
>>     106        18       0       0       0       0
>> root@imx6qrom5420b1:~#
>> /////re-connect cable
>>   can0  169   [8]  35 55 A3 1C 0F 47 2E 5B
>>   can0  318   [8]  11 AA 27 11 D2 1B CE 34
>>   can0  577   [8]  A0 A4 EE 50 8D A2 E1 3E
>>   can0  4ED   [8]  52 96 17 7E 31 FC 7D 7C
>>   can0  2E7   [8]  92 48 D4 39 05 1E 9F 50
>>   can0  200   [8]  4A 66 F6 02 1E 71 8E 26
>>   can0  29A   [8]  49 63 2E 7D C9 77 85 7A
>>   can0  15A   [7]  3C 0E 65 74 C3 62 80
>>   can0  011   [1]  D2
>>   can0  26B   [3]  FC D6 68
>>   can0  5CE   [8]  6F 02 B5 14 BC 7A D7 02
>>
>> 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-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          7          0          1          1          0
>>     RX: bytes  packets  errors  dropped overrun mcast
>>     0          0        7       0       0       0
>>     TX: bytes  packets  errors  dropped carrier collsns
>>     181        29       0       0       0       0
>>
>>
>> //Reboot the board and test with bus error reporting off
>>
>> root@imx6qrom5420b1:~# ip link set can0 up type can bitrate 1000000
>> berr-reporting off
>> root@imx6qrom5420b1:~# candump -e any,0:0,#FFFFFFF &
>> [1] 782
>> root@imx6qrom5420b1:~# cangen can0
>>   can0  1FA   [3]  C9 FE C2
>>   can0  3E2   [5]  85 37 03 5B 6F
>>   can0  289   [8]  A4 F6 BF 4A 3F 70 65 1B
>>   can0  12D   [8]  B2 72 10 33 AB B4 68 64
>>   can0  054   [2]  01 D7
>>   can0  4A6   [8]  29 7D 76 56 CA C1 60 00
>>   can0  768   [8]  97 3D 92 08 61 C1 D9 03
>>   can0  098   [6]  A4 A8 5A 60 92 1A
>>   can0  3C9   [8]  71 78 0D 25 AB 27 8B 51
>> /////disconnect cable
>> 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 state ERROR-ACTIVE (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          0          0          0          0          0
>>     RX: bytes  packets  errors  dropped overrun mcast
>>     0          0        0       0       0       0
>>     TX: bytes  packets  errors  dropped carrier collsns
>>     56         9        0       0       0       0
>> root@imx6qrom5420b1:~#
>> /////re-connect cable
>> can0  20000088   [8]  00 00 00 19 00 00 7F 00   ERRORFRAME
>>     protocol-violation{{}{acknowledge-slot}}
>>     bus-error
>>     error-counter-tx-rx{{127}{0}}
>>   can0  553   [6]  1A E4 60 6B DC 07
>>   can0  7E3   [8]  1C 78 95 6E 10 81 AA 40
>>   can0  20C   [8]  BB 35 13 25 60 0A 56 57
>>   can0  1D0   [8]  48 4A 39 64 76 E6 57 08
>>   can0  43A   [1]  40
>>   can0  2CF   [7]  03 45 5E 0F 67 33 4C
>>   can0  1CD   [8]  F9 4D AB 1D 96 A5 67 0E
>>   can0  515   [8]  41 CD F2 5F 68 92 43 16
>>   can0  661   [8]  45 9A 73 69 45 EE 8B 42
>>   can0  41B   [1]  55
>>   can0  52F   [1]  87
>
> 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.
>
>
>>
>> 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.
>
>>     RX: bytes  packets  errors  dropped overrun mcast
>>     0          0        1       0       0       0
>>     TX: bytes  packets  errors  dropped carrier collsns
>>     120        20       0       0       0       0
>>
>>
>>> Also, any error message should show the bus error counts in data[7,8]:
>>>
>>> http://lxr.free-electrons.com/source/drivers/net/can/sja1000/sja1000.c#L408
>>>
>>>
>>
>> I can add this in v4 version of the patch (Above log has this patch
>> applied).
>
> Looks good.
>
>>> And please check bus-off as well (short-circuiting CAN low and high).
>>>
>>
>> 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?

You could try to set a different bit-rate on the other CAN controller. 
Then try to send or receive messages.

Wolfgang.

[toc] | [prev] | [next] | [standalone]


#1601034

FromAkshay Bhat <nodeax@gmail.com>
Date2017-03-15 05:50 +0100
Message-ID<tl92N-2mK-11@gated-at.bofh.it>
In reply to#1600783
Hi Wolfgang,

On Tue, Mar 14, 2017 at 2:08 PM, Wolfgang Grandegger <wg@grandegger.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.

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

[toc] | [prev] | [next] | [standalone]


#1601112

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-15 08:30 +0100
Message-ID<tlbxE-4kc-25@gated-at.bofh.it>
In reply to#1601034
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@grandegger.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.

To double check: Could you please read INTF, ERR and STATF at the 
beginning of the ISR and print it out (using dev_dbg and fiends). Then 
run a test with no cable connected and bus error reporting off.

Wolfgang.

[toc] | [prev] | [next] | [standalone]


#1601186

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-15 10:50 +0100
Message-ID<tldJ8-5Lz-19@gated-at.bofh.it>
In reply to#1601034
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@grandegger.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@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.

Seems OK according to:

http://electronics.stackexchange.com/questions/220596/can-error-counters-behaviour

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

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.

[toc] | [prev] | [next] | [standalone]


#1602670

FromAkshay Bhat <akshay.bhat@timesys.com>
Date2017-03-16 18:10 +0100
Message-ID<tlH4t-1sh-21@gated-at.bofh.it>
In reply to#1601186
Hi Wolfgang,

On 03/15/2017 05:42 AM, Wolfgang Grandegger wrote:
> Hello Akshay,
> 
..snip..
>>
>> 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
> 
> 

I have fixed the driver to handle the error reporting. Also thanks for
your tip for generating bus-off by setting the host device at a
different CAN bit rate! Below are logs with the updated driver. Let me
know if you have any concerns, if not I will submit the v4 patch.

berr-reporting on case:
http://pastebin.com/qDRLERmW

berr-reporting off case:
http://pastebin.com/fUn3j7qU

Thanks,
Akshay

[toc] | [prev] | [next] | [standalone]


#1602818

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-16 21:10 +0100
Message-ID<tlJSG-3pV-27@gated-at.bofh.it>
In reply to#1602670
Hello Akshay,

Am 16.03.2017 um 18:06 schrieb Akshay Bhat:
> Hi Wolfgang,
> 
> On 03/15/2017 05:42 AM, Wolfgang Grandegger wrote:
>> Hello Akshay,
>>
> ..snip..
>>>
>>> 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
>>
>>
> 
> I have fixed the driver to handle the error reporting. Also thanks for
> your tip for generating bus-off by setting the host device at a
> different CAN bit rate! Below are logs with the updated driver. Let me
> know if you have any concerns, if not I will submit the v4 patch.
> 
> berr-reporting on case:
> http://pastebin.com/qDRLERmW

Looks much better now! There are message for state changes to error
warning and passive. Just the following message is not correct:

 (000.200824)  can0  20000004   [8]  00 40 00 00 00 00 5F 19   ERRORFRAME
    controller-problem{}
    error-counter-tx-rx{{95}{25}}

Sorry, forgot to mention... the function can_change_state() [1]
should be used for that purpose, if possible. It fixes the issue
above as well.

> berr-reporting off case:
> http://pastebin.com/fUn3j7qU

Ditto.

I just had another look to the manual and there is this undocumented
STATFE register at offset 0x1E. It's mentioned in some other parts of
the doc as interrupt enable register for STATF events. I would assume
the same bit layout than STATF. If you set bit 2 (BUSOFF), 3 (ERRP)
and 4 (ERRW), you may get interrupts. It's worth a try, I think. If
it works, it's the much better solution.

Wolfgang.

[1] http://lxr.free-electrons.com/ident?i=can_change_state

Wolfgang.

[toc] | [prev] | [next] | [standalone]


#1602894

FromAkshay Bhat <akshay.bhat@timesys.com>
Date2017-03-16 23:40 +0100
Message-ID<tlMdQ-4Yc-5@gated-at.bofh.it>
In reply to#1602818
Hi Wolfgang,

On 03/16/2017 04:02 PM, Wolfgang Grandegger wrote:
> 
> Looks much better now! There are message for state changes to error
> warning and passive. Just the following message is not correct:
> 
>  (000.200824)  can0  20000004   [8]  00 40 00 00 00 00 5F 19   ERRORFRAME
>     controller-problem{}
>     error-counter-tx-rx{{95}{25}}
> 
> Sorry, forgot to mention... the function can_change_state() [1]
> should be used for that purpose, if possible. It fixes the issue
> above as well.
> 

The updated driver (the one used to create the above log) is using
can_change_state() function. data[1] 40 corresponds to
CAN_ERR_CRTL_ACTIVE, so looks correct? Could it be that the can-utils I
am using is old and not reporting state change?

Tentative v4 driver for reference:
http://pastebin.com/4xFVL1Sj

>> berr-reporting off case:
>> http://pastebin.com/fUn3j7qU
> 
> Ditto.
> 
> I just had another look to the manual and there is this undocumented
> STATFE register at offset 0x1E. It's mentioned in some other parts of
> the doc as interrupt enable register for STATF events. I would assume
> the same bit layout than STATF. If you set bit 2 (BUSOFF), 3 (ERRP)
> and 4 (ERRW), you may get interrupts. It's worth a try, I think. If
> it works, it's the much better solution.
> 

The HI-311x has a INT pin and a STAT pin. The hardware I have has only
the INT pin connected to the processor. If the STAT pin was also
connected, then like you mentioned it could be a much better solution to
use STAT for state changes.

Enabling BUSOFF/ERRP/ERRW bits in STATFE did not generate any interrupts
on the INT pin. Should we make it a requirement that both INT and STAT
pins need to be connected in hardware for the driver to do the error
reporting?

Thanks,
Akshay

> Wolfgang.
> 
> [1] http://lxr.free-electrons.com/ident?i=can_change_state
> 
> Wolfgang.
> 

[toc] | [prev] | [next] | [standalone]


#1603089

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-17 08:40 +0100
Message-ID<tlUEq-2QJ-3@gated-at.bofh.it>
In reply to#1602894
Hello Akshay,

Am 16.03.2017 um 23:29 schrieb Akshay Bhat:
> Hi Wolfgang,
>
> On 03/16/2017 04:02 PM, Wolfgang Grandegger wrote:
>>
>> Looks much better now! There are message for state changes to error
>> warning and passive. Just the following message is not correct:
>>
>>  (000.200824)  can0  20000004   [8]  00 40 00 00 00 00 5F 19   ERRORFRAME
>>     controller-problem{}
>>     error-counter-tx-rx{{95}{25}}
>>
>> Sorry, forgot to mention... the function can_change_state() [1]
>> should be used for that purpose, if possible. It fixes the issue
>> above as well.
>>
>
> The updated driver (the one used to create the above log) is using
> can_change_state() function. data[1] 40 corresponds to
> CAN_ERR_CRTL_ACTIVE, so looks correct? Could it be that the can-utils I
> am using is old and not reporting state change?

Hm, yes. The raw data looks correct. You could download and build a 
recent version from "https://github.com/linux-can/can-utils" to check. 
It could also be a bug.

> Tentative v4 driver for reference:
> http://pastebin.com/4xFVL1Sj
>
>>> berr-reporting off case:
>>> http://pastebin.com/fUn3j7qU
>>
>> Ditto.
>>
>> I just had another look to the manual and there is this undocumented
>> STATFE register at offset 0x1E. It's mentioned in some other parts of
>> the doc as interrupt enable register for STATF events. I would assume
>> the same bit layout than STATF. If you set bit 2 (BUSOFF), 3 (ERRP)
>> and 4 (ERRW), you may get interrupts. It's worth a try, I think. If
>> it works, it's the much better solution.
>>
>
> The HI-311x has a INT pin and a STAT pin. The hardware I have has only
> the INT pin connected to the processor. If the STAT pin was also
> connected, then like you mentioned it could be a much better solution to
> use STAT for state changes.

OK, I understand.

> Enabling BUSOFF/ERRP/ERRW bits in STATFE did not generate any interrupts
> on the INT pin. Should we make it a requirement that both INT and STAT
> pins need to be connected in hardware for the driver to do the error
> reporting?

As I said, it's the better solution, especially if interrupt flooding 
does harm. How does your system behave when bus errors come in due to no 
cable connected?

So far using NAPI was mandatory. There is the problem of out-of-order 
message reception if handled in the isr on multi processor systems. 
Marc, what is the current policy?

Wolfgang.

[toc] | [prev] | [next] | [standalone]


#1603101

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-17 09:20 +0100
Message-ID<tlVh7-3rB-1@gated-at.bofh.it>
In reply to#1603089
Hello Akshay,

Am 17.03.2017 um 08:39 schrieb Wolfgang Grandegger:
> Hello Akshay,
>
> Am 16.03.2017 um 23:29 schrieb Akshay Bhat:
>> Hi Wolfgang,
>>
>> On 03/16/2017 04:02 PM, Wolfgang Grandegger wrote:
>>>
>>> Looks much better now! There are message for state changes to error
>>> warning and passive. Just the following message is not correct:
>>>
>>>  (000.200824)  can0  20000004   [8]  00 40 00 00 00 00 5F 19
>>> ERRORFRAME
>>>     controller-problem{}
>>>     error-counter-tx-rx{{95}{25}}
>>>
>>> Sorry, forgot to mention... the function can_change_state() [1]
>>> should be used for that purpose, if possible. It fixes the issue
>>> above as well.
>>>
>>
>> The updated driver (the one used to create the above log) is using
>> can_change_state() function. data[1] 40 corresponds to
>> CAN_ERR_CRTL_ACTIVE, so looks correct? Could it be that the can-utils I
>> am using is old and not reporting state change?
>
> Hm, yes. The raw data looks correct. You could download and build a
> recent version from "https://github.com/linux-can/can-utils" to check.
> It could also be a bug.

Support for that flags has been added in December 2014.

Wolfgang-

[toc] | [prev] | [next] | [standalone]


#1603432

FromAkshay Bhat <akshay.bhat@timesys.com>
Date2017-03-17 17:10 +0100
Message-ID<tm2BX-FF-9@gated-at.bofh.it>
In reply to#1603089
Hi Wolfgang,

On 03/17/2017 03:39 AM, Wolfgang Grandegger wrote:
> Hello Akshay,
> 
> Am 16.03.2017 um 23:29 schrieb Akshay Bhat:
>> Hi Wolfgang,
>>
>> On 03/16/2017 04:02 PM, Wolfgang Grandegger wrote:
>>>
>>> Looks much better now! There are message for state changes to error
>>> warning and passive. Just the following message is not correct:
>>>
>>>  (000.200824)  can0  20000004   [8]  00 40 00 00 00 00 5F 19  
>>> ERRORFRAME
>>>     controller-problem{}
>>>     error-counter-tx-rx{{95}{25}}
>>>
>>> Sorry, forgot to mention... the function can_change_state() [1]
>>> should be used for that purpose, if possible. It fixes the issue
>>> above as well.
>>>
>>
>> The updated driver (the one used to create the above log) is using
>> can_change_state() function. data[1] 40 corresponds to
>> CAN_ERR_CRTL_ACTIVE, so looks correct? Could it be that the can-utils I
>> am using is old and not reporting state change?
> 
> Hm, yes. The raw data looks correct. You could download and build a
> recent version from "https://github.com/linux-can/can-utils" to check.
> It could also be a bug.
> 

Turned out to be a old version of can-utils. Using the above git tree
reports the flag.

 (000.200308)  can0  20000004   [8]  00 40 00 00 00 00 5F 00   ERRORFRAME
        controller-problem{back-to-error-active}
        error-counter-tx-rx{{95}{0}}

>> Enabling BUSOFF/ERRP/ERRW bits in STATFE did not generate any interrupts
>> on the INT pin. Should we make it a requirement that both INT and STAT
>> pins need to be connected in hardware for the driver to do the error
>> reporting?
> 
> As I said, it's the better solution, especially if interrupt flooding
> does harm. How does your system behave when bus errors come in due to no
> cable connected?
>

I did not see any issues on the system with the cable disconnected. In
my particular setup with the cable disconnected the system goes to
tx-error-passive and does not get any further interrupts until a state
change occurs.

> So far using NAPI was mandatory. There is the problem of out-of-order
> message reception if handled in the isr on multi processor systems.
> Marc, what is the current policy?
> 

Since this is a SPI based CAN, I am wary for any additional latencies
NAPI might introduce. The RX handling is being done at the very
beginning of the ISR for this reason.

Can we go ahead with the existing implementation and re-visit this at a
later time?

Thanks again for all your help in reviewing/improving the driver :)

Akshay

> Wolfgang.

[toc] | [prev] | [next] | [standalone]


#1603489

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-17 18:40 +0100
Message-ID<tm414-1ya-27@gated-at.bofh.it>
In reply to#1603432
Hi Akshay,

Am 17.03.2017 um 17:00 schrieb Akshay Bhat:
> Hi Wolfgang,
>
> On 03/17/2017 03:39 AM, Wolfgang Grandegger wrote:
>> Hello Akshay,
>>
>> Am 16.03.2017 um 23:29 schrieb Akshay Bhat:
>>> Hi Wolfgang,
>>>
>>> On 03/16/2017 04:02 PM, Wolfgang Grandegger wrote:
>>>>
>>>> Looks much better now! There are message for state changes to error
>>>> warning and passive. Just the following message is not correct:
>>>>
>>>>  (000.200824)  can0  20000004   [8]  00 40 00 00 00 00 5F 19
>>>> ERRORFRAME
>>>>     controller-problem{}
>>>>     error-counter-tx-rx{{95}{25}}
>>>>
>>>> Sorry, forgot to mention... the function can_change_state() [1]
>>>> should be used for that purpose, if possible. It fixes the issue
>>>> above as well.
>>>>
>>>
>>> The updated driver (the one used to create the above log) is using
>>> can_change_state() function. data[1] 40 corresponds to
>>> CAN_ERR_CRTL_ACTIVE, so looks correct? Could it be that the can-utils I
>>> am using is old and not reporting state change?
>>
>> Hm, yes. The raw data looks correct. You could download and build a
>> recent version from "https://github.com/linux-can/can-utils" to check.
>> It could also be a bug.
>>
>
> Turned out to be a old version of can-utils. Using the above git tree
> reports the flag.
>
>  (000.200308)  can0  20000004   [8]  00 40 00 00 00 00 5F 00   ERRORFRAME
>         controller-problem{back-to-error-active}
>         error-counter-tx-rx{{95}{0}}
>
>>> Enabling BUSOFF/ERRP/ERRW bits in STATFE did not generate any interrupts
>>> on the INT pin. Should we make it a requirement that both INT and STAT
>>> pins need to be connected in hardware for the driver to do the error
>>> reporting?
>>
>> As I said, it's the better solution, especially if interrupt flooding
>> does harm. How does your system behave when bus errors come in due to no
>> cable connected?
>>
>
> I did not see any issues on the system with the cable disconnected. In
> my particular setup with the cable disconnected the system goes to
> tx-error-passive and does not get any further interrupts until a state
> change occurs.

Hm, that's unusual. Cable disconnected and then send a message:

$ grep /proc/interrupts; sleep 10; /proc/interrupts

should make things clear. But maybe it's a clever chip and it does stop 
sending error messages if the error counter does not change any more. 
After bus-off, the chip is quiet, of course. Should have a closer look 
to the CAN standard.

>> So far using NAPI was mandatory. There is the problem of out-of-order
>> message reception if handled in the isr on multi processor systems.
>> Marc, what is the current policy?
>>
>
> Since this is a SPI based CAN, I am wary for any additional latencies
> NAPI might introduce. The RX handling is being done at the very
> beginning of the ISR for this reason.
>
> Can we go ahead with the existing implementation and re-visit this at a
> later time?

Likely yes, as Marc has already reviewed the driver once.

BTW: what system board/processor are you using?

> Thanks again for all your help in reviewing/improving the driver :)

You are welcome!

Wolfgang.

[toc] | [prev] | [next] | [standalone]


#1603539

FromAkshay Bhat <akshay.bhat@timesys.com>
Date2017-03-17 19:40 +0100
Message-ID<tm4X7-2fG-13@gated-at.bofh.it>
In reply to#1603489
Hi Wolfgang,

On 03/17/2017 01:04 PM, Wolfgang Grandegger wrote:
> 
> Hm, that's unusual. Cable disconnected and then send a message:
> 
> $ grep /proc/interrupts; sleep 10; /proc/interrupts
> 
> should make things clear. But maybe it's a clever chip and it does stop
> sending error messages if the error counter does not change any more.
> After bus-off, the chip is quiet, of course. Should have a closer look
> to the CAN standard.
>

The interrupt count does not increment after device reaches
tx-error-passive (with cable disconnected).

# while true; do grep -i hi3110 /proc/interrupts; sleep 10; done &
[1] 793
#
111:          0          0  gpio-mxc  12 Edge      hi3110
# candump -t d -e any,0:0,#FFFFFFF &
[2] 798
# cansend can0 123#
#
 (000.000000)  can0  20000004   [8]  00 08 00 00 00 00 60 00   ERRORFRAME
	controller-problem{tx-error-warning}
	error-counter-tx-rx{{96}{0}}
 (000.002122)  can0  20000004   [8]  00 20 00 00 00 00 80 00   ERRORFRAME
	controller-problem{tx-error-passive}
	error-counter-tx-rx{{128}{0}}
111:         10          0  gpio-mxc  12 Edge      hi3110
111:         10          0  gpio-mxc  12 Edge      hi3110
111:         10          0  gpio-mxc  12 Edge      hi3110

>>> So far using NAPI was mandatory. There is the problem of out-of-order
>>> message reception if handled in the isr on multi processor systems.
>>> Marc, what is the current policy?
>>>
>>
>> Since this is a SPI based CAN, I am wary for any additional latencies
>> NAPI might introduce. The RX handling is being done at the very
>> beginning of the ISR for this reason.
>>
>> Can we go ahead with the existing implementation and re-visit this at a
>> later time?
> 
> Likely yes, as Marc has already reviewed the driver once.
> 

Thanks, I will go ahead and submit v4 patch.

> BTW: what system board/processor are you using?
> 

It is a custom board using Phytec phyFLEX-i.MX6 Dual SOM.

[toc] | [prev] | [next] | [standalone]


#1603782

FromWolfgang Grandegger <wg@grandegger.com>
Date2017-03-18 13:40 +0100
Message-ID<tmlOi-6iH-15@gated-at.bofh.it>
In reply to#1603539
Hello Akshay,

Am 17.03.2017 um 19:28 schrieb Akshay Bhat:
> Hi Wolfgang,
>
> On 03/17/2017 01:04 PM, Wolfgang Grandegger wrote:
>>
>> Hm, that's unusual. Cable disconnected and then send a message:
>>
>> $ grep /proc/interrupts; sleep 10; /proc/interrupts
>>
>> should make things clear. But maybe it's a clever chip and it does stop
>> sending error messages if the error counter does not change any more.
>> After bus-off, the chip is quiet, of course. Should have a closer look
>> to the CAN standard.
>>
>
> The interrupt count does not increment after device reaches
> tx-error-passive (with cable disconnected).
>
> # while true; do grep -i hi3110 /proc/interrupts; sleep 10; done &
> [1] 793
> #
> 111:          0          0  gpio-mxc  12 Edge      hi3110
> # candump -t d -e any,0:0,#FFFFFFF &
> [2] 798
> # cansend can0 123#
> #
>  (000.000000)  can0  20000004   [8]  00 08 00 00 00 00 60 00   ERRORFRAME
> 	controller-problem{tx-error-warning}
> 	error-counter-tx-rx{{96}{0}}
>  (000.002122)  can0  20000004   [8]  00 20 00 00 00 00 80 00   ERRORFRAME
> 	controller-problem{tx-error-passive}
> 	error-counter-tx-rx{{128}{0}}
> 111:         10          0  gpio-mxc  12 Edge      hi3110
> 111:         10          0  gpio-mxc  12 Edge      hi3110
> 111:         10          0  gpio-mxc  12 Edge      hi3110

OK, then there is no good reason connecting the STAT interrupt pin.

Wolfgang.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web