diff --git a/drivers/can/can_common.c b/drivers/can/can_common.c index c7902ee9d20..6541c977255 100644 --- a/drivers/can/can_common.c +++ b/drivers/can/can_common.c @@ -170,7 +170,7 @@ static int can_calc_timing_int(uint32_t core_clock, struct can_timing *res, CAN_SYNC_SEG; uint16_t sp_err_min = UINT16_MAX; int sp_err; - struct can_timing tmp_res; + struct can_timing tmp_res = { 0 }; if (bitrate == 0 || sp >= 1000) { return -EINVAL; @@ -209,6 +209,10 @@ static int can_calc_timing_int(uint32_t core_clock, struct can_timing *res, LOG_DBG("SP error: %d 1/1000", sp_err_min); } + /* Calculate default sjw as phase_seg2 / 2 and clamp the result */ + res->sjw = MIN(res->phase_seg1, res->phase_seg2 / 2); + res->sjw = CLAMP(res->sjw, min->sjw, max->sjw); + return sp_err_min == UINT16_MAX ? -ENOTSUP : (int)sp_err_min; } @@ -300,18 +304,18 @@ static int check_timing_in_range(const struct can_timing *timing, const struct can_timing *min, const struct can_timing *max) { - if (timing->sjw != CAN_SJW_NO_CHANGE && - !IN_RANGE(timing->sjw, min->sjw, max->sjw)) { - return -ENOTSUP; - } - - if (!IN_RANGE(timing->prop_seg, min->prop_seg, max->prop_seg) || + if (!IN_RANGE(timing->sjw, min->sjw, max->sjw) || + !IN_RANGE(timing->prop_seg, min->prop_seg, max->prop_seg) || !IN_RANGE(timing->phase_seg1, min->phase_seg1, max->phase_seg1) || !IN_RANGE(timing->phase_seg2, min->phase_seg2, max->phase_seg2) || !IN_RANGE(timing->prescaler, min->prescaler, max->prescaler)) { return -ENOTSUP; } + if ((timing->sjw > timing->phase_seg1) || (timing->sjw > timing->phase_seg2)) { + return -ENOTSUP; + } + return 0; } @@ -333,7 +337,7 @@ int z_impl_can_set_timing(const struct device *dev, int z_impl_can_set_bitrate(const struct device *dev, uint32_t bitrate) { - struct can_timing timing; + struct can_timing timing = { 0 }; uint32_t max_bitrate; uint16_t sample_pnt; int ret; @@ -360,8 +364,6 @@ int z_impl_can_set_bitrate(const struct device *dev, uint32_t bitrate) return -ERANGE; } - timing.sjw = CAN_SJW_NO_CHANGE; - return can_set_timing(dev, &timing); } @@ -388,7 +390,7 @@ int z_impl_can_set_timing_data(const struct device *dev, int z_impl_can_set_bitrate_data(const struct device *dev, uint32_t bitrate_data) { - struct can_timing timing_data; + struct can_timing timing_data = { 0 }; uint32_t max_bitrate; uint16_t sample_pnt; int ret; @@ -415,8 +417,6 @@ int z_impl_can_set_bitrate_data(const struct device *dev, uint32_t bitrate_data) return -ERANGE; } - timing_data.sjw = CAN_SJW_NO_CHANGE; - return can_set_timing_data(dev, &timing_data); } #endif /* CONFIG_CAN_FD_MODE */ diff --git a/drivers/can/can_shell.c b/drivers/can/can_shell.c index a6e3e8045af..e07de338952 100644 --- a/drivers/can/can_shell.c +++ b/drivers/can/can_shell.c @@ -368,7 +368,7 @@ static int cmd_can_show(const struct shell *sh, size_t argc, char **argv) static int cmd_can_bitrate_set(const struct shell *sh, size_t argc, char **argv) { const struct device *dev = device_get_binding(argv[1]); - struct can_timing timing; + struct can_timing timing = { 0 }; uint16_t sample_pnt; uint32_t bitrate; char *endptr; @@ -392,16 +392,6 @@ static int cmd_can_bitrate_set(const struct shell *sh, size_t argc, char **argv) return -EINVAL; } - if (argc >= 5) { - timing.sjw = (uint16_t)strtoul(argv[4], &endptr, 10); - if (*endptr != '\0') { - shell_error(sh, "failed to parse SJW"); - return -EINVAL; - } - } else { - timing.sjw = CAN_SJW_NO_CHANGE; - } - err = can_calc_timing(dev, &timing, bitrate, sample_pnt); if (err < 0) { shell_error(sh, "failed to calculate timing for " @@ -410,17 +400,20 @@ static int cmd_can_bitrate_set(const struct shell *sh, size_t argc, char **argv) return err; } - if (timing.sjw == CAN_SJW_NO_CHANGE) { - shell_print(sh, "setting bitrate to %d bps, sample point %d.%d%% " - "(+/- %d.%d%%)", - bitrate, sample_pnt / 10, sample_pnt % 10, err / 10, err % 10); - } else { - shell_print(sh, "setting bitrate to %d bps, sample point %d.%d%% " - "(+/- %d.%d%%), sjw %d", - bitrate, sample_pnt / 10, sample_pnt % 10, err / 10, err % 10, - timing.sjw); + if (argc >= 5) { + /* Overwrite calculated default SJW with user-provided value */ + timing.sjw = (uint16_t)strtoul(argv[4], &endptr, 10); + if (*endptr != '\0') { + shell_error(sh, "failed to parse SJW"); + return -EINVAL; + } } + shell_print(sh, "setting bitrate to %d bps, sample point %d.%d%% " + "(+/- %d.%d%%), sjw %d", + bitrate, sample_pnt / 10, sample_pnt % 10, err / 10, err % 10, + timing.sjw); + LOG_DBG("sjw %u, prop_seg %u, phase_seg1 %u, phase_seg2 %u, prescaler %u", timing.sjw, timing.prop_seg, timing.phase_seg1, timing.phase_seg2, timing.prescaler); @@ -446,7 +439,7 @@ static int cmd_can_bitrate_set(const struct shell *sh, size_t argc, char **argv) static int cmd_can_dbitrate_set(const struct shell *sh, size_t argc, char **argv) { const struct device *dev = device_get_binding(argv[1]); - struct can_timing timing; + struct can_timing timing = { 0 }; uint16_t sample_pnt; uint32_t bitrate; char *endptr; @@ -470,16 +463,6 @@ static int cmd_can_dbitrate_set(const struct shell *sh, size_t argc, char **argv return -EINVAL; } - if (argc >= 5) { - timing.sjw = (uint16_t)strtoul(argv[4], &endptr, 10); - if (*endptr != '\0') { - shell_error(sh, "failed to parse SJW"); - return -EINVAL; - } - } else { - timing.sjw = CAN_SJW_NO_CHANGE; - } - err = can_calc_timing_data(dev, &timing, bitrate, sample_pnt); if (err < 0) { shell_error(sh, "failed to calculate timing for " @@ -488,17 +471,20 @@ static int cmd_can_dbitrate_set(const struct shell *sh, size_t argc, char **argv return err; } - if (timing.sjw == CAN_SJW_NO_CHANGE) { - shell_print(sh, "setting data bitrate to %d bps, sample point %d.%d%% " - "(+/- %d.%d%%)", - bitrate, sample_pnt / 10, sample_pnt % 10, err / 10, err % 10); - } else { - shell_print(sh, "setting data bitrate to %d bps, sample point %d.%d%% " - "(+/- %d.%d%%), sjw %d", - bitrate, sample_pnt / 10, sample_pnt % 10, err / 10, err % 10, - timing.sjw); + if (argc >= 5) { + /* Overwrite calculated default SJW with user-provided value */ + timing.sjw = (uint16_t)strtoul(argv[4], &endptr, 10); + if (*endptr != '\0') { + shell_error(sh, "failed to parse SJW"); + return -EINVAL; + } } + shell_print(sh, "setting data bitrate to %d bps, sample point %d.%d%% " + "(+/- %d.%d%%), sjw %d", + bitrate, sample_pnt / 10, sample_pnt % 10, err / 10, err % 10, + timing.sjw); + LOG_DBG("sjw %u, prop_seg %u, phase_seg1 %u, phase_seg2 %u, prescaler %u", timing.sjw, timing.prop_seg, timing.phase_seg1, timing.phase_seg2, timing.prescaler); diff --git a/include/zephyr/drivers/can.h b/include/zephyr/drivers/can.h index 15387afd2c4..db3d961db8b 100644 --- a/include/zephyr/drivers/can.h +++ b/include/zephyr/drivers/can.h @@ -865,8 +865,6 @@ __syscall int can_calc_timing_data(const struct device *dev, struct can_timing * /** * @brief Configure the bus timing for the data phase of a CAN-FD controller. * - * If the sjw equals CAN_SJW_NO_CHANGE, the sjw parameter is not changed. - * * @note @kconfig{CONFIG_CAN_FD_MODE} must be selected for this function to be * available. * @@ -936,8 +934,6 @@ int can_calc_prescaler(const struct device *dev, struct can_timing *timing, /** * @brief Configure the bus timing of a CAN controller. * - * If the sjw equals CAN_SJW_NO_CHANGE, the sjw parameter is not changed. - * * @see can_set_timing_data() * * @param dev Pointer to the device structure for the driver instance.