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