Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dts: bindings: can: remove deprecated properties for initial timing #68714

Merged
merged 2 commits into from
Mar 1, 2024
Merged

dts: bindings: can: remove deprecated properties for initial timing #68714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4dad088
dts: bindings: can: remove deprecated properties for initial timing
henrikbrixandersen Feb 7, 2024
8ba8321
doc: releases: migration guide: 3.7: removal of deprecated CAN DTS props
henrikbrixandersen Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/releases/migration-guide-3.7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ Bluetooth HCI
Controller Area Network (CAN)
=============================

* Removed the following deprecated CAN controller devicetree properties. Out-of-tree boards using
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a link to can-controller DTS binding?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no common binding for CAN controllers, just a base binding included by all CAN controller DTS bindings.

these properties need to switch to using the ``bus-speed``, ``sample-point``, ``bus-speed-data``,
and ``sample-point-data`` devicetree properties for specifying the initial CAN bitrate:

* ``sjw``
* ``prop-seg``
* ``phase-seg1``
* ``phase-seg1``
Comment on lines +54 to +55
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@henrikbrixandersen fyi I will be fixing the two "seg1 instead of seg2" typos in a global migration guide / release notes cleanup PR later today

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

* ``sjw-data``
* ``prop-seg-data``
* ``phase-seg1-data``
* ``phase-seg1-data``

Display
=======

Expand Down
53 changes: 16 additions & 37 deletions drivers/can/can_mcan.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1457,46 +1457,25 @@ int can_mcan_init(const struct device *dev)
return err;
}

if (config->common.sample_point) {
err = can_calc_timing(dev, &timing, config->common.bus_speed,
config->common.sample_point);
if (err == -EINVAL) {
LOG_ERR("Can't find timing for given param");
return -EIO;
}
LOG_DBG("Presc: %d, TS1: %d, TS2: %d", timing.prescaler, timing.phase_seg1,
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;
err = can_calc_prescaler(dev, &timing, config->common.bus_speed);
if (err != 0) {
LOG_WRN("Bitrate error: %d", err);
}
err = can_calc_timing(dev, &timing, config->common.bus_speed,
config->common.sample_point);
if (err == -EINVAL) {
LOG_ERR("Can't find timing for given param");
return -EIO;
}
#ifdef CONFIG_CAN_FD_MODE
if (config->common.sample_point_data) {
err = can_calc_timing_data(dev, &timing_data, config->common.bus_speed_data,
config->common.sample_point_data);
if (err == -EINVAL) {
LOG_ERR("Can't find timing for given dataphase param");
return -EIO;
}

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;
err = can_calc_prescaler(dev, &timing_data, config->common.bus_speed_data);
if (err != 0) {
LOG_WRN("Dataphase bitrate error: %d", err);
}
LOG_DBG("Presc: %d, TS1: %d, TS2: %d", timing.prescaler, timing.phase_seg1,
timing.phase_seg2);
LOG_DBG("Sample-point err : %d", err);
#ifdef CONFIG_CAN_FD_MODE
err = can_calc_timing_data(dev, &timing_data, config->common.bus_speed_data,
config->common.sample_point_data);
if (err == -EINVAL) {
LOG_ERR("Can't find timing for given dataphase param");
return -EIO;
}

LOG_DBG("Sample-point err data phase: %d", err);
#endif /* CONFIG_CAN_FD_MODE */

err = can_set_timing(dev, &timing);
Expand Down
51 changes: 10 additions & 41 deletions drivers/can/can_mcp2515.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,6 @@ LOG_MODULE_REGISTER(can_mcp2515, CONFIG_CAN_LOG_LEVEL);

#include "can_mcp2515.h"

#define SP_IS_SET(inst) DT_INST_NODE_HAS_PROP(inst, sample_point) ||

/* Macro to exclude the sample point algorithm from compilation if not used
* Without the macro, the algorithm would always waste ROM
*/
#define USE_SP_ALGO (DT_INST_FOREACH_STATUS_OKAY(SP_IS_SET) 0)

#define SP_AND_TIMING_NOT_SET(inst) \
(!DT_INST_NODE_HAS_PROP(inst, sample_point) && \
!(DT_INST_NODE_HAS_PROP(inst, prop_seg) && \
DT_INST_NODE_HAS_PROP(inst, phase_seg1) && \
DT_INST_NODE_HAS_PROP(inst, phase_seg2))) ||

#if DT_INST_FOREACH_STATUS_OKAY(SP_AND_TIMING_NOT_SET) 0
#error You must either set a sampling-point or timings (phase-seg* and prop-seg)
#endif

/* Timeout for changing mode */
#define MCP2515_MODE_CHANGE_TIMEOUT_USEC 1000
#define MCP2515_MODE_CHANGE_RETRIES 100
Expand Down Expand Up @@ -1009,27 +992,17 @@ 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;

if (dev_cfg->common.sample_point && USE_SP_ALGO) {
ret = can_calc_timing(dev, &timing, dev_cfg->common.bus_speed,
dev_cfg->common.sample_point);
if (ret == -EINVAL) {
LOG_ERR("Can't find timing for given param");
return -EIO;
}
LOG_DBG("Presc: %d, BS1: %d, BS2: %d",
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;
ret = can_calc_prescaler(dev, &timing, dev_cfg->common.bus_speed);
if (ret) {
LOG_WRN("Bitrate error: %d", ret);
}
ret = can_calc_timing(dev, &timing, dev_cfg->common.bus_speed,
dev_cfg->common.sample_point);
if (ret == -EINVAL) {
LOG_ERR("Can't find timing for given param");
return -EIO;
}

LOG_DBG("Presc: %d, BS1: %d, BS2: %d",
timing.prescaler, timing.phase_seg1, timing.phase_seg2);
LOG_DBG("Sample-point err : %d", ret);

k_usleep(MCP2515_OSC_STARTUP_US);

ret = can_set_timing(dev, &timing);
Expand Down Expand Up @@ -1058,14 +1031,10 @@ static int mcp2515_init(const struct device *dev)
.int_gpio = GPIO_DT_SPEC_INST_GET(inst, int_gpios), \
.int_thread_stack_size = CONFIG_CAN_MCP2515_INT_THREAD_STACK_SIZE, \
.int_thread_priority = CONFIG_CAN_MCP2515_INT_THREAD_PRIO, \
.tq_sjw = DT_INST_PROP(inst, sjw), \
.tq_prop = DT_INST_PROP_OR(inst, prop_seg, 0), \
.tq_bs1 = DT_INST_PROP_OR(inst, phase_seg1, 0), \
.tq_bs2 = DT_INST_PROP_OR(inst, phase_seg2, 0), \
.osc_freq = DT_INST_PROP(inst, osc_freq), \
}; \
\
CAN_DEVICE_DT_INST_DEFINE(inst, mcp2515_init, NULL, &mcp2515_data_##inst, \
CAN_DEVICE_DT_INST_DEFINE(inst, mcp2515_init, NULL, &mcp2515_data_##inst, \
&mcp2515_config_##inst, POST_KERNEL, CONFIG_CAN_INIT_PRIORITY, \
&can_api_funcs);

Expand Down
4 changes: 0 additions & 4 deletions drivers/can/can_mcp2515.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ struct mcp2515_config {
int int_thread_priority;

/* CAN timing */
uint8_t tq_sjw;
uint8_t tq_prop;
uint8_t tq_bs1;
uint8_t tq_bs2;
uint32_t osc_freq;
};

Expand Down
103 changes: 13 additions & 90 deletions drivers/can/can_mcp251xfd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,6 @@
#include <zephyr/logging/log.h>
LOG_MODULE_REGISTER(can_mcp251xfd, CONFIG_CAN_LOG_LEVEL);

#define SP_IS_SET(inst) DT_INST_NODE_HAS_PROP(inst, sample_point) ||

/*
* Macro to exclude the sample point algorithm from compilation if not used
* Without the macro, the algorithm would always waste ROM
*/
#define USE_SP_ALGO (DT_INST_FOREACH_STATUS_OKAY(SP_IS_SET) 0)

static void mcp251xfd_canframe_to_txobj(const struct can_frame *src, int mailbox_idx,
struct mcp251xfd_txobj *dst)
{
Expand Down Expand Up @@ -1296,68 +1288,6 @@ static void mcp251xfd_tef_fifo_handler(const struct device *dev, void *data)
k_sem_give(&dev_data->tx_sem);
}

#if defined(CONFIG_CAN_FD_MODE)
static int mcp251xfd_init_timing_struct_data(struct can_timing *timing,
const struct device *dev,
const struct mcp251xfd_timing_params *timing_params)
{
const struct mcp251xfd_config *dev_cfg = dev->config;
int ret;

if (USE_SP_ALGO && dev_cfg->common.sample_point_data > 0) {
ret = can_calc_timing_data(dev, timing, dev_cfg->common.bus_speed_data,
dev_cfg->common.sample_point_data);
if (ret < 0) {
return ret;
}
LOG_DBG("Data phase Presc: %d, BS1: %d, BS2: %d", timing->prescaler,
timing->phase_seg1, timing->phase_seg2);
LOG_DBG("Data phase Sample-point err : %d", ret);
} else {
timing->sjw = timing_params->sjw;
timing->prop_seg = timing_params->prop_seg;
timing->phase_seg1 = timing_params->phase_seg1;
timing->phase_seg2 = timing_params->phase_seg2;
ret = can_calc_prescaler(dev, timing, dev_cfg->common.bus_speed_data);
if (ret > 0) {
LOG_WRN("Data phase Bitrate error: %d", ret);
}
}

return ret;
}
#endif

static int mcp251xfd_init_timing_struct(struct can_timing *timing,
const struct device *dev,
const struct mcp251xfd_timing_params *timing_params)
{
const struct mcp251xfd_config *dev_cfg = dev->config;
int ret;

if (USE_SP_ALGO && dev_cfg->common.sample_point > 0) {
ret = can_calc_timing(dev, timing, dev_cfg->common.bus_speed,
dev_cfg->common.sample_point);
if (ret < 0) {
return ret;
}
LOG_DBG("Presc: %d, BS1: %d, BS2: %d", timing->prescaler, timing->phase_seg1,
timing->phase_seg2);
LOG_DBG("Sample-point err : %d", ret);
} else {
timing->sjw = timing_params->sjw;
timing->prop_seg = timing_params->prop_seg;
timing->phase_seg1 = timing_params->phase_seg1;
timing->phase_seg2 = timing_params->phase_seg2;
ret = can_calc_prescaler(dev, timing, dev_cfg->common.bus_speed);
if (ret > 0) {
LOG_WRN("Bitrate error: %d", ret);
}
}

return ret;
}

static inline int mcp251xfd_init_con_reg(const struct device *dev)
{
uint32_t *reg;
Expand Down Expand Up @@ -1593,18 +1523,28 @@ static int mcp251xfd_init(const struct device *dev)
goto done;
}

ret = mcp251xfd_init_timing_struct(&timing, dev, &dev_cfg->timing_params);
ret = can_calc_timing(dev, &timing, dev_cfg->common.bus_speed,
dev_cfg->common.sample_point);
if (ret < 0) {
LOG_ERR("Can't find timing for given param");
goto done;
}

LOG_DBG("Presc: %d, BS1: %d, BS2: %d", timing.prescaler, timing.phase_seg1,
timing.phase_seg2);
LOG_DBG("Sample-point err : %d", ret);

#if defined(CONFIG_CAN_FD_MODE)
ret = mcp251xfd_init_timing_struct_data(&timing_data, dev, &dev_cfg->timing_params_data);
ret = can_calc_timing_data(dev, &timing_data, dev_cfg->common.bus_speed_data,
dev_cfg->common.sample_point_data);
if (ret < 0) {
LOG_ERR("Can't find data timing for given param");
goto done;
}

LOG_DBG("Data phase Presc: %d, BS1: %d, BS2: %d", timing_data.prescaler,
timing_data.phase_seg1, timing_data.phase_seg2);
LOG_DBG("Data phase Sample-point err : %d", ret);
#endif

reg = mcp251xfd_read_crc(dev, MCP251XFD_REG_CON, MCP251XFD_REG_SIZE);
Expand Down Expand Up @@ -1745,23 +1685,6 @@ static const struct can_driver_api mcp251xfd_api_funcs = {
#endif
};

#define MCP251XFD_SET_TIMING_MACRO(inst, type) \
.timing_params##type = { \
.sjw = DT_INST_PROP(inst, sjw##type), \
.prop_seg = DT_INST_PROP_OR(inst, prop_seg##type, 0), \
.phase_seg1 = DT_INST_PROP_OR(inst, phase_seg1##type, 0), \
.phase_seg2 = DT_INST_PROP_OR(inst, phase_seg2##type, 0), \
}

#if defined(CONFIG_CAN_FD_MODE)
#define MCP251XFD_SET_TIMING(inst) \
MCP251XFD_SET_TIMING_MACRO(inst,), \
MCP251XFD_SET_TIMING_MACRO(inst, _data)
#else
#define MCP251XFD_SET_TIMING(inst) \
MCP251XFD_SET_TIMING_MACRO(inst,)
#endif

#define MCP251XFD_SET_CLOCK(inst) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, clocks), \
(.clk_dev = DEVICE_DT_GET(DT_INST_CLOCKS_CTLR(inst)), \
Expand All @@ -1786,7 +1709,7 @@ static const struct can_driver_api mcp251xfd_api_funcs = {
.timestamp_prescaler = DT_INST_PROP(inst, timestamp_prescaler), \
\
.osc_freq = DT_INST_PROP(inst, osc_freq), \
MCP251XFD_SET_TIMING(inst), \
\
.rx_fifo = {.ram_start_addr = MCP251XFD_RX_FIFO_START_ADDR, \
.reg_fifocon_addr = MCP251XFD_REG_FIFOCON(MCP251XFD_RX_FIFO_IDX), \
.capacity = MCP251XFD_RX_FIFO_ITEMS, \
Expand Down
13 changes: 0 additions & 13 deletions drivers/can/can_mcp251xfd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,13 +511,6 @@ struct mcp251xfd_data {

};

struct mcp251xfd_timing_params {
uint8_t sjw;
uint8_t prop_seg;
uint8_t phase_seg1;
uint8_t phase_seg2;
};

struct mcp251xfd_config {
const struct can_driver_config common;

Expand All @@ -534,12 +527,6 @@ struct mcp251xfd_config {

uint16_t timestamp_prescaler;

/* CAN Timing */
struct mcp251xfd_timing_params timing_params;
#if defined(CONFIG_CAN_FD_MODE)
struct mcp251xfd_timing_params timing_params_data;
#endif

const struct device *clk_dev;
uint8_t clk_id;

Expand Down
Loading
Loading