michaelh/zephyr
1
0
Fork 0
Code Releases Activity

drivers: can: solely use sjw from devicetree for initial timing

Browse source 
Update the CAN controller drivers to solely use the sjw and sjw-data
devicetree properties for setting the initial timing when devicetree timing
parameters are specified in Time Quanta (TQ).

Any timing set via the CAN timing APIs will contain either user-provided or
automatically calculated SJW values. This includes any timing parameters
calculated from bus-speed and bus-speed-data devicetree properties.

Update the CAN controller driver tests accordingly and remove the
CAN_SJW_NO_CHANGE definition as it has lost its meaning.

Fixes: #63033

Signed-off-by: Henrik Brix Andersen <hebad@vestas.com>
This commit is contained in:
Henrik Brix Andersen 2023-09-22 12:07:16 +02:00 • committed by Carles Cufí
commit a9d3935fa0
14 changed files with 50 additions and 112 deletions
Download patch file Download diff file Expand all files Collapse all files

13
drivers/can/can_esp32_twai.c
View file

@ -118,10 +118,8 @@ static int can_esp32_twai_set_timing(const struct device *dev, const struct can_
struct can_sja1000_data *data = dev->data;
uint8_t btr0;
uint8_t btr1;
uint8_t sjw;
__ASSERT_NO_MSG(timing->sjw == CAN_SJW_NO_CHANGE ||
(timing->sjw >= 0x1 && timing->sjw <= 0x4));
__ASSERT_NO_MSG(timing->sjw >= 0x1 && timing->sjw <= 0x4);
__ASSERT_NO_MSG(timing->prop_seg == 0);
__ASSERT_NO_MSG(timing->phase_seg1 >= 0x1 && timing->phase_seg1 <= 0x10);
__ASSERT_NO_MSG(timing->phase_seg2 >= 0x1 && timing->phase_seg2 <= 0x8);
@ -133,15 +131,8 @@ static int can_esp32_twai_set_timing(const struct device *dev, const struct can_
k_mutex_lock(&data->mod_lock, K_FOREVER);
if (timing->sjw == CAN_SJW_NO_CHANGE) {
sjw = data->sjw;
} else {
sjw = timing->sjw;
data->sjw = timing->sjw;
}
btr0 = TWAI_BAUD_PRESC_PREP(timing->prescaler - 1) |
TWAI_SYNC_JUMP_WIDTH_PREP(sjw - 1);
TWAI_SYNC_JUMP_WIDTH_PREP(timing->sjw - 1);
btr1 = TWAI_TIME_SEG1_PREP(timing->phase_seg1 - 1) |
TWAI_TIME_SEG2_PREP(timing->phase_seg2 - 1);

54
drivers/can/can_mcan.c
View file

@ -208,23 +208,12 @@ int can_mcan_set_timing(const struct device *dev, const struct can_timing *timin
__ASSERT_NO_MSG(timing->phase_seg1 <= 0x100 && timing->phase_seg1 > 1U);
__ASSERT_NO_MSG(timing->phase_seg2 <= 0x80 && timing->phase_seg2 > 1U);
__ASSERT_NO_MSG(timing->prescaler <= 0x200 && timing->prescaler > 0U);
__ASSERT_NO_MSG(timing->sjw == CAN_SJW_NO_CHANGE ||
(timing->sjw <= 0x80 && timing->sjw > 0U));
__ASSERT_NO_MSG(timing->sjw <= 0x80 && timing->sjw > 0U);
k_mutex_lock(&data->lock, K_FOREVER);
if (timing->sjw == CAN_SJW_NO_CHANGE) {
err = can_mcan_read_reg(dev, CAN_MCAN_NBTP, &nbtp);
if (err != 0) {
goto unlock;
}
nbtp &= CAN_MCAN_NBTP_NSJW;
} else {
nbtp |= FIELD_PREP(CAN_MCAN_NBTP_NSJW, timing->sjw - 1UL);
}
nbtp |= FIELD_PREP(CAN_MCAN_NBTP_NTSEG1, timing->phase_seg1 - 1UL) |
nbtp |= FIELD_PREP(CAN_MCAN_NBTP_NSJW, timing->sjw - 1UL) |
FIELD_PREP(CAN_MCAN_NBTP_NTSEG1, timing->phase_seg1 - 1UL) |
FIELD_PREP(CAN_MCAN_NBTP_NTSEG2, timing->phase_seg2 - 1UL) |
FIELD_PREP(CAN_MCAN_NBTP_NBRP, timing->prescaler - 1UL);
@ -254,23 +243,12 @@ int can_mcan_set_timing_data(const struct device *dev, const struct can_timing *
__ASSERT_NO_MSG(timing_data->phase_seg1 <= 0x20 && timing_data->phase_seg1 > 0U);
__ASSERT_NO_MSG(timing_data->phase_seg2 <= 0x10 && timing_data->phase_seg2 > 0U);
__ASSERT_NO_MSG(timing_data->prescaler <= 0x20 && timing_data->prescaler > 0U);
__ASSERT_NO_MSG(timing_data->sjw == CAN_SJW_NO_CHANGE ||
(timing_data->sjw <= 0x10 && timing_data->sjw > 0U));
__ASSERT_NO_MSG(timing_data->sjw <= 0x10 && timing_data->sjw > 0U);
k_mutex_lock(&data->lock, K_FOREVER);
if (timing_data->sjw == CAN_SJW_NO_CHANGE) {
err = can_mcan_read_reg(dev, CAN_MCAN_DBTP, &dbtp);
if (err != 0) {
goto unlock;
}
dbtp &= CAN_MCAN_DBTP_DSJW;
} else {
dbtp |= FIELD_PREP(CAN_MCAN_DBTP_DSJW, timing_data->sjw - 1UL);
}
dbtp |= FIELD_PREP(CAN_MCAN_DBTP_DTSEG1, timing_data->phase_seg1 - 1UL) |
dbtp |= FIELD_PREP(CAN_MCAN_DBTP_DSJW, timing_data->sjw - 1UL) |
FIELD_PREP(CAN_MCAN_DBTP_DTSEG1, timing_data->phase_seg1 - 1UL) |
FIELD_PREP(CAN_MCAN_DBTP_DTSEG2, timing_data->phase_seg2 - 1UL) |
FIELD_PREP(CAN_MCAN_DBTP_DBRP, timing_data->prescaler - 1UL);
@ -1357,9 +1335,9 @@ int can_mcan_init(const struct device *dev)
const struct can_mcan_config *config = dev->config;
const struct can_mcan_callbacks *cbs = config->callbacks;
struct can_mcan_data *data = dev->data;
struct can_timing timing;
struct can_timing timing = { 0 };
#ifdef CONFIG_CAN_FD_MODE
struct can_timing timing_data;
struct can_timing timing_data = { 0 };
#endif /* CONFIG_CAN_FD_MODE */
uint32_t reg;
int err;
@ -1485,6 +1463,7 @@ int can_mcan_init(const struct device *dev)
timing.phase_seg2);
LOG_DBG("Sample-point err : %d", err);
} else if (config->prop_ts1) {
timing.sjw = config->sjw;
timing.prop_seg = 0U;
timing.phase_seg1 = config->prop_ts1;
timing.phase_seg2 = config->ts2;
@ -1504,6 +1483,7 @@ int can_mcan_init(const struct device *dev)
LOG_DBG("Sample-point err data phase: %d", err);
} else if (config->prop_ts1_data) {
timing_data.sjw = config->sjw_data;
timing_data.prop_seg = 0U;
timing_data.phase_seg1 = config->prop_ts1_data;
timing_data.phase_seg2 = config->ts2_data;
@ -1514,12 +1494,18 @@ int can_mcan_init(const struct device *dev)
}
#endif /* CONFIG_CAN_FD_MODE */
timing.sjw = config->sjw;
can_mcan_set_timing(dev, &timing);
err = can_set_timing(dev, &timing);
if (err != 0) {
LOG_ERR("failed to set timing (err %d)", err);
return -ENODEV;
}
#ifdef CONFIG_CAN_FD_MODE
timing_data.sjw = config->sjw_data;
can_mcan_set_timing_data(dev, &timing_data);
err = can_set_timing_data(dev, &timing_data);
if (err != 0) {
LOG_ERR("failed to set data phase timing (err %d)", err);
return -ENODEV;
}
#endif /* CONFIG_CAN_FD_MODE */
reg = CAN_MCAN_IE_BOE | CAN_MCAN_IE_EWE | CAN_MCAN_IE_EPE | CAN_MCAN_IE_MRAFE |

11
drivers/can/can_mcp2515.c
View file

@ -361,11 +361,8 @@ 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;
if (timing->sjw != CAN_SJW_NO_CHANGE) {
dev_data->sjw = (timing->sjw - 1) << 6;
}
uint8_t cnf1 = dev_data->sjw | brp;
uint8_t sjw = (timing->sjw - 1) << 6;
uint8_t cnf1 = sjw | brp;
/* CNF2; BTLMODE<7>|SAM<6>|PHSEG1<5:3>|PRSEG<2:0> */
const uint8_t btlmode = 1 << 7;
@ -937,7 +934,7 @@ static int mcp2515_init(const struct device *dev)
{
const struct mcp2515_config *dev_cfg = dev->config;
struct mcp2515_data *dev_data = dev->data;
struct can_timing timing;
struct can_timing timing = { 0 };
k_tid_t tid;
int ret;
@ -998,7 +995,6 @@ static int mcp2515_init(const struct device *dev)
(void)memset(dev_data->filter, 0, sizeof(dev_data->filter));
dev_data->old_state = CAN_STATE_ERROR_ACTIVE;
timing.sjw = dev_cfg->tq_sjw;
if (dev_cfg->sample_point && USE_SP_ALGO) {
ret = can_calc_timing(dev, &timing, dev_cfg->bus_speed,
dev_cfg->sample_point);
@ -1010,6 +1006,7 @@ static int mcp2515_init(const struct device *dev)
timing.prescaler, timing.phase_seg1, timing.phase_seg2);
LOG_DBG("Sample-point err : %d", ret);
} else {
timing.sjw = dev_cfg->tq_sjw;
timing.prop_seg = dev_cfg->tq_prop;
timing.phase_seg1 = dev_cfg->tq_bs1;
timing.phase_seg2 = dev_cfg->tq_bs2;

1
drivers/can/can_mcp2515.h
View file

@ -46,7 +46,6 @@ struct mcp2515_data {
enum can_state old_state;
uint8_t mcp2515_mode;
bool started;
uint8_t sjw;
};
struct mcp2515_config {

10
drivers/can/can_mcux_flexcan.c
View file

@ -182,7 +182,6 @@ static int mcux_flexcan_set_timing(const struct device *dev,
const struct can_timing *timing)
{
struct mcux_flexcan_data *data = dev->data;
uint8_t sjw_backup = data->timing.sjw;
if (!timing) {
return -EINVAL;
@ -193,9 +192,6 @@ 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;
}
return 0;
}
@ -205,7 +201,6 @@ static int mcux_flexcan_set_timing_data(const struct device *dev,
const struct can_timing *timing_data)
{
struct mcux_flexcan_data *data = dev->data;
uint8_t sjw_backup = data->timing_data.sjw;
if (!timing_data) {
return -EINVAL;
@ -216,9 +211,6 @@ static int mcux_flexcan_set_timing_data(const struct device *dev,
}
data->timing_data = *timing_data;
if (timing_data->sjw == CAN_SJW_NO_CHANGE) {
data->timing_data.sjw = sjw_backup;
}
return 0;
}
@ -1203,6 +1195,7 @@ static int mcux_flexcan_init(const struct device *dev)
data->timing.phase_seg2);
LOG_DBG("Sample-point err : %d", err);
} else {
data->timing.sjw = config->sjw;
data->timing.prop_seg = config->prop_seg;
data->timing.phase_seg1 = config->phase_seg1;
data->timing.phase_seg2 = config->phase_seg2;
@ -1227,6 +1220,7 @@ static int mcux_flexcan_init(const struct device *dev)
data->timing_data.phase_seg2);
LOG_DBG("Sample-point err : %d", err);
} else {
data->timing_data.sjw = config->sjw_data;
data->timing_data.prop_seg = config->prop_seg_data;
data->timing_data.phase_seg1 = config->phase_seg1_data;
data->timing_data.phase_seg2 = config->phase_seg2_data;

6
drivers/can/can_rcar.c
View file

@ -987,7 +987,7 @@ static int can_rcar_init(const struct device *dev)
{
const struct can_rcar_cfg *config = dev->config;
struct can_rcar_data *data = dev->data;
struct can_timing timing;
struct can_timing timing = { 0 };
int ret;
uint16_t ctlr;
@ -1053,7 +1053,6 @@ static int can_rcar_init(const struct device *dev)
return ret;
}
timing.sjw = config->sjw;
if (config->sample_point) {
ret = can_calc_timing(dev, &timing, config->bus_speed,
config->sample_point);
@ -1065,6 +1064,7 @@ static int can_rcar_init(const struct device *dev)
timing.prescaler, timing.phase_seg1, timing.phase_seg2);
LOG_DBG("Sample-point err : %d", ret);
} else {
timing.sjw = config->sjw;
timing.prop_seg = config->prop_seg;
timing.phase_seg1 = config->phase_seg1;
timing.phase_seg2 = config->phase_seg2;
@ -1074,7 +1074,7 @@ static int can_rcar_init(const struct device *dev)
}
}
ret = can_rcar_set_timing(dev, &timing);
ret = can_set_timing(dev, &timing);
if (ret) {
return ret;
}

19
drivers/can/can_sja1000.c
View file

@ -107,9 +107,8 @@ int can_sja1000_set_timing(const struct device *dev, const struct can_timing *ti
struct can_sja1000_data *data = dev->data;
uint8_t btr0;
uint8_t btr1;
uint8_t sjw;
__ASSERT_NO_MSG(timing->sjw == CAN_SJW_NO_CHANGE || (timing->sjw >= 1 && timing->sjw <= 4));
__ASSERT_NO_MSG(timing->sjw >= 1 && timing->sjw <= 4);
__ASSERT_NO_MSG(timing->prop_seg == 0);
__ASSERT_NO_MSG(timing->phase_seg1 >= 1 && timing->phase_seg1 <= 16);
__ASSERT_NO_MSG(timing->phase_seg2 >= 1 && timing->phase_seg2 <= 8);
@ -121,15 +120,8 @@ int can_sja1000_set_timing(const struct device *dev, const struct can_timing *ti
k_mutex_lock(&data->mod_lock, K_FOREVER);
if (timing->sjw == CAN_SJW_NO_CHANGE) {
sjw = data->sjw;
} else {
sjw = timing->sjw;
data->sjw = timing->sjw;
}
btr0 = CAN_SJA1000_BTR0_BRP_PREP(timing->prescaler - 1) |
CAN_SJA1000_BTR0_SJW_PREP(sjw - 1);
CAN_SJA1000_BTR0_SJW_PREP(timing->sjw - 1);
btr1 = CAN_SJA1000_BTR1_TSEG1_PREP(timing->phase_seg1 - 1) |
CAN_SJA1000_BTR1_TSEG2_PREP(timing->phase_seg2 - 1);
@ -668,7 +660,7 @@ int can_sja1000_init(const struct device *dev)
{
const struct can_sja1000_config *config = dev->config;
struct can_sja1000_data *data = dev->data;
struct can_timing timing;
struct can_timing timing = { 0 };
int err;
__ASSERT_NO_MSG(config->read_reg != NULL);
@ -708,10 +700,6 @@ int can_sja1000_init(const struct device *dev)
can_sja1000_write_reg(dev, CAN_SJA1000_AMR2, 0xFF);
can_sja1000_write_reg(dev, CAN_SJA1000_AMR3, 0xFF);
/* Calculate initial timing parameters */
data->sjw = config->sjw;
timing.sjw = CAN_SJW_NO_CHANGE;
if (config->sample_point != 0) {
err = can_calc_timing(dev, &timing, config->bitrate, config->sample_point);
if (err == -EINVAL) {
@ -721,6 +709,7 @@ int can_sja1000_init(const struct device *dev)
LOG_DBG("initial sample point error: %d", err);
} else {
timing.sjw = config->sjw;
timing.prop_seg = 0;
timing.phase_seg1 = config->phase_seg1;
timing.phase_seg2 = config->phase_seg2;

15
drivers/can/can_stm32_bxcan.c
View file

@ -555,16 +555,13 @@ static int can_stm32_set_timing(const struct device *dev,
return -EBUSY;
}
can->BTR = (can->BTR & ~(CAN_BTR_BRP_Msk | CAN_BTR_TS1_Msk | CAN_BTR_TS2_Msk)) |
can->BTR = (can->BTR & ~(CAN_BTR_SJW_Msk | CAN_BTR_BRP_Msk |
CAN_BTR_TS1_Msk | CAN_BTR_TS2_Msk)) |
(((timing->sjw - 1) << CAN_BTR_SJW_Pos) & CAN_BTR_SJW_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->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);
}
k_mutex_unlock(&data->inst_mutex);
return 0;
@ -614,7 +611,7 @@ static int can_stm32_init(const struct device *dev)
const struct can_stm32_config *cfg = dev->config;
struct can_stm32_data *data = dev->data;
CAN_TypeDef *can = cfg->can;
struct can_timing timing;
struct can_timing timing = { 0 };
const struct device *clock;
uint32_t bank_offset;
int ret;
@ -675,7 +672,6 @@ static int can_stm32_init(const struct device *dev)
#ifdef CONFIG_CAN_AUTO_BUS_OFF_RECOVERY
can->MCR |= CAN_MCR_ABOM;
#endif
timing.sjw = cfg->sjw;
if (cfg->sample_point && USE_SP_ALGO) {
ret = can_calc_timing(dev, &timing, cfg->bus_speed,
cfg->sample_point);
@ -687,6 +683,7 @@ static int can_stm32_init(const struct device *dev)
timing.prescaler, timing.phase_seg1, timing.phase_seg2);
LOG_DBG("Sample-point err : %d", ret);
} else {
timing.sjw = cfg->sjw;
timing.prop_seg = 0;
timing.phase_seg1 = cfg->prop_ts1;
timing.phase_seg2 = cfg->ts2;
@ -696,7 +693,7 @@ static int can_stm32_init(const struct device *dev)
}
}
ret = can_stm32_set_timing(dev, &timing);
ret = can_set_timing(dev, &timing);
if (ret) {
return ret;
}

5
include/zephyr/drivers/can.h
View file

@ -234,11 +234,6 @@ struct can_bus_err_cnt {
uint8_t rx_err_cnt;
};
/** Synchronization Jump Width (SJW) value to indicate that the SJW should not
* be changed by the timing calculation.
*/
#define CAN_SJW_NO_CHANGE 0
/**
* @brief CAN bus timing structure
*

1
include/zephyr/drivers/can/can_sja1000.h
View file

@ -176,7 +176,6 @@ struct can_sja1000_data {
struct k_sem tx_idle;
can_tx_callback_t tx_callback;
void *tx_user_data;
uint32_t sjw;
void *custom;
};

4
tests/drivers/can/api/src/canfd.c
View file

@ -389,11 +389,9 @@ ZTEST_USER(canfd, test_set_bitrate_data_while_started)
*/
ZTEST_USER(canfd, test_set_timing_data_while_started)
{
struct can_timing timing;
struct can_timing timing = { 0 };
int err;
timing.sjw = CAN_SJW_NO_CHANGE;
err = can_calc_timing_data(can_dev, &timing, TEST_BITRATE_3, TEST_SAMPLE_POINT);
zassert_ok(err, "failed to calculate data timing (err %d)", err);

4
tests/drivers/can/api/src/classic.c
View file

@ -1031,11 +1031,9 @@ ZTEST_USER(can_classic, test_set_bitrate_while_started)
*/
ZTEST_USER(can_classic, test_set_timing_while_started)
{
struct can_timing timing;
struct can_timing timing = { 0 };
int err;
timing.sjw = CAN_SJW_NO_CHANGE;
err = can_calc_timing(can_dev, &timing, TEST_BITRATE_1, TEST_SAMPLE_POINT);
zassert_ok(err, "failed to calculate timing (err %d)", err);

8
tests/drivers/can/shell/src/main.c
View file

@ -136,11 +136,9 @@ static void can_shell_test_bitrate(const char *cmd, uint32_t expected_bitrate,
uint16_t expected_sample_pnt)
{
const struct shell *sh = shell_backend_dummy_get_ptr();
struct can_timing expected;
struct can_timing expected = { 0 };
int err;
expected.sjw = CAN_SJW_NO_CHANGE;
err = can_calc_timing(fake_can_dev, &expected, expected_bitrate, expected_sample_pnt);
zassert_ok(err, "failed to calculate reference timing (err %d)", err);
@ -180,13 +178,11 @@ static void can_shell_test_dbitrate(const char *cmd, uint32_t expected_bitrate,
uint16_t expected_sample_pnt)
{
const struct shell *sh = shell_backend_dummy_get_ptr();
struct can_timing expected;
struct can_timing expected = { 0 };
int err;
Z_TEST_SKIP_IFNDEF(CONFIG_CAN_FD_MODE);
expected.sjw = CAN_SJW_NO_CHANGE;
err = can_calc_timing_data(fake_can_dev, &expected, expected_bitrate, expected_sample_pnt);
zassert_ok(err, "failed to calculate reference timing (err %d)", err);

11
tests/drivers/can/timing/src/main.c
View file

@ -115,13 +115,13 @@ static void assert_timing_within_bounds(struct can_timing *timing,
const struct can_timing *min,
const struct can_timing *max)
{
zassert_true(timing->sjw == CAN_SJW_NO_CHANGE, "sjw does not equal CAN_SJW_NO_CHANGE");
zassert_true(timing->sjw <= max->sjw, "sjw exceeds max");
zassert_true(timing->prop_seg <= max->prop_seg, "prop_seg exceeds max");
zassert_true(timing->phase_seg1 <= max->phase_seg1, "phase_seg1 exceeds max");
zassert_true(timing->phase_seg2 <= max->phase_seg2, "phase_seg2 exceeds max");
zassert_true(timing->prescaler <= max->prescaler, "prescaler exceeds max");
zassert_true(timing->sjw >= min->sjw, "sjw lower than min");
zassert_true(timing->prop_seg >= min->prop_seg, "prop_seg lower than min");
zassert_true(timing->phase_seg1 >= min->phase_seg1, "phase_seg1 lower than min");
zassert_true(timing->phase_seg2 >= min->phase_seg2, "phase_seg2 lower than min");
@ -169,8 +169,6 @@ static void test_timing_values(const struct device *dev, const struct can_timing
printk("testing bitrate %u, sample point %u.%u%% (%s): ",
test->bitrate, test->sp / 10, test->sp % 10, test->invalid ? "invalid" : "valid");
timing.sjw = CAN_SJW_NO_CHANGE;
if (data_phase) {
if (IS_ENABLED(CONFIG_CAN_FD_MODE)) {
min = can_get_timing_data_min(dev);
@ -193,8 +191,9 @@ static void test_timing_values(const struct device *dev, const struct can_timing
zassert_true(sp_err <= SAMPLE_POINT_MARGIN, "sample point error %d too large",
sp_err);
printk("prop_seg = %u, phase_seg1 = %u, phase_seg2 = %u, prescaler = %u ",
timing.prop_seg, timing.phase_seg1, timing.phase_seg2, timing.prescaler);
printk("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);
assert_bitrate_correct(dev, &timing, test->bitrate);
assert_timing_within_bounds(&timing, min, max);