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 <alexander@wachter.cloud>
This commit is contained in:
Alexander Wachter 2021-04-25 17:15:56 +02:00 committed by Kumar Gala
commit 5e0ca9b41e
6 changed files with 44 additions and 14 deletions

View file

@ -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);
@ -114,10 +123,15 @@ void can_mcan_configure_timing(struct can_mcan_reg *can,
CAN_MCAN_DBTP_DTSEG1_POS |
(((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
}

View file

@ -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),

View file

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

View file

@ -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;

View file

@ -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");

View file

@ -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);