From 5e0ca9b41e5cd4d1d84ceef919510f3f2a22ecfe Mon Sep 17 00:00:00 2001 From: Alexander Wachter Date: Sun, 25 Apr 2021 17:15:56 +0200 Subject: [PATCH] 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 --- drivers/can/can_mcan.c | 24 +++++++++++++++++++----- drivers/can/can_mcp2515.c | 10 ++++++---- drivers/can/can_mcp2515.h | 1 + drivers/can/can_mcux_flexcan.c | 4 ++++ drivers/can/can_stm32.c | 11 ++++++----- include/drivers/can.h | 8 ++++++++ 6 files changed, 44 insertions(+), 14 deletions(-) diff --git a/drivers/can/can_mcan.c b/drivers/can/can_mcan.c index 39d37904936..2e8ae15caba 100644 --- a/drivers/can/can_mcan.c +++ b/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 } diff --git a/drivers/can/can_mcp2515.c b/drivers/can/can_mcp2515.c index 107a9466e4a..a6a3dc0f5d7 100644 --- a/drivers/can/can_mcp2515.c +++ b/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), diff --git a/drivers/can/can_mcp2515.h b/drivers/can/can_mcp2515.h index 33ffe175aa9..88f4a1677e4 100644 --- a/drivers/can/can_mcp2515.h +++ b/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 { diff --git a/drivers/can/can_mcux_flexcan.c b/drivers/can/can_mcux_flexcan.c index d255e8a513d..ca97eca0f1d 100644 --- a/drivers/can/can_mcux_flexcan.c +++ b/drivers/can/can_mcux_flexcan.c @@ -134,6 +134,7 @@ 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) { @@ -141,6 +142,9 @@ static int mcux_flexcan_set_timing(const struct device *dev, } 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; diff --git a/drivers/can/can_stm32.c b/drivers/can/can_stm32.c index cf869c7510f..17575dc99ca 100644 --- a/drivers/can/can_stm32.c +++ b/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"); diff --git a/include/drivers/can.h b/include/drivers/can.h index ac081f41bd1..8863c3cf548 100644 --- a/include/drivers/can.h +++ b/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);