Hello Joe,
Am 21.06.2018 um 15:04 schrieb Joe Burmeister:
> Hi Wolfgang,
>
>
> On 21/06/18 13:59, Wolfgang Grandegger wrote:
>
> <snip>
>
>> Looking to the code, I see a problem with reading the status register
>> (C_CAN_STS_REG). It should *only* be read if bit 15 of the interrupt
>> status register (C_CAN_INT_REG) is set/pending. According to the manual,
>> reading the status register will also clear the bit in C_CAN_INT_REG.
>> This could explain lost (bus-off) status interrupts and also the strange
>> messages I mentioned above.
>>
>> Wolfgang.
>
> It sounds plausible. I can try it. Have you got a good datasheet for D_Can ?
I have attached my (untested) patch. TXOK and RXOK is still handled
unconditionally, but that's maybe less critical.
> Best I have is:
>
> https://www.yumpu.com/en/document/view/24499391/d-can-bosch-semiconductors-and-sensors
>
> which is terrible!
I'm looking to the C_CAN manual you listed in a previous Email. I think
D_CAN is mainly the same core just with different register mapping.
Wolfgang.
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 606b7d8..17edb67 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -97,6 +97,9 @@
#define BTR_TSEG2_SHIFT 12
#define BTR_TSEG2_MASK (0x7 << BTR_TSEG2_SHIFT)
+/* interrupt register */
+#define INT_STS_PENDING 0x8000
+
/* brp extension register */
#define BRP_EXT_BRPE_MASK 0x0f
#define BRP_EXT_BRPE_SHIFT 0
@@ -1029,41 +1032,49 @@ static int c_can_poll(struct napi_struct *napi, int quota)
u16 curr, last = priv->last_status;
int work_done = 0;
- priv->last_status = curr = priv->read_reg(priv, C_CAN_STS_REG);
- /* Ack status on C_CAN. D_CAN is self clearing */
- if (priv->type != BOSCH_D_CAN)
- priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
+ /* Only read the status register if a status interrupt is pending */
+ if (priv->read_reg(priv, C_CAN_INT_REG) & INT_STS_PENDING) {
+ priv->last_status = curr = priv->read_reg(priv, C_CAN_STS_REG);
+ /* Ack status on C_CAN. D_CAN is self clearing */
+ if (priv->type != BOSCH_D_CAN)
+ priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
+
+ /* handle state changes */
+ if ((curr & STATUS_EWARN) && (!(last & STATUS_EWARN))) {
+ netdev_dbg(dev, "entered error warning state\n");
+ work_done +=
+ c_can_handle_state_change(dev,
+ C_CAN_ERROR_WARNING);
+ }
- /* handle state changes */
- if ((curr & STATUS_EWARN) && (!(last & STATUS_EWARN))) {
- netdev_dbg(dev, "entered error warning state\n");
- work_done += c_can_handle_state_change(dev, C_CAN_ERROR_WARNING);
- }
+ if ((curr & STATUS_EPASS) && (!(last & STATUS_EPASS))) {
+ netdev_dbg(dev, "entered error passive state\n");
+ work_done +=
+ c_can_handle_state_change(dev,
+ C_CAN_ERROR_PASSIVE);
+ }
- if ((curr & STATUS_EPASS) && (!(last & STATUS_EPASS))) {
- netdev_dbg(dev, "entered error passive state\n");
- work_done += c_can_handle_state_change(dev, C_CAN_ERROR_PASSIVE);
- }
+ if ((curr & STATUS_BOFF) && (!(last & STATUS_BOFF))) {
+ netdev_dbg(dev, "entered bus off state\n");
+ work_done +=
+ c_can_handle_state_change(dev, C_CAN_BUS_OFF);
+ goto end;
+ }
- if ((curr & STATUS_BOFF) && (!(last & STATUS_BOFF))) {
- netdev_dbg(dev, "entered bus off state\n");
- work_done += c_can_handle_state_change(dev, C_CAN_BUS_OFF);
- goto end;
- }
+ /* handle bus recovery events */
+ if ((!(curr & STATUS_BOFF)) && (last & STATUS_BOFF)) {
+ netdev_dbg(dev, "left bus off state\n");
+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ }
+ if ((!(curr & STATUS_EPASS)) && (last & STATUS_EPASS)) {
+ netdev_dbg(dev, "left error passive state\n");
+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ }
- /* handle bus recovery events */
- if ((!(curr & STATUS_BOFF)) && (last & STATUS_BOFF)) {
- netdev_dbg(dev, "left bus off state\n");
- priv->can.state = CAN_STATE_ERROR_ACTIVE;
- }
- if ((!(curr & STATUS_EPASS)) && (last & STATUS_EPASS)) {
- netdev_dbg(dev, "left error passive state\n");
- priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ /* handle lec errors on the bus */
+ work_done += c_can_handle_bus_err(dev, curr & LEC_MASK);
}
- /* handle lec errors on the bus */
- work_done += c_can_handle_bus_err(dev, curr & LEC_MASK);
-
/* Handle Tx/Rx events. We do this unconditionally */
work_done += c_can_do_rx_poll(dev, (quota - work_done));
c_can_do_tx(dev);
