drivers: can: sjw == 0 in can_set_timing should not change sjw

If the supplied sjw in the timing parameters is zero,
the sjw parameter should not be changed.
This fixes the uninitialized swj in can_set_bitrate.

Signed-off-by: Alexander Wachter <[email protected]>

Author: alexanderwachter

drivers/can/can_mcan.c

@@ -79,6 +79,8 @@ void can_mcan_configure_timing(struct can_mcan_reg  *can,
 			       const struct can_timing *timing_data)
 {
 	if (timing) {
+		uint32_t nbtp_sjw = can->nbtp & CAN_MCAN_NBTP_NSJW_MSK;
+
 		__ASSERT_NO_MSG(timing->prop_seg == 0);
 		__ASSERT_NO_MSG(timing->phase_seg1 <= 0x100 &&
 				timing->phase_seg1 > 0);
@@ -92,14 +94,21 @@ void can_mcan_configure_timing(struct can_mcan_reg  *can,
 				CAN_MCAN_NBTP_NTSEG1_POS |
 			    (((uint32_t)timing->phase_seg2 - 1UL) & 0x7F) <<
 				CAN_MCAN_NBTP_NTSEG2_POS |
-			    (((uint32_t)timing->sjw        - 1UL) & 0x7F) <<
-				CAN_MCAN_NBTP_NSJW_POS   |
 			    (((uint32_t)timing->prescaler  - 1UL) & 0x1FF) <<
 				CAN_MCAN_NBTP_NBRP_POS;
+
+		if (timing->sjw == CAN_SJW_NO_CHANGE) {
+			can->nbtp |= nbtp_sjw;
+		} else {
+			can->nbtp |= (((uint32_t)timing->sjw - 1UL) & 0x7F) <<
+				     CAN_MCAN_NBTP_NSJW_POS;
+		}
 	}
 
 #ifdef CONFIG_CAN_FD_MODE
 	if (timing_data) {
+		uint32_t dbtp_sjw = can->dbtp & CAN_MCAN_DBTP_DSJW_MSK;
+
 		__ASSERT_NO_MSG(timing_data->prop_seg == 0);
 		__ASSERT_NO_MSG(timing_data->phase_seg1 <= 0x20 &&
 				timing_data->phase_seg1 > 0);
@@ -112,12 +121,17 @@ void can_mcan_configure_timing(struct can_mcan_reg  *can,
 
 		can->dbtp = (((uint32_t)timing_data->phase_seg1 - 1UL) & 0x1F) <<
 				CAN_MCAN_DBTP_DTSEG1_POS |
-			    (((uint32_t)timing_data->phase_seg2 - 1UL) & 0x0F)  <<
+			    (((uint32_t)timing_data->phase_seg2 - 1UL) & 0x0F) <<
 				CAN_MCAN_DBTP_DTSEG2_POS |
-			    (((uint32_t)timing_data->sjw        - 1UL) & 0x0F)  <<
-				CAN_MCAN_DBTP_DSJW_POS   |
 			    (((uint32_t)timing_data->prescaler  - 1UL) & 0x1F) <<
 				CAN_MCAN_DBTP_DBRP_POS;
+
+		if (timing_data->sjw == CAN_SJW_NO_CHANGE) {
+			can->nbtp |= dbtp_sjw;
+		} else {
+			can->nbtp |= (((uint32_t)timing_data->sjw - 1UL) & 0x0F) <<
+				     CAN_MCAN_DBTP_DSJW_POS;
+		}
 	}
 #endif
 }

drivers/can/can_mcp2515.c

@@ -335,8 +335,11 @@ static int mcp2515_set_timing(const struct device *dev,
 	/* CNF1; SJW<7:6> | BRP<5:0> */
 	__ASSERT(timing->prescaler > 0, "Prescaler should be bigger than zero");
 	uint8_t brp = timing->prescaler - 1;
-	const uint8_t sjw = (timing->sjw - 1) << 6;
-	uint8_t cnf1 = sjw | brp;
+	if (timing->sjw != CAN_SJW_NO_CHANGE) {
+		dev_data->sjw = (timing->sjw - 1) << 6;
+	}
+
+	uint8_t cnf1 = dev_data->sjw | brp;
 
 	/* CNF2; BTLMODE<7>|SAM<6>|PHSEG1<5:3>|PRSEG<2:0> */
 	const uint8_t btlmode = 1 << 7;
@@ -363,8 +366,7 @@ static int mcp2515_set_timing(const struct device *dev,
 	const uint8_t rx0_ctrl = BIT(6) | BIT(5) | BIT(2);
 	const uint8_t rx1_ctrl = BIT(6) | BIT(5);
 
-	__ASSERT((timing->sjw >= 1) && (timing->sjw <= 4),
-		 "1 <= SJW <= 4");
+	__ASSERT(timing->sjw <= 4, "1 <= SJW <= 4");
 	__ASSERT((timing->prop_seg >= 1) && (timing->prop_seg <= 8),
 		 "1 <= PROP <= 8");
 	__ASSERT((timing->phase_seg1 >= 1) && (timing->phase_seg1 <= 8),

drivers/can/can_mcp2515.h

@@ -54,6 +54,7 @@ struct mcp2515_data {
 	/* general data */
 	struct k_mutex mutex;
 	enum can_state old_state;
+	uint8_t sjw;
 };
 
 struct mcp2515_config {

drivers/can/can_mcux_flexcan.c

@@ -134,13 +134,17 @@ static int mcux_flexcan_set_timing(const struct device *dev,
 	ARG_UNUSED(timing_data);
 	struct mcux_flexcan_data *data = dev->data;
 	const struct mcux_flexcan_config *config = dev->config;
+	uint8_t sjw_backup = data->timing.sjw;
 	flexcan_timing_config_t timing_tmp;
 
 	if (!timing) {
 		return -EINVAL;
 	}
 
 	data->timing = *timing;
+	if (timing->sjw == CAN_SJW_NO_CHANGE) {
+		data->timing.sjw = sjw_backup;
+	}
 
 	timing_tmp.preDivider = data->timing.prescaler - 1U;
 	timing_tmp.rJumpwidth = data->timing.sjw - 1U;

drivers/can/can_stm32.c

@@ -371,15 +371,16 @@ int can_stm32_set_timing(const struct device *dev,
 		goto done;
 	}
 
-	can->BTR &= ~(CAN_BTR_BRP_Msk | CAN_BTR_TS1_Msk |
-		      CAN_BTR_TS2_Msk | CAN_BTR_SJW_Msk);
-
-	can->BTR |=
+	can->BTR = (can->BTR & ~(CAN_BTR_BRP_Msk | CAN_BTR_TS1_Msk | CAN_BTR_TS2_Msk)) |
 	     (((timing->phase_seg1 - 1) << CAN_BTR_TS1_Pos) & CAN_BTR_TS1_Msk) |
 	     (((timing->phase_seg2 - 1) << CAN_BTR_TS2_Pos) & CAN_BTR_TS2_Msk) |
-	     (((timing->sjw        - 1) << CAN_BTR_SJW_Pos) & CAN_BTR_SJW_Msk) |
 	     (((timing->prescaler  - 1) << CAN_BTR_BRP_Pos) & CAN_BTR_BRP_Msk);
 
+	if (timing->sjw != CAN_SJW_NO_CHANGE) {
+		can->BTR = (can->BTR & ~CAN_BTR_SJW_Msk) |
+			   (((timing->sjw - 1) << CAN_BTR_SJW_Pos) & CAN_BTR_SJW_Msk);
+	}
+
 	ret = can_leave_init_mode(can);
 	if (ret) {
 		LOG_ERR("Failed to leave init mode");

include/drivers/can.h

@@ -259,6 +259,8 @@ struct can_bus_err_cnt {
 	uint8_t rx_err_cnt;
 };
 
+/** SWJ value to indicate that the SJW should not be changed */
+#define CAN_SJW_NO_CHANGE 0
 
 /**
  * @brief canbus timings
@@ -727,6 +729,8 @@ static inline int z_impl_can_set_mode(const struct device *dev,
 /**
  * @brief Configure timing of a host controller.
  *
+ * If the sjw equals CAN_SJW_NO_CHANGE, the sjw parameter is not changed.
+ *
  * The second parameter timing_data is only relevant for CAN-FD.
  * If the controller does not support CAN-FD or the FD mode is not enabled,
  * this parameter is ignored.
@@ -783,12 +787,16 @@ static inline int can_set_bitrate(const struct device *dev,
 		return -EINVAL;
 	}
 
+	timing.sjw = CAN_SJW_NO_CHANGE;
+
 #ifdef CONFIG_CAN_FD_MODE
 	ret = can_calc_timing_data(dev, &timing_data, bitrate_data, 875);
 	if (ret < 0) {
 		return -EINVAL;
 	}
 
+	timing_data.sjw = CAN_SJW_NO_CHANGE;
+
 	return can_set_timing(dev, &timing, &timing_data);
 #else
 	return can_set_timing(dev, &timing, NULL);