Merge pull request #312 from marckleinebudde/cleanup-CAN-bus-error-handling

cleanup CAN bus error handling

Author: marckleinebudde

src/can/bxcan.c

@@ -312,27 +312,31 @@ bool can_send(can_data_t *channel, struct gs_host_frame *frame)
 	}
 }
 
-uint32_t can_drv_read_reg_status(const struct can_channel *channel)
-{
-	return channel->instance->ESR;
-}
-
 bool can_drv_bus_error_pending(const uint32_t reg_esr)
 {
 	const uint32_t lec = FIELD_GET(CAN_ESR_LEC, reg_esr);
 
 	return can_is_lec_error(lec);
 }
 
+uint32_t can_drv_read_reg_status(const struct can_channel *channel)
+{
+	const uint32_t reg_esr = channel->instance->ESR;
+
+	if (can_drv_bus_error_pending(reg_esr)) {
+		/* mark as handled by software */
+		channel->instance->ESR |= FIELD_PREP(CAN_ESR_LEC, CAN_LEC_SOFTWARE);
+	}
+
+	return reg_esr;
+}
+
 bool can_drv_handle_bus_error(const struct can_channel __maybe_unused *channel, struct gs_host_frame *frame,
 							  const uint32_t reg_esr)
 {
 	const uint8_t tx_err = FIELD_GET(CAN_ESR_TEC, reg_esr);
 	const uint8_t rx_err = FIELD_GET(CAN_ESR_REC, reg_esr);
 
-	/* mark as handled by software */
-	channel->instance->ESR |= FIELD_PREP(CAN_ESR_LEC, CAN_LEC_SOFTWARE);
-
 	if (tx_err == 0 && rx_err == 0)
 		return false;
 
@@ -371,7 +375,7 @@ void can_drv_get_device_state(const struct can_channel __maybe_unused *channel,
 	state->txerr = FIELD_GET(CAN_ESR_TEC, reg_esr);
 }
 
-void can_drv_handle_state_change(const struct can_channel *channel, struct gs_host_frame *frame,
+void can_drv_handle_state_change(const struct can_channel __maybe_unused *channel, struct gs_host_frame *frame,
 								 const uint32_t reg_esr)
 {
 	enum gs_can_state tx_state, rx_state;
@@ -387,9 +391,4 @@ void can_drv_handle_state_change(const struct can_channel *channel, struct gs_ho
 		frame->classic_can->data[1] |= gs_can_tx_state_to_frame(tx_state);
 	if (tx_state <= rx_state)
 		frame->classic_can->data[1] |= gs_can_rx_state_to_frame(rx_state);
-
-	frame->classic_can->data[6] = tx_err;
-	frame->classic_can->data[7] = rx_err;
-
-	can_drv_handle_bus_error(channel, frame, reg_esr);
 }

src/can_common.c

@@ -255,6 +255,15 @@ static void can_handle_bus_error(USBD_GS_CAN_HandleTypeDef *hcan, const struct c
 	}
 }
 
+static bool can_bus_error_pending(const struct can_channel *channel, const uint32_t reg_status)
+{
+	if (!(channel->feature & GS_CAN_FEATURE_BERR_REPORTING)) {
+		return false;
+	}
+
+	return can_drv_bus_error_pending(reg_status);
+}
+
 static void can_handle_state_change(USBD_GS_CAN_HandleTypeDef *hcan, struct can_channel *channel,
 									const uint32_t reg_status)
 {
@@ -270,20 +279,12 @@ static void can_handle_state_change(USBD_GS_CAN_HandleTypeDef *hcan, struct can_
 	} else {
 		frame->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
 		can_drv_handle_state_change(channel, frame, reg_status);
+		can_drv_handle_bus_error(channel, frame, reg_status);
 	}
 
 	list_add_tail_locked(&frame_object->list, &hcan->list_to_host);
 }
 
-static bool can_bus_error_pending(const struct can_channel *channel, const uint32_t reg_status)
-{
-	if (!(channel->feature & GS_CAN_FEATURE_BERR_REPORTING)) {
-		return false;
-	}
-
-	return can_drv_bus_error_pending(reg_status);
-}
-
 static bool can_state_change_pending(struct can_channel *channel, const uint32_t reg_status)
 {
 	if (channel->state >= GS_CAN_STATE_STOPPED)
@@ -310,11 +311,9 @@ void CAN_HandleError(USBD_GS_CAN_HandleTypeDef *hcan, can_data_t *channel)
 
 	const uint32_t reg_status = can_drv_read_reg_status(channel);
 
-	if (can_bus_error_pending(channel, reg_status)) {
-		can_handle_bus_error(hcan, channel, reg_status);
-	}
-
 	if (can_state_change_pending(channel, reg_status)) {
 		can_handle_state_change(hcan, channel, reg_status);
+	} else if (can_bus_error_pending(channel, reg_status)) {
+		can_handle_bus_error(hcan, channel, reg_status);
 	}
 }