drivers: can: mcan: Add CAN statistics

Add CAN stats for MCAN drivers. Update MCAN drivers to use CAN_DEVICE_DT_INST_DEFINE which initialises and registers CAN stats if enabled. Signed-off-by: Grant Ramsay <gramsay@enphaseenergy.com>
zephyrproject-rtos · Sep 19, 2023 · 8856dbd · 8856dbd
1 parent a4de2eb
commit 8856dbd
Show file tree

Hide file tree

Showing 8 changed files with 117 additions and 34 deletions.
diff --git a/drivers/can/can_mcan.c b/drivers/can/can_mcan.c
@@ -524,11 +524,67 @@ static void can_mcan_tx_event_handler(const struct device *dev)
 	}
 }
 
+#ifdef CONFIG_CAN_STATS
+static void can_mcan_lec_update_stats(const struct device *dev, enum can_mcan_psr_lec lec)
+{
+	switch (lec) {
+	case CAN_MCAN_PSR_LEC_STUFF_ERROR:
+		CAN_STATS_STUFF_ERROR_INC(dev);
+		break;
+	case CAN_MCAN_PSR_LEC_FORM_ERROR:
+		CAN_STATS_FORM_ERROR_INC(dev);
+		break;
+	case CAN_MCAN_PSR_LEC_ACK_ERROR:
+		CAN_STATS_ACK_ERROR_INC(dev);
+		break;
+	case CAN_MCAN_PSR_LEC_BIT1_ERROR:
+		CAN_STATS_BIT1_ERROR_INC(dev);
+		break;
+	case CAN_MCAN_PSR_LEC_BIT0_ERROR:
+		CAN_STATS_BIT0_ERROR_INC(dev);
+		break;
+	case CAN_MCAN_PSR_LEC_CRC_ERROR:
+		CAN_STATS_CRC_ERROR_INC(dev);
+		break;
+	case CAN_MCAN_PSR_LEC_NO_ERROR:
+	case CAN_MCAN_PSR_LEC_NO_CHANGE:
+	default:
+		break;
+	}
+}
+#endif
+
+int can_mcan_read_psr(const struct device *dev, uint32_t *val)
+{
+	/* Reading the lower byte of the PSR register clears the protocol last
+	 * error codes (LEC). To avoid missing errors, this function should be
+	 * used whenever the PSR register is read.
+	 */
+	int err = can_mcan_read_reg(dev, CAN_MCAN_PSR, val);
+
+	if (err != 0) {
+		return err;
+	}
+
+#ifdef CONFIG_CAN_STATS
+	enum can_mcan_psr_lec lec;
+
+	lec = FIELD_GET(CAN_MCAN_PSR_LEC, *val);
+	can_mcan_lec_update_stats(dev, lec);
+#ifdef CONFIG_CAN_FD_MODE
+	lec = FIELD_GET(CAN_MCAN_PSR_DLEC, *val);
+	can_mcan_lec_update_stats(dev, lec);
+#endif
+#endif
+
+	return 0;
+}
+
 void can_mcan_line_0_isr(const struct device *dev)
 {
 	const uint32_t events = CAN_MCAN_IR_BO | CAN_MCAN_IR_EP | CAN_MCAN_IR_EW |
 				CAN_MCAN_IR_TEFN | CAN_MCAN_IR_TEFL | CAN_MCAN_IR_ARA |
-				CAN_MCAN_IR_MRAF;
+				CAN_MCAN_IR_MRAF | CAN_MCAN_IR_PEA | CAN_MCAN_IR_PED;
 	struct can_mcan_data *data = dev->data;
 	uint32_t ir;
 	int err;
@@ -562,10 +618,18 @@ void can_mcan_line_0_isr(const struct device *dev)
 			LOG_ERR("Access to reserved address");
 		}
 
-		if (ir & CAN_MCAN_IR_MRAF) {
+		if ((ir & CAN_MCAN_IR_MRAF) != 0U) {
 			LOG_ERR("Message RAM access failure");
 		}
 
+#ifdef CONFIG_CAN_STATS
+		if ((ir & (CAN_MCAN_IR_PEA | CAN_MCAN_IR_PED)) != 0U) {
+			uint32_t reg;
+			/* This function automatically updates protocol error stats */
+			can_mcan_read_psr(dev, &reg);
+		}
+#endif
+
 		err = can_mcan_read_reg(dev, CAN_MCAN_IR, &ir);
 		if (err != 0) {
 			return;
@@ -739,10 +803,12 @@ void can_mcan_line_1_isr(const struct device *dev)
 
 		if ((ir & CAN_MCAN_IR_RF0L) != 0U) {
 			LOG_ERR("Message lost on FIFO0");
+			CAN_STATS_RX_OVERRUN_INC(dev);
 		}
 
 		if ((ir & CAN_MCAN_IR_RF1L) != 0U) {
 			LOG_ERR("Message lost on FIFO1");
+			CAN_STATS_RX_OVERRUN_INC(dev);
 		}
 
 		err = can_mcan_read_reg(dev, CAN_MCAN_IR, &ir);
@@ -760,7 +826,7 @@ int can_mcan_get_state(const struct device *dev, enum can_state *state,
 	int err;
 
 	if (state != NULL) {
-		err = can_mcan_read_reg(dev, CAN_MCAN_PSR, &reg);
+		err = can_mcan_read_psr(dev, &reg);
 		if (err != 0) {
 			return err;
 		}
@@ -877,7 +943,7 @@ int can_mcan_send(const struct device *dev, const struct can_frame *frame, k_tim
 		return -ENETDOWN;
 	}
 
-	err = can_mcan_read_reg(dev, CAN_MCAN_PSR, &reg);
+	err = can_mcan_read_psr(dev, &reg);
 	if (err != 0) {
 		return err;
 	}
@@ -1456,6 +1522,12 @@ int can_mcan_init(const struct device *dev)
 	reg = CAN_MCAN_IE_BOE | CAN_MCAN_IE_EWE | CAN_MCAN_IE_EPE | CAN_MCAN_IE_MRAFE |
 	      CAN_MCAN_IE_TEFLE | CAN_MCAN_IE_TEFNE | CAN_MCAN_IE_RF0NE | CAN_MCAN_IE_RF1NE |
 	      CAN_MCAN_IE_RF0LE | CAN_MCAN_IE_RF1LE;
+#ifdef CONFIG_CAN_STATS
+	/* These ISRs are only enabled/used for statistics, they are otherwise
+	 * disabled as they may produce a significant amount of frequent ISRs.
+	 */
+	reg |= CAN_MCAN_IE_PEAE | CAN_MCAN_IE_PEDE;
+#endif
 
 	err = can_mcan_write_reg(dev, CAN_MCAN_IE, reg);
 	if (err != 0) {

diff --git a/drivers/can/can_mcux_mcan.c b/drivers/can/can_mcux_mcan.c
@@ -208,12 +208,12 @@ static const struct can_mcan_ops mcux_mcan_ops = {
 	static struct can_mcan_data can_mcan_data_##n =			\
 		CAN_MCAN_DATA_INITIALIZER(NULL);			\
 									\
-	DEVICE_DT_INST_DEFINE(n, &mcux_mcan_init, NULL,			\
-			      &can_mcan_data_##n,			\
-			      &can_mcan_config_##n,			\
-			      POST_KERNEL,				\
-			      CONFIG_CAN_INIT_PRIORITY,			\
-			      &mcux_mcan_driver_api);			\
+	CAN_DEVICE_DT_INST_DEFINE(n, mcux_mcan_init, NULL,		\
+				  &can_mcan_data_##n,			\
+				  &can_mcan_config_##n,			\
+				  POST_KERNEL,				\
+				  CONFIG_CAN_INIT_PRIORITY,		\
+				  &mcux_mcan_driver_api);		\
 									\
 	static void mcux_mcan_irq_config_##n(const struct device *dev)	\
 	{								\

diff --git a/drivers/can/can_sam.c b/drivers/can/can_sam.c
@@ -186,12 +186,12 @@ static void config_can_##inst##_irq(void)
 	static struct can_mcan_data can_mcan_data_##inst =		\
 		CAN_MCAN_DATA_INITIALIZER(NULL);
 
-#define CAN_SAM_DEVICE_INST(inst)					\
-	DEVICE_DT_INST_DEFINE(inst, &can_sam_init, NULL,		\
-			      &can_mcan_data_##inst,			\
-			      &can_mcan_cfg_##inst,			\
-			      POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,	\
-			      &can_sam_driver_api);
+#define CAN_SAM_DEVICE_INST(inst)						\
+	CAN_DEVICE_DT_INST_DEFINE(inst, can_sam_init, NULL,			\
+				  &can_mcan_data_##inst,			\
+				  &can_mcan_cfg_##inst,				\
+				  POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,	\
+				  &can_sam_driver_api);
 
 #define CAN_SAM_INST(inst)                                                     \
 	CAN_MCAN_DT_INST_BUILD_ASSERT_MRAM_CFG(inst);                          \

diff --git a/drivers/can/can_sam0.c b/drivers/can/can_sam0.c
@@ -214,11 +214,11 @@ static void config_can_##inst##_irq(void)						\
 		CAN_MCAN_DATA_INITIALIZER(NULL);
 
 #define CAN_SAM0_DEVICE_INST(inst)							\
-	DEVICE_DT_INST_DEFINE(inst, &can_sam0_init, NULL,				\
-			      &can_mcan_data_##inst,					\
-			      &can_mcan_cfg_##inst,					\
-			      POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,			\
-			      &can_sam0_driver_api);
+	CAN_DEVICE_DT_INST_DEFINE(inst, can_sam0_init, NULL,				\
+				  &can_mcan_data_##inst,				\
+				  &can_mcan_cfg_##inst,					\
+				  POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,		\
+				  &can_sam0_driver_api);
 
 #define CAN_SAM0_INST(inst)								\
 	CAN_MCAN_DT_INST_BUILD_ASSERT_MRAM_CFG(inst);					\

diff --git a/drivers/can/can_stm32_fdcan.c b/drivers/can/can_stm32_fdcan.c
@@ -673,11 +673,11 @@ static void config_can_##inst##_irq(void)                                      \
 	static struct can_mcan_data can_mcan_data_##inst =		\
 		CAN_MCAN_DATA_INITIALIZER(NULL);
 
-#define CAN_STM32FD_DEVICE_INST(inst)					\
-	DEVICE_DT_INST_DEFINE(inst, &can_stm32fd_init, NULL,		\
-			      &can_mcan_data_##inst, &can_mcan_cfg_##inst, \
-			      POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,	\
-			      &can_stm32fd_driver_api);
+#define CAN_STM32FD_DEVICE_INST(inst)						\
+	CAN_DEVICE_DT_INST_DEFINE(inst, can_stm32fd_init, NULL,			\
+				  &can_mcan_data_##inst, &can_mcan_cfg_##inst,	\
+				  POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,	\
+				  &can_stm32fd_driver_api);
 
 #define CAN_STM32FD_INST(inst)          \
 CAN_STM32FD_BUILD_ASSERT_MRAM_CFG(inst) \

diff --git a/drivers/can/can_stm32h7_fdcan.c b/drivers/can/can_stm32h7_fdcan.c
@@ -224,11 +224,11 @@ static const struct can_mcan_ops can_stm32h7_ops = {
 	static struct can_mcan_data can_mcan_data_##n =			    \
 		CAN_MCAN_DATA_INITIALIZER(NULL);			    \
 									    \
-	DEVICE_DT_INST_DEFINE(n, &can_stm32h7_init, NULL,		    \
-			      &can_mcan_data_##n,			    \
-			      &can_mcan_cfg_##n,			    \
-			      POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,	    \
-			      &can_stm32h7_driver_api);			    \
+	CAN_DEVICE_DT_INST_DEFINE(n, can_stm32h7_init, NULL,		    \
+				  &can_mcan_data_##n,			    \
+				  &can_mcan_cfg_##n,			    \
+				  POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,    \
+				  &can_stm32h7_driver_api);		    \
 									    \
 	static void stm32h7_mcan_irq_config_##n(void)			    \
 	{								    \

diff --git a/drivers/can/can_tcan4x5x.c b/drivers/can/can_tcan4x5x.c
@@ -789,8 +789,8 @@ static const struct can_mcan_ops tcan4x5x_ops = {
 	static struct can_mcan_data can_mcan_data_##inst =                                         \
 		CAN_MCAN_DATA_INITIALIZER(&tcan4x5x_data_##inst);                                  \
                                                                                                    \
-	DEVICE_DT_INST_DEFINE(inst, &tcan4x5x_init, NULL, &can_mcan_data_##inst,                   \
-			      &can_mcan_config_##inst, POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,      \
-			      &tcan4x5x_driver_api);
+	CAN_DEVICE_DT_INST_DEFINE(inst, tcan4x5x_init, NULL, &can_mcan_data_##inst,                \
+				  &can_mcan_config_##inst, POST_KERNEL, CONFIG_CAN_INIT_PRIORITY,  \
+				  &tcan4x5x_driver_api);
 
 DT_INST_FOREACH_STATUS_OKAY(TCAN4X5X_INIT)
diff --git a/include/zephyr/drivers/can/can_mcan.h b/include/zephyr/drivers/can/can_mcan.h
@@ -127,6 +127,17 @@
 #define CAN_MCAN_PSR_ACT  GENMASK(4, 3)
 #define CAN_MCAN_PSR_LEC  GENMASK(2, 0)
 
+enum can_mcan_psr_lec {
+	CAN_MCAN_PSR_LEC_NO_ERROR    = 0,
+	CAN_MCAN_PSR_LEC_STUFF_ERROR = 1,
+	CAN_MCAN_PSR_LEC_FORM_ERROR  = 2,
+	CAN_MCAN_PSR_LEC_ACK_ERROR   = 3,
+	CAN_MCAN_PSR_LEC_BIT1_ERROR  = 4,
+	CAN_MCAN_PSR_LEC_BIT0_ERROR  = 5,
+	CAN_MCAN_PSR_LEC_CRC_ERROR   = 6,
+	CAN_MCAN_PSR_LEC_NO_CHANGE   = 7
+};
+
 /* Transmitter Delay Compensation register */
 #define CAN_MCAN_TDCR      0x048
 #define CAN_MCAN_TDCR_TDCO GENMASK(14, 8)