Commit a39dc2e2 authored by Marc Kleine-Budde's avatar Marc Kleine-Budde
Browse files

Merge patch series "Fix {rx,tx}_errors CAN statistics" 

Dario Binacchi <dario.binacchi@amarulasolutions.com> says:

This series extends the patch 4d6d2653 ("can: c_can: fix {rx,tx}_errors statistics"),
already merged into the mainline, to other CAN devices that similarly do
not correctly increment the error counters for reception/transmission.

Changes in v2:
- Fix patches 7 through 12 to ensure that statistics are updated even
  if the allocation of skb fails.
- Add five new patches (i. e. 1-5), created during the further analysis
  of the code while correcting patches from the v1 series (i. e. 7-12).

Link: https://patch.msgid.link/20241122221650.633981-1-dario.binacchi@amarulasolutions.com


[mkl: omitted patch 3]
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parents 889b2ae9 d7b91654

drivers/net/can/c_can/c_can_main.c

+17 −9
Original line number Diff line number Diff line
@@ -1014,42 +1014,47 @@ static int c_can_handle_bus_err(struct net_device *dev, 


	/* propagate the error condition to the CAN stack */

	skb = alloc_can_err_skb(dev, &cf);

	if (unlikely(!skb))

		return 0;



	/* check for 'last error code' which tells us the

	 * type of the last error to occur on the CAN bus

	 */

	if (likely(skb))

		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;



	switch (lec_type) {

	case LEC_STUFF_ERROR:

		netdev_dbg(dev, "stuff error\n");

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_STUFF;

		stats->rx_errors++;

		break;

	case LEC_FORM_ERROR:

		netdev_dbg(dev, "form error\n");

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_FORM;

		stats->rx_errors++;

		break;

	case LEC_ACK_ERROR:

		netdev_dbg(dev, "ack error\n");

		if (likely(skb))

			cf->data[3] = CAN_ERR_PROT_LOC_ACK;

		stats->tx_errors++;

		break;

	case LEC_BIT1_ERROR:

		netdev_dbg(dev, "bit1 error\n");

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_BIT1;

		stats->tx_errors++;

		break;

	case LEC_BIT0_ERROR:

		netdev_dbg(dev, "bit0 error\n");

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_BIT0;

		stats->tx_errors++;

		break;

	case LEC_CRC_ERROR:

		netdev_dbg(dev, "CRC error\n");

		if (likely(skb))

			cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;

		stats->rx_errors++;

		break;
@@ -1057,6 +1062,9 @@ static int c_can_handle_bus_err(struct net_device *dev, 
		break;

	}



	if (unlikely(!skb))

		return 0;



	netif_receive_skb(skb);

	return 1;

}

drivers/net/can/ifi_canfd/ifi_canfd.c

+40 −18
Original line number Diff line number Diff line
@@ -390,36 +390,55 @@ static int ifi_canfd_handle_lec_err(struct net_device *ndev) 
		return 0;



	priv->can.can_stats.bus_error++;

	stats->rx_errors++;



	/* Propagate the error condition to the CAN stack. */

	skb = alloc_can_err_skb(ndev, &cf);

	if (unlikely(!skb))

		return 0;



	/* Read the error counter register and check for new errors. */

	if (likely(skb))

		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;



	if (errctr & IFI_CANFD_ERROR_CTR_OVERLOAD_FIRST)

	if (errctr & IFI_CANFD_ERROR_CTR_OVERLOAD_FIRST) {

		stats->rx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_OVERLOAD;

	}



	if (errctr & IFI_CANFD_ERROR_CTR_ACK_ERROR_FIRST)

	if (errctr & IFI_CANFD_ERROR_CTR_ACK_ERROR_FIRST) {

		stats->tx_errors++;

		if (likely(skb))

			cf->data[3] = CAN_ERR_PROT_LOC_ACK;

	}



	if (errctr & IFI_CANFD_ERROR_CTR_BIT0_ERROR_FIRST)

	if (errctr & IFI_CANFD_ERROR_CTR_BIT0_ERROR_FIRST) {

		stats->tx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_BIT0;

	}



	if (errctr & IFI_CANFD_ERROR_CTR_BIT1_ERROR_FIRST)

	if (errctr & IFI_CANFD_ERROR_CTR_BIT1_ERROR_FIRST) {

		stats->tx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_BIT1;

	}



	if (errctr & IFI_CANFD_ERROR_CTR_STUFF_ERROR_FIRST)

	if (errctr & IFI_CANFD_ERROR_CTR_STUFF_ERROR_FIRST) {

		stats->rx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_STUFF;

	}



	if (errctr & IFI_CANFD_ERROR_CTR_CRC_ERROR_FIRST)

	if (errctr & IFI_CANFD_ERROR_CTR_CRC_ERROR_FIRST) {

		stats->rx_errors++;

		if (likely(skb))

			cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;

	}



	if (errctr & IFI_CANFD_ERROR_CTR_FORM_ERROR_FIRST)

	if (errctr & IFI_CANFD_ERROR_CTR_FORM_ERROR_FIRST) {

		stats->rx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_FORM;

	}



	/* Reset the error counter, ack the IRQ and re-enable the counter. */

	writel(IFI_CANFD_ERROR_CTR_ER_RESET, priv->base + IFI_CANFD_ERROR_CTR);
@@ -427,6 +446,9 @@ static int ifi_canfd_handle_lec_err(struct net_device *ndev) 
	       priv->base + IFI_CANFD_INTERRUPT);

	writel(IFI_CANFD_ERROR_CTR_ER_ENABLE, priv->base + IFI_CANFD_ERROR_CTR);



	if (unlikely(!skb))

		return 0;



	netif_receive_skb(skb);



	return 1;

drivers/net/can/m_can/m_can.c

+23 −10
Original line number Diff line number Diff line
@@ -695,47 +695,60 @@ static int m_can_handle_lec_err(struct net_device *dev, 
	u32 timestamp = 0;



	cdev->can.can_stats.bus_error++;

	stats->rx_errors++;



	/* propagate the error condition to the CAN stack */

	skb = alloc_can_err_skb(dev, &cf);

	if (unlikely(!skb))

		return 0;



	/* check for 'last error code' which tells us the

	 * type of the last error to occur on the CAN bus

	 */

	if (likely(skb))

		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;



	switch (lec_type) {

	case LEC_STUFF_ERROR:

		netdev_dbg(dev, "stuff error\n");

		stats->rx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_STUFF;

		break;

	case LEC_FORM_ERROR:

		netdev_dbg(dev, "form error\n");

		stats->rx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_FORM;

		break;

	case LEC_ACK_ERROR:

		netdev_dbg(dev, "ack error\n");

		stats->tx_errors++;

		if (likely(skb))

			cf->data[3] = CAN_ERR_PROT_LOC_ACK;

		break;

	case LEC_BIT1_ERROR:

		netdev_dbg(dev, "bit1 error\n");

		stats->tx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_BIT1;

		break;

	case LEC_BIT0_ERROR:

		netdev_dbg(dev, "bit0 error\n");

		stats->tx_errors++;

		if (likely(skb))

			cf->data[2] |= CAN_ERR_PROT_BIT0;

		break;

	case LEC_CRC_ERROR:

		netdev_dbg(dev, "CRC error\n");

		stats->rx_errors++;

		if (likely(skb))

			cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;

		break;

	default:

		break;

	}



	if (unlikely(!skb))

		return 0;



	if (cdev->is_peripheral)

		timestamp = m_can_get_timestamp(cdev);

drivers/net/can/sja1000/sja1000.c

+39 −28
Original line number Diff line number Diff line
@@ -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");

		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,10 +461,9 @@ 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;



			/* set error type */
@@ -483,10 +483,16 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) 


			/* set error location */

			cf->data[3] = ecc & ECC_SEG;

		}



		/* Error occurred during transmission? */

		if ((ecc & ECC_DIR) == 0)

		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,9 +508,11 @@ 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++;

		if (skb) {

			cf->can_id |= CAN_ERR_LOSTARB;

			cf->data[0] = alc & 0x1f;

		}

	}



	if (state != priv->can.state) {

		tx_state = txerr >= rxerr ? state : 0;
@@ -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;

drivers/net/can/spi/hi311x.c

+33 −22
Original line number Diff line number Diff line
@@ -663,27 +663,27 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id) 
			u8 rxerr, txerr;



			skb = alloc_can_err_skb(net, &cf);

			if (!skb)

				break;



			txerr = hi3110_read(spi, HI3110_READ_TEC);

			rxerr = hi3110_read(spi, HI3110_READ_REC);

			tx_state = txerr >= rxerr ? new_state : 0;

			rx_state = txerr <= rxerr ? new_state : 0;

			can_change_state(net, cf, tx_state, rx_state);

			netif_rx(skb);



			if (new_state == CAN_STATE_BUS_OFF) {

				if (skb)

					netif_rx(skb);

				can_bus_off(net);

				if (priv->can.restart_ms == 0) {

					priv->force_quit = 1;

					hi3110_hw_sleep(spi);

					break;

				}

			} else {

			} else if (skb) {

				cf->can_id |= CAN_ERR_CNT;

				cf->data[6] = txerr;

				cf->data[7] = rxerr;

				netif_rx(skb);

			}

		}
@@ -696,29 +696,40 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id) 
			/* Check for protocol errors */

			if (eflag & HI3110_ERR_PROTOCOL_MASK) {

				skb = alloc_can_err_skb(net, &cf);

				if (!skb)

					break;



				if (skb)

					cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;



				priv->can.can_stats.bus_error++;

				priv->net->stats.rx_errors++;

				if (eflag & HI3110_ERR_BITERR)

				if (eflag & HI3110_ERR_BITERR) {

					priv->net->stats.tx_errors++;

					if (skb)

						cf->data[2] |= CAN_ERR_PROT_BIT;

				else if (eflag & HI3110_ERR_FRMERR)

				} else if (eflag & HI3110_ERR_FRMERR) {

					priv->net->stats.rx_errors++;

					if (skb)

						cf->data[2] |= CAN_ERR_PROT_FORM;

				else if (eflag & HI3110_ERR_STUFERR)

				} else if (eflag & HI3110_ERR_STUFERR) {

					priv->net->stats.rx_errors++;

					if (skb)

						cf->data[2] |= CAN_ERR_PROT_STUFF;

				else if (eflag & HI3110_ERR_CRCERR)

				} else if (eflag & HI3110_ERR_CRCERR) {

					priv->net->stats.rx_errors++;

					if (skb)

						cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;

				else if (eflag & HI3110_ERR_ACKERR)

				} else if (eflag & HI3110_ERR_ACKERR) {

					priv->net->stats.tx_errors++;

					if (skb)

						cf->data[3] |= CAN_ERR_PROT_LOC_ACK;

				}



				netdev_dbg(priv->net, "Bus Error\n");

				if (skb) {

					cf->data[6] = hi3110_read(spi, HI3110_READ_TEC);

					cf->data[7] = hi3110_read(spi, HI3110_READ_REC);

				netdev_dbg(priv->net, "Bus Error\n");

					netif_rx(skb);

				}

			}

		}



		if (priv->tx_busy && statf & HI3110_STAT_TXMTY) {

			net->stats.tx_packets++;