]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
can: ems_usb: ems_usb_rx_err(): fix {rx,tx}_errors statistics
authorDario Binacchi <dario.binacchi@amarulasolutions.com>
Fri, 22 Nov 2024 22:15:52 +0000 (23:15 +0100)
committerMarc Kleine-Budde <mkl@pengutronix.de>
Tue, 26 Nov 2024 09:51:06 +0000 (10:51 +0100)
The ems_usb_rx_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: 702171adeed3 ("ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface")
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Link: https://patch.msgid.link/20241122221650.633981-12-dario.binacchi@amarulasolutions.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
drivers/net/can/usb/ems_usb.c

index 050c0b49938a42a1cd84024ab98ad33cbe8bccca..5355bac4dccbe0e31ba115909b0537a9b92c2fa6 100644 (file)
@@ -335,15 +335,14 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg)
        struct net_device_stats *stats = &dev->netdev->stats;
 
        skb = alloc_can_err_skb(dev->netdev, &cf);
-       if (skb == NULL)
-               return;
 
        if (msg->type == CPC_MSG_TYPE_CAN_STATE) {
                u8 state = msg->msg.can_state;
 
                if (state & SJA1000_SR_BS) {
                        dev->can.state = CAN_STATE_BUS_OFF;
-                       cf->can_id |= CAN_ERR_BUSOFF;
+                       if (skb)
+                               cf->can_id |= CAN_ERR_BUSOFF;
 
                        dev->can.can_stats.bus_off++;
                        can_bus_off(dev->netdev);
@@ -361,44 +360,53 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg)
 
                /* bus error interrupt */
                dev->can.can_stats.bus_error++;
-               stats->rx_errors++;
 
-               cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+               if (skb) {
+                       cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
 
-               switch (ecc & SJA1000_ECC_MASK) {
-               case SJA1000_ECC_BIT:
-                       cf->data[2] |= CAN_ERR_PROT_BIT;
-                       break;
-               case SJA1000_ECC_FORM:
-                       cf->data[2] |= CAN_ERR_PROT_FORM;
-                       break;
-               case SJA1000_ECC_STUFF:
-                       cf->data[2] |= CAN_ERR_PROT_STUFF;
-                       break;
-               default:
-                       cf->data[3] = ecc & SJA1000_ECC_SEG;
-                       break;
+                       switch (ecc & SJA1000_ECC_MASK) {
+                       case SJA1000_ECC_BIT:
+                               cf->data[2] |= CAN_ERR_PROT_BIT;
+                               break;
+                       case SJA1000_ECC_FORM:
+                               cf->data[2] |= CAN_ERR_PROT_FORM;
+                               break;
+                       case SJA1000_ECC_STUFF:
+                               cf->data[2] |= CAN_ERR_PROT_STUFF;
+                               break;
+                       default:
+                               cf->data[3] = ecc & SJA1000_ECC_SEG;
+                               break;
+                       }
                }
 
                /* Error occurred during transmission? */
-               if ((ecc & SJA1000_ECC_DIR) == 0)
-                       cf->data[2] |= CAN_ERR_PROT_TX;
+               if ((ecc & SJA1000_ECC_DIR) == 0) {
+                       stats->tx_errors++;
+                       if (skb)
+                               cf->data[2] |= CAN_ERR_PROT_TX;
+               } else {
+                       stats->rx_errors++;
+               }
 
-               if (dev->can.state == CAN_STATE_ERROR_WARNING ||
-                   dev->can.state == CAN_STATE_ERROR_PASSIVE) {
+               if (skb && (dev->can.state == CAN_STATE_ERROR_WARNING ||
+                           dev->can.state == CAN_STATE_ERROR_PASSIVE)) {
                        cf->can_id |= CAN_ERR_CRTL;
                        cf->data[1] = (txerr > rxerr) ?
                            CAN_ERR_CRTL_TX_PASSIVE : CAN_ERR_CRTL_RX_PASSIVE;
                }
        } else if (msg->type == CPC_MSG_TYPE_OVERRUN) {
-               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++;
        }
 
-       netif_rx(skb);
+       if (skb)
+               netif_rx(skb);
 }
 
 /*