Messages in this thread | | | Subject | Re: [PATCH v2 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver | From | Wolfgang Grandegger <> | Date | Fri, 17 Mar 2017 18:04:42 +0100 |
| |
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.
| |