Re: [PATCH v3] can: grcan: Add device driver for GRCAN and GRHCAN cores
From: Andreas Larsson <andreas@gaisler.com>
Date: 2012-10-31 16:33:25
Also in:
linux-devicetree
On 2012-10-31 13:51, Wolfgang Grandegger wrote:
On 10/30/2012 05:24 PM, Andreas Larsson wrote:quoted
On 10/30/2012 11:07 AM, Wolfgang Grandegger wrote:quoted
quoted
+ /* AHB bus error interrupts (not CAN bus errors) - shut down the + * device. + */ + if (sources & (GRCAN_IRQ_TXAHBERR | GRCAN_IRQ_RXAHBERR)) { + if (sources & GRCAN_IRQ_TXAHBERR) { + netdev_err(dev, "got AHB bus error on tx\n"); + stats->tx_errors++; + } else { + netdev_err(dev, "got AHB bus error on rx\n"); + stats->rx_errors++; + } + netdev_err(dev, "halting device\n"); + + /* Prevent anything to be enabled again and halt device */ + SPIN_LOCK(&priv->lock); + priv->closing = true; + netif_stop_queue(dev); + grcan_stop(dev); + SPIN_UNLOCK(&priv->lock);Hm, does that really happen? How can the user/app realized the problem and recover?My understanding of it is that if you get amba bus errors something is seriously wrong and nothing can be done at driver level to recover. Shutting down the device is to prevent the driver from spamming console messages. I used to have a sysfs indication of such errors. Now dmesg is the way to find out about the problem. The user can always bring the interface down and up again and try again after such an error.Well, sounds like a fatal error. Did you ever saw it? If that happens frequently, or even once, the device/system seems not really usable.
Fatal and yes if this is ever seen something is very bad with the system. I have never seen it. However the hardware reports if it would happen, so why not try to take care of it in some manner. If this is seen the can device is probably useless but it is possible that the rest of the system might be usable. I don't really know in what circumstances this would trigger.
quoted
quoted
Furthermore, why is a spin_clock enough here? THe interrupt may run a thread.It should be enough because the function is only called directly from the interrupt handler, right? Or did I miss something?The interrupt handler may run as thread, e.g. if the -rt patch has been applied. Therefore it's safer to use the irqsave/restore variants of the spin_lock functions. At least that's my understanding. See also "Documentation/spinlock.txt".
I'll look into this. Seems strange if the same cpu can be interrupted in an interrupt handler to run the very same interrupt handler. Sounds like a horrible situation for any driver that does not lock down most of its interrupt handler. Or maybe there is some situation that I don't foresee.
quoted
quoted
quoted
+ priv->can.ctrlmode_supported = + CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT;What about triple-sampling?That is not supported by the hardware.
Well, now it will be in future versions of the controller, so support is added to the driver.
quoted
quoted
I curious how the device behaves on bus errors and state changes. Could you please show the output of "candump -e any,0:0,#FFFFFFFF" while sending a CAN message with no other node on the bus (not connected) and with going bus off (by short-circuiting CAN high and low).Here is the output (with long sequences of similar error frames where only one counter is increasing cut out) from the the upcoming v4. let me know if you see any problems with this.How did your trigger that sequence? Short-circuit or not cable connected? The latter, I assume, as bus-off is not reached.
Triggered by short-circuiting, just as asked.
quoted
can0 20000006 [8] 00 00 00 00 00 00 10 00 ERRORFRAME lost-arbitration{at bit 0} controller-problem{} error-counter-tx-rx{{16}{0}}This is most probably an ack slot bus error similar to. You seem not to handle bus errors and it's not a controller problem as the state did not change.
No, the hardware has no support for can-bus error reporting.
http://lxr.linux.no/#linux+v3.6.4/drivers/net/can/flexcan.c#L353quoted
can0 20000004 [8] 00 20 00 00 00 00 88 00 ERRORFRAME controller-problem{tx-error-passive} error-counter-tx-rx{{136}{0}}OK, a state change to error passive. The device seems not to report changes to error warning.
No, there is no interrupts or anything that alert about the error states. That is why I set the sate based on the error counters in the error handler.
quoted
can0 20000006 [8] 00 20 00 00 00 00 90 00 ERRORFRAME lost-arbitration{at bit 0} controller-problem{tx-error-passive} error-counter-tx-rx{{144}{0}}State changes should be reported only once.
Do you mean that an error message should only be sent on state change?
But it did not change. This is then a bus error (CAN_ERR_PROT | CAN_ERR_BUSERROR). See also above.quoted
[...] can0 20000006 [8] 00 20 00 00 00 00 F0 00 ERRORFRAME lost-arbitration{at bit 0} controller-problem{tx-error-passive} error-counter-tx-rx{{240}{0}} can0 20000006 [8] 00 20 00 00 00 00 F8 00 ERRORFRAME lost-arbitration{at bit 0} controller-problem{tx-error-passive} error-counter-tx-rx{{248}{0}} can0 20000042 [8] 00 00 00 00 00 00 80 00 ERRORFRAME lost-arbitration{at bit 0} bus-off error-counter-tx-rx{{128}{0}} can0 20000006 [8] 00 00 00 00 00 00 18 00 ERRORFRAME lost-arbitration{at bit 0} controller-problem{} error-counter-tx-rx{{24}{0}} can0 20000004 [8] 00 10 00 00 00 00 4F 80 ERRORFRAME controller-problem{rx-error-passive} error-counter-tx-rx{{79}{128}} [...] can0 20000006 [8] 00 10 00 00 00 00 77 80 ERRORFRAME lost-arbitration{at bit 0} controller-problem{rx-error-passive} error-counter-tx-rx{{119}{128}} can0 20000006 [8] 00 30 00 00 00 00 7F 80 ERRORFRAME lost-arbitration{at bit 0} controller-problem{rx-error-passive,tx-error-passive} error-counter-tx-rx{{127}{128}} can0 20000006 [8] 00 30 00 00 00 00 87 80 ERRORFRAME lost-arbitration{at bit 0} controller-problem{rx-error-passive,tx-error-passive} error-counter-tx-rx{{135}{128}} [...] can0 20000006 [8] 00 30 00 00 00 00 F7 80 ERRORFRAME lost-arbitration{at bit 0} controller-problem{rx-error-passive,tx-error-passive} error-counter-tx-rx{{247}{128}} can0 20000006 [8] 00 30 00 00 00 00 FF 80 ERRORFRAME lost-arbitration{at bit 0} controller-problem{rx-error-passive,tx-error-passive} error-counter-tx-rx{{255}{128}} can0 20000042 [8] 00 00 00 00 00 00 80 00 ERRORFRAME lost-arbitration{at bit 0} bus-off error-counter-tx-rx{{128}{0}} can0 20000006 [8] 00 00 00 00 00 00 18 00 ERRORFRAME lost-arbitration{at bit 0} controller-problem{} error-counter-tx-rx{{24}{0}} can0 20000004 [8] 00 10 00 00 00 00 4F 80 ERRORFRAME controller-problem{rx-error-passive} error-counter-tx-rx{{79}{128}} can0 20000006 [8] 00 10 00 00 00 00 57 80 ERRORFRAME lost-arbitration{at bit 0} controller-problem{rx-error-passive} error-counter-tx-rx{{87}{128}} [...]Also a sequence to bus-off including the recovery would be nice.
Here is a longer unedited run up to the second bus-off (also with
short-circuited bus)
can0 20000006 [8] 00 00 00 00 00 00 20 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{}
error-counter-tx-rx{{32}{0}}
can0 20000004 [8] 00 20 00 00 00 00 88 00 ERRORFRAME
controller-problem{tx-error-passive}
error-counter-tx-rx{{136}{0}}
can0 20000006 [8] 00 20 00 00 00 00 90 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{144}{0}}
can0 20000006 [8] 00 20 00 00 00 00 98 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{152}{0}}
can0 20000006 [8] 00 20 00 00 00 00 A0 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{160}{0}}
can0 20000006 [8] 00 20 00 00 00 00 A8 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{168}{0}}
can0 20000006 [8] 00 20 00 00 00 00 B0 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{176}{0}}
can0 20000006 [8] 00 20 00 00 00 00 B8 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{184}{0}}
can0 20000006 [8] 00 20 00 00 00 00 C0 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{192}{0}}
can0 20000006 [8] 00 20 00 00 00 00 C8 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{200}{0}}
can0 20000006 [8] 00 20 00 00 00 00 D0 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{208}{0}}
can0 20000006 [8] 00 20 00 00 00 00 D8 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{216}{0}}
can0 20000006 [8] 00 20 00 00 00 00 E0 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{224}{0}}
can0 20000006 [8] 00 20 00 00 00 00 E8 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{232}{0}}
can0 20000006 [8] 00 20 00 00 00 00 F0 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{240}{0}}
can0 20000006 [8] 00 20 00 00 00 00 F8 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{248}{0}}
can0 20000042 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
lost-arbitration{at bit 0}
bus-off
error-counter-tx-rx{{128}{0}}
can0 20000006 [8] 00 00 00 00 00 00 20 00 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{}
error-counter-tx-rx{{32}{0}}
can0 20000006 [8] 00 10 00 00 00 00 57 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive}
error-counter-tx-rx{{87}{128}}
can0 20000006 [8] 00 10 00 00 00 00 5F 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive}
error-counter-tx-rx{{95}{128}}
can0 20000006 [8] 00 10 00 00 00 00 67 80 ERRORFRAME
lost-arbitration{at bit 0}
error-counter-tx-rx{{103}{128}}
can0 20000006 [8] 00 10 00 00 00 00 6F 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive}
error-counter-tx-rx{{111}{128}}
can0 20000006 [8] 00 10 00 00 00 00 77 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive}
error-counter-tx-rx{{119}{128}}
can0 20000006 [8] 00 30 00 00 00 00 7F 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{127}{128}}
can0 20000006 [8] 00 30 00 00 00 00 87 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{135}{128}}
can0 20000006 [8] 00 30 00 00 00 00 8F 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{143}{128}}
can0 20000006 [8] 00 30 00 00 00 00 97 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{151}{128}}
can0 20000006 [8] 00 30 00 00 00 00 9F 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{159}{128}}
can0 20000006 [8] 00 30 00 00 00 00 A7 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{167}{128}}
can0 20000006 [8] 00 30 00 00 00 00 AF 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{175}{128}}
can0 20000006 [8] 00 30 00 00 00 00 B7 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{183}{128}}
can0 20000006 [8] 00 30 00 00 00 00 BF 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{191}{128}}
can0 20000006 [8] 00 30 00 00 00 00 C7 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{199}{128}}
can0 20000006 [8] 00 30 00 00 00 00 CF 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{207}{128}}
can0 20000006 [8] 00 30 00 00 00 00 D7 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{215}{128}}
can0 20000006 [8] 00 30 00 00 00 00 DF 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{223}{128}}
can0 20000006 [8] 00 30 00 00 00 00 E7 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{231}{128}}
can0 20000006 [8] 00 30 00 00 00 00 EF 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{239}{128}}
can0 20000006 [8] 00 30 00 00 00 00 F7 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{247}{128}}
can0 20000006 [8] 00 30 00 00 00 00 FF 80 ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{255}{128}}
can0 20000042 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
lost-arbitration{at bit 0}
bus-off
error-counter-tx-rx{{128}{0}}
Thanks for the input! I'll send in v4 so you can see what the code that
generated the above.
Cheers,
Andreas