Skip to content

L
linux
Commit 1dbd8748 authored by Marc Kleine-Budde's avatar Marc Kleine-Budde
Browse files
Options

Merge branch 'can-error-set-of-fixes-and-improvement-on-txerr-and-rxerr-reporting' 

Vincent Mailhol says:

====================
can: error: set of fixes and improvement on txerr and rxerr reporting

This series is a collection of patches targeting the CAN error
counter. The series is split in three blocks (with small relation to
each other).

Several drivers uses the data[6] and data[7] fields (both of type u8)
of the CAN error frame to report those values. However, the maximum
size an u8 can hold is 255 and the error counter can exceed this value
if bus-off status occurs. As such, the first nine patches of this
series make sure that no drivers try to report txerr or rxerr through
the CAN error frame when bus-off status is reached.

can_frame::data[5..7] are defined as being "controller
specific". Controller specific behaviors are not something desirable
(portability issue...) The tenth patch of this series specifies how
can_frame::data[5..7] should be use and remove any "controller
specific" freedom. The eleventh patch adds a flag to notify though
can_frame::can_id that data[6..7] were populated (in order to be
consistent with other fields).

Finally, the twelfth and last patch add three macro values to specify
the different error counter threshold with so far was hard-coded as
magic numbers in the drivers.

N.B.:
  * patches 1 to 10 are for net (stable).
  * patches 11 and 12 are for net-next (but depends on patches 1 to 10).

** Changelog **

v1 -> v2: https://lore.kernel.org/all/20220712153157.83847-1-mailhol.vincent@wanadoo.fr
  * Fix typo in patch #10: data[7] of CAN error frames is for the RX
    error counter, not the TX one (this is litteraly a one byte
    change).
====================

As discussed take the whole series via can-next -> net-next.

Link: https://lore.kernel.org/all/20220719143550.3681-1-mailhol.vincent@wanadoo.frSigned-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parents d79ee9a6 3f9c2621
Hide whitespace changes
Inline Side-by-side
Showing with 94 additions and 52 deletions
drivers/net/can/c_can/c_can_main.c
View file @ 1dbd8748
......@@ -952,14 +952,14 @@ static int c_can_handle_state_change(struct net_device *dev,
switch (error_type) {
case C_CAN_NO_ERROR:
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = CAN_ERR_CRTL_ACTIVE;
cf->data[6] = bec.txerr;
cf->data[7] = bec.rxerr;
break;
case C_CAN_ERROR_WARNING:
/* error warning state */
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = (bec.txerr > bec.rxerr) ?
CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
......@@ -969,7 +969,7 @@ static int c_can_handle_state_change(struct net_device *dev,
break;
case C_CAN_ERROR_PASSIVE:
/* error passive state */
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
if (rx_err_passive)
cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
if (bec.txerr > 127)
......
drivers/net/can/cc770/cc770.c
View file @ 1dbd8748
......@@ -512,6 +512,7 @@ static int cc770_err(struct net_device *dev, u8 status)
/* Use extended functions of the CC770 */
if (priv->control_normal_mode & CTRL_EAF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = cc770_read_reg(priv, tx_error_counter);
cf->data[7] = cc770_read_reg(priv, rx_error_counter);
}
......
drivers/net/can/ctucanfd/ctucanfd_base.c
View file @ 1dbd8748
......@@ -847,7 +847,7 @@ static void ctucan_err_interrupt(struct net_device *ndev, u32 isr)
case CAN_STATE_ERROR_PASSIVE:
priv->can.can_stats.error_passive++;
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = (bec.rxerr > 127) ?
CAN_ERR_CRTL_RX_PASSIVE :
CAN_ERR_CRTL_TX_PASSIVE;
......@@ -858,7 +858,7 @@ static void ctucan_err_interrupt(struct net_device *ndev, u32 isr)
case CAN_STATE_ERROR_WARNING:
priv->can.can_stats.error_warning++;
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] |= (bec.txerr > bec.rxerr) ?
CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
......@@ -867,6 +867,7 @@ static void ctucan_err_interrupt(struct net_device *ndev, u32 isr)
}
break;
case CAN_STATE_ERROR_ACTIVE:
cf->can_id |= CAN_ERR_CNT;
cf->data[1] = CAN_ERR_CRTL_ACTIVE;
cf->data[6] = bec.txerr;
cf->data[7] = bec.rxerr;
......
drivers/net/can/grcan.c
View file @ 1dbd8748
......@@ -671,6 +671,7 @@ static void grcan_err(struct net_device *dev, u32 sources, u32 status)
/* There are no others at this point */
break;
}
cf.can_id |= CAN_ERR_CNT;
cf.data[6] = txerr;
cf.data[7] = rxerr;
priv->can.state = state;
......
drivers/net/can/ifi_canfd/ifi_canfd.c
View file @ 1dbd8748
......@@ -492,7 +492,7 @@ static int ifi_canfd_handle_state_change(struct net_device *ndev,
switch (new_state) {
case CAN_STATE_ERROR_WARNING:
/* error warning state */
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = (bec.txerr > bec.rxerr) ?
CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
......@@ -501,7 +501,7 @@ static int ifi_canfd_handle_state_change(struct net_device *ndev,
break;
case CAN_STATE_ERROR_PASSIVE:
/* error passive state */
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
if (bec.txerr > 127)
cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
......
drivers/net/can/janz-ican3.c
View file @ 1dbd8748
......@@ -1127,7 +1127,7 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg)
/* bus error interrupt */
if (isrc == CEVTIND_BEI) {
mod->can.can_stats.bus_error++;
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR | CAN_ERR_CNT;
switch (ecc & ECC_MASK) {
case ECC_BIT:
......@@ -1153,7 +1153,7 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg)
if (state != mod->can.state && (state == CAN_STATE_ERROR_WARNING ||
state == CAN_STATE_ERROR_PASSIVE)) {
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
if (state == CAN_STATE_ERROR_WARNING) {
mod->can.can_stats.error_warning++;
cf->data[1] = (txerr > rxerr) ?
......
drivers/net/can/kvaser_pciefd.c
View file @ 1dbd8748
......@@ -1306,7 +1306,7 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
shhwtstamps->hwtstamp =
ns_to_ktime(div_u64(p->timestamp * 1000,
can->kv_pcie->freq_to_ticks_div));
cf->can_id |= CAN_ERR_BUSERROR;
cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_CNT;
cf->data[6] = bec.txerr;
cf->data[7] = bec.rxerr;
......
drivers/net/can/m_can/m_can.c
View file @ 1dbd8748
......@@ -741,7 +741,7 @@ static int m_can_handle_state_change(struct net_device *dev,
switch (new_state) {
case CAN_STATE_ERROR_WARNING:
/* error warning state */
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = (bec.txerr > bec.rxerr) ?
CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
......@@ -750,7 +750,7 @@ static int m_can_handle_state_change(struct net_device *dev,
break;
case CAN_STATE_ERROR_PASSIVE:
/* error passive state */
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
ecr = m_can_read(cdev, M_CAN_ECR);
if (ecr & ECR_RP)
cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
......
drivers/net/can/pch_can.c
View file @ 1dbd8748
......@@ -496,6 +496,10 @@ static void pch_can_error(struct net_device *ndev, u32 status)
cf->can_id |= CAN_ERR_BUSOFF;
priv->can.can_stats.bus_off++;
can_bus_off(ndev);
} else {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = errc & PCH_TEC;
cf->data[7] = (errc & PCH_REC) >> 8;
}
errc = ioread32(&priv->regs->errc);
......@@ -556,9 +560,6 @@ static void pch_can_error(struct net_device *ndev, u32 status)
break;
}
cf->data[6] = errc & PCH_TEC;
cf->data[7] = (errc & PCH_REC) >> 8;
priv->can.state = state;
netif_receive_skb(skb);
}
......
drivers/net/can/peak_canfd/peak_canfd.c
View file @ 1dbd8748
......@@ -373,7 +373,7 @@ static int pucan_handle_status(struct peak_canfd_priv *priv,
priv->can.state = CAN_STATE_ERROR_PASSIVE;
priv->can.can_stats.error_passive++;
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = (priv->bec.txerr > priv->bec.rxerr) ?
CAN_ERR_CRTL_TX_PASSIVE :
CAN_ERR_CRTL_RX_PASSIVE;
......@@ -386,7 +386,7 @@ static int pucan_handle_status(struct peak_canfd_priv *priv,
priv->can.state = CAN_STATE_ERROR_WARNING;
priv->can.can_stats.error_warning++;
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = (priv->bec.txerr > priv->bec.rxerr) ?
CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
......@@ -430,7 +430,7 @@ static int pucan_handle_cache_critical(struct peak_canfd_priv *priv)
return -ENOMEM;
}
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
cf->data[6] = priv->bec.txerr;
......
drivers/net/can/rcar/rcar_can.c
View file @ 1dbd8748
......@@ -232,11 +232,8 @@ static void rcar_can_error(struct net_device *ndev)
if (eifr & (RCAR_CAN_EIFR_EWIF | RCAR_CAN_EIFR_EPIF)) {
txerr = readb(&priv->regs->tecr);
rxerr = readb(&priv->regs->recr);
if (skb) {
if (skb)
cf->can_id |= CAN_ERR_CRTL;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
}
if (eifr & RCAR_CAN_EIFR_BEIF) {
int rx_errors = 0, tx_errors = 0;
......@@ -336,6 +333,10 @@ static void rcar_can_error(struct net_device *ndev)
can_bus_off(ndev);
if (skb)
cf->can_id |= CAN_ERR_BUSOFF;
} else if (skb) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
if (eifr & RCAR_CAN_EIFR_ORIF) {
netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
......
drivers/net/can/rcar/rcar_canfd.c
View file @ 1dbd8748
......@@ -1052,7 +1052,7 @@ static void rcar_canfd_error(struct net_device *ndev, u32 cerfl,
netdev_dbg(ndev, "Error warning interrupt\n");
priv->can.state = CAN_STATE_ERROR_WARNING;
priv->can.can_stats.error_warning++;
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
cf->data[6] = txerr;
......@@ -1062,7 +1062,7 @@ static void rcar_canfd_error(struct net_device *ndev, u32 cerfl,
netdev_dbg(ndev, "Error passive interrupt\n");
priv->can.state = CAN_STATE_ERROR_PASSIVE;
priv->can.can_stats.error_passive++;
cf->can_id |= CAN_ERR_CRTL;
cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_PASSIVE :
CAN_ERR_CRTL_RX_PASSIVE;
cf->data[6] = txerr;
......
drivers/net/can/sja1000/sja1000.c
View file @ 1dbd8748
......@@ -404,9 +404,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
txerr = priv->read_reg(priv, SJA1000_TXERR);
rxerr = priv->read_reg(priv, SJA1000_RXERR);
cf->data[6] = txerr;
cf->data[7] = rxerr;
if (isrc & IRQ_DOI) {
/* data overrun interrupt */
netdev_dbg(dev, "data overrun interrupt\n");
......@@ -428,6 +425,11 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
else
state = CAN_STATE_ERROR_ACTIVE;
}
if (state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
if (isrc & IRQ_BEI) {
/* bus error interrupt */
priv->can.can_stats.bus_error++;
......
drivers/net/can/slcan/slcan-core.c
View file @ 1dbd8748
......@@ -306,19 +306,18 @@ static void slc_bump_state(struct slcan *sl)
return;
skb = alloc_can_err_skb(dev, &cf);
if (skb) {
cf->data[6] = txerr;
cf->data[7] = rxerr;
} else {
cf = NULL;
}
tx_state = txerr >= rxerr ? state : 0;
rx_state = txerr <= rxerr ? state : 0;
can_change_state(dev, cf, tx_state, rx_state);
if (state == CAN_STATE_BUS_OFF)
if (state == CAN_STATE_BUS_OFF) {
can_bus_off(dev);
} else if (skb) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
if (skb)
netif_rx(skb);
......
drivers/net/can/spi/hi311x.c
View file @ 1dbd8748
......@@ -667,8 +667,6 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id)
txerr = hi3110_read(spi, HI3110_READ_TEC);
rxerr = hi3110_read(spi, HI3110_READ_REC);
cf->data[6] = txerr;
cf->data[7] = rxerr;
tx_state = txerr >= rxerr ? new_state : 0;
rx_state = txerr <= rxerr ? new_state : 0;
can_change_state(net, cf, tx_state, rx_state);
......@@ -681,6 +679,10 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id)
hi3110_hw_sleep(spi);
break;
}
} else {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
}
......
drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
View file @ 1dbd8748
......@@ -1099,6 +1099,7 @@ static int mcp251xfd_handle_cerrif(struct mcp251xfd_priv *priv)
err = mcp251xfd_get_berr_counter(priv->ndev, &bec);
if (err)
return err;
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = bec.txerr;
cf->data[7] = bec.rxerr;
}
......
drivers/net/can/sun4i_can.c
View file @ 1dbd8748
......@@ -535,11 +535,6 @@ static int sun4i_can_err(struct net_device *dev, u8 isrc, u8 status)
rxerr = (errc >> 16) & 0xFF;
txerr = errc & 0xFF;
if (skb) {
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
if (isrc & SUN4I_INT_DATA_OR) {
/* data overrun interrupt */
netdev_dbg(dev, "data overrun interrupt\n");
......@@ -570,6 +565,11 @@ static int sun4i_can_err(struct net_device *dev, u8 isrc, u8 status)
else
state = CAN_STATE_ERROR_ACTIVE;
}
if (skb && state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
if (isrc & SUN4I_INT_BUS_ERR) {
/* bus error interrupt */
netdev_dbg(dev, "bus error interrupt\n");
......
drivers/net/can/ti_hecc.c
View file @ 1dbd8748
......@@ -662,6 +662,7 @@ static void ti_hecc_change_state(struct net_device *ndev,
can_change_state(priv->ndev, cf, tx_state, rx_state);
if (max(tx_state, rx_state) != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = hecc_read(priv, HECC_CANTEC);
cf->data[7] = hecc_read(priv, HECC_CANREC);
}
......
drivers/net/can/usb/esd_usb.c
View file @ 1dbd8748
......@@ -265,7 +265,8 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
priv->can.can_stats.bus_error++;
stats->rx_errors++;
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR |
CAN_ERR_CNT;
switch (ecc & SJA1000_ECC_MASK) {
case SJA1000_ECC_BIT:
......
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
View file @ 1dbd8748
......@@ -917,8 +917,11 @@ static void kvaser_usb_hydra_update_state(struct kvaser_usb_net_priv *priv,
new_state < CAN_STATE_BUS_OFF)
priv->can.can_stats.restarts++;
cf->data[6] = bec->txerr;
cf->data[7] = bec->rxerr;
if (new_state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = bec->txerr;
cf->data[7] = bec->rxerr;
}
netif_rx(skb);
}
......@@ -1069,8 +1072,11 @@ kvaser_usb_hydra_error_frame(struct kvaser_usb_net_priv *priv,
shhwtstamps->hwtstamp = hwtstamp;
cf->can_id |= CAN_ERR_BUSERROR;
cf->data[6] = bec.txerr;
cf->data[7] = bec.rxerr;
if (new_state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = bec.txerr;
cf->data[7] = bec.rxerr;
}
netif_rx(skb);
......
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
View file @ 1dbd8748
......@@ -853,8 +853,11 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
break;
}
cf->data[6] = es->txerr;
cf->data[7] = es->rxerr;
if (new_state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = es->txerr;
cf->data[7] = es->rxerr;
}
netif_rx(skb);
}
......
drivers/net/can/usb/peak_usb/pcan_usb.c
View file @ 1dbd8748
......@@ -506,6 +506,7 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
/* Supply TX/RX error counters in case of
* controller error.
*/
cf->can_id = CAN_ERR_CNT;
cf->data[6] = mc->pdev->bec.txerr;
cf->data[7] = mc->pdev->bec.rxerr;
}
......
drivers/net/can/usb/usb_8dev.c
View file @ 1dbd8748
......@@ -438,9 +438,11 @@ static void usb_8dev_rx_err_msg(struct usb_8dev_priv *priv,
if (rx_errors)
stats->rx_errors++;
cf->data[6] = txerr;
cf->data[7] = rxerr;
if (priv->can.state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
priv->bec.txerr = txerr;
priv->bec.rxerr = rxerr;
......
drivers/net/can/xilinx_can.c
View file @ 1dbd8748
......@@ -965,6 +965,7 @@ static void xcan_set_error_state(struct net_device *ndev,
can_change_state(ndev, cf, tx_state, rx_state);
if (cf) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = txerr;
cf->data[7] = rxerr;
}
......
include/uapi/linux/can/error.h
View file @ 1dbd8748
......@@ -57,6 +57,8 @@
#define CAN_ERR_BUSOFF 0x00000040U /* bus off */
#define CAN_ERR_BUSERROR 0x00000080U /* bus error (may flood!) */
#define CAN_ERR_RESTARTED 0x00000100U /* controller restarted */
#define CAN_ERR_CNT 0x00000200U /* TX error counter / data[6] */
/* RX error counter / data[7] */
/* arbitration lost in bit ... / data[0] */
#define CAN_ERR_LOSTARB_UNSPEC 0x00 /* unspecified */
......@@ -120,6 +122,22 @@
#define CAN_ERR_TRX_CANL_SHORT_TO_GND 0x70 /* 0111 0000 */
#define CAN_ERR_TRX_CANL_SHORT_TO_CANH 0x80 /* 1000 0000 */
/* controller specific additional information / data[5..7] */
/* data[5] is reserved (do not use) */
/* TX error counter / data[6] */
/* RX error counter / data[7] */
/* CAN state thresholds
*
* Error counter Error state
* -----------------------------------
* 0 - 95 Error-active
* 96 - 127 Error-warning
* 128 - 255 Error-passive
* 256 and greater Bus-off
*/
#define CAN_ERROR_WARNING_THRESHOLD 96
#define CAN_ERROR_PASSIVE_THRESHOLD 128
#define CAN_BUS_OFF_THRESHOLD 256
#endif /* _UAPI_CAN_ERROR_H */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7