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

From	Akshay Bhat <akshay.bhat@timesys.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver
Date	2017-03-16 23:40 +0100
Message-ID	<tlMdQ-4Yc-5@gated-at.bofh.it> (permalink)
References	(6 earlier) <tkZ3s-3WN-29@gated-at.bofh.it> <tl92N-2mK-11@gated-at.bofh.it> <tldJ8-5Lz-19@gated-at.bofh.it> <tlH4t-1sh-21@gated-at.bofh.it> <tlJSG-3pV-27@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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

Thread

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

csiph-web