can: sja1000: sja1000_err(): fix {rx,tx}_errors statistics

The sja1000_err() function only incremented the receive error counter
and never the transmit error counter, even if the ECC_DIR flag reported
that an error had occurred during transmission.

Increment the receive/transmit error counter based on the value of the
ECC_DIR flag.

Fixes: 429da1cc841b ("can: Driver for the SJA1000 CAN controller")
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Link: https://patch.msgid.link/20241122221650.633981-10-dario.binacchi@amarulasolutions.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index ddb3247948ad2fba657741d7e442c571311506f7..4d245857ef1cec0ae1ee82008b96912bbb1f2638 100644 (file)
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -416,8 +416,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
        int ret = 0;
 
        skb = alloc_can_err_skb(dev, &cf);
-       if (skb == NULL)
-               return -ENOMEM;
 
        txerr = priv->read_reg(priv, SJA1000_TXERR);
        rxerr = priv->read_reg(priv, SJA1000_RXERR);
@@ -425,8 +423,11 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
        if (isrc & IRQ_DOI) {
                /* data overrun interrupt */
                netdev_dbg(dev, "data overrun interrupt\n");
-               cf->can_id |= CAN_ERR_CRTL;
-               cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+               if (skb) {
+                       cf->can_id |= CAN_ERR_CRTL;
+                       cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+               }
+
                stats->rx_over_errors++;
                stats->rx_errors++;
                sja1000_write_cmdreg(priv, CMD_CDO);    /* clear bit */
@@ -452,7 +453,7 @@ 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) {
+       if (state != CAN_STATE_BUS_OFF && skb) {
                cf->can_id |= CAN_ERR_CNT;
                cf->data[6] = txerr;
                cf->data[7] = rxerr;
@@ -460,33 +461,38 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
        if (isrc & IRQ_BEI) {
                /* bus error interrupt */
                priv->can.can_stats.bus_error++;
-               stats->rx_errors++;
 
                ecc = priv->read_reg(priv, SJA1000_ECC);
+               if (skb) {
+                       cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
 
-               cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
-
-               /* set error type */
-               switch (ecc & ECC_MASK) {
-               case ECC_BIT:
-                       cf->data[2] |= CAN_ERR_PROT_BIT;
-                       break;
-               case ECC_FORM:
-                       cf->data[2] |= CAN_ERR_PROT_FORM;
-                       break;
-               case ECC_STUFF:
-                       cf->data[2] |= CAN_ERR_PROT_STUFF;
-                       break;
-               default:
-                       break;
-               }
+                       /* set error type */
+                       switch (ecc & ECC_MASK) {
+                       case ECC_BIT:
+                               cf->data[2] |= CAN_ERR_PROT_BIT;
+                               break;
+                       case ECC_FORM:
+                               cf->data[2] |= CAN_ERR_PROT_FORM;
+                               break;
+                       case ECC_STUFF:
+                               cf->data[2] |= CAN_ERR_PROT_STUFF;
+                               break;
+                       default:
+                               break;
+                       }
 
-               /* set error location */
-               cf->data[3] = ecc & ECC_SEG;
+                       /* set error location */
+                       cf->data[3] = ecc & ECC_SEG;
+               }
 
                /* Error occurred during transmission? */
-               if ((ecc & ECC_DIR) == 0)
-                       cf->data[2] |= CAN_ERR_PROT_TX;
+               if ((ecc & ECC_DIR) == 0) {
+                       stats->tx_errors++;
+                       if (skb)
+                               cf->data[2] |= CAN_ERR_PROT_TX;
+               } else {
+                       stats->rx_errors++;
+               }
        }
        if (isrc & IRQ_EPI) {
                /* error passive interrupt */
@@ -502,8 +508,10 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
                netdev_dbg(dev, "arbitration lost interrupt\n");
                alc = priv->read_reg(priv, SJA1000_ALC);
                priv->can.can_stats.arbitration_lost++;
-               cf->can_id |= CAN_ERR_LOSTARB;
-               cf->data[0] = alc & 0x1f;
+               if (skb) {
+                       cf->can_id |= CAN_ERR_LOSTARB;
+                       cf->data[0] = alc & 0x1f;
+               }
        }
 
        if (state != priv->can.state) {
@@ -516,6 +524,9 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
                        can_bus_off(dev);
        }
 
+       if (!skb)
+               return -ENOMEM;
+
        netif_rx(skb);
 
        return ret;