From 3436c93387fc73dd0eb54cb3cb6959ac95ed639e Mon Sep 17 00:00:00 2001 From: Henrik Brix Andersen Date: Fri, 15 Dec 2023 13:36:18 +0100 Subject: [PATCH] drivers: can: remove run-time RTR filtering, add build-time RTR filter A growing number of CAN controllers do not have support for individual RX hardware filters based on the Remote Transmission Request (RTR) bit. This leads to various work-arounds on the driver level mixing hardware and software filtering. As the use of RTR frames is discouraged by CAN in Automation (CiA) - and not even supported by newer standards, e.g. CAN FD - this often leads to unnecessary overhead, added complexity, and worst-case to non-portable behavior between various CAN controller drivers. Instead, move to a simpler approach where the ability to accept/reject RTR frames is globally configured via Kconfig. By default, all incoming RTR frames are rejected at the driver level, a setting which can be supported in hardware by most in-tree CAN controllers drivers. Legacy applications or protocol implementations, where RTR reception is required, can now select CONFIG_CAN_ACCEPT_RTR to accept incoming RTR frames matching added CAN filters. These applications or protocols will need to distinguish between RTR and data frames in their respective CAN RX frame handling routines. Signed-off-by: Henrik Brix Andersen --- doc/hardware/peripherals/can/controller.rst | 4 +- drivers/can/Kconfig | 7 ++ drivers/can/can_loopback.c | 8 +- drivers/can/can_mcan.c | 21 +--- drivers/can/can_mcp2515.c | 8 +- drivers/can/can_mcp251xfd.c | 11 +- drivers/can/can_mcux_flexcan.c | 12 +- drivers/can/can_native_linux.c | 8 +- drivers/can/can_nxp_s32_canxl.c | 7 +- drivers/can/can_rcar.c | 8 +- drivers/can/can_shell.c | 24 +--- drivers/can/can_sja1000.c | 24 ++-- drivers/can/can_stm32_bxcan.c | 13 +- include/zephyr/drivers/can.h | 17 +-- include/zephyr/drivers/can/can_mcan.h | 1 - include/zephyr/net/socketcan_utils.h | 12 +- modules/canopennode/CO_driver.c | 18 ++- modules/canopennode/CO_driver_target.h | 3 + samples/drivers/can/counter/src/main.c | 12 +- samples/net/sockets/can/src/main.c | 2 +- subsys/canbus/isotp/isotp.c | 10 +- tests/drivers/can/api/src/classic.c | 111 +++++++++++++----- tests/drivers/can/api/src/common.c | 40 ++----- tests/drivers/can/api/src/common.h | 12 -- tests/drivers/can/api/src/utilities.c | 18 +-- tests/drivers/can/api/testcase.yaml | 16 +-- tests/drivers/can/shell/src/main.c | 34 +----- tests/net/socket/can/src/main.c | 15 ++- .../canbus/isotp/conformance/src/main.c | 2 +- 29 files changed, 243 insertions(+), 235 deletions(-) diff --git a/doc/hardware/peripherals/can/controller.rst b/doc/hardware/peripherals/can/controller.rst index 6943949d1a..e871020134 100644 --- a/doc/hardware/peripherals/can/controller.rst +++ b/doc/hardware/peripherals/can/controller.rst @@ -203,7 +203,7 @@ The filter for this example is configured to match the identifier 0x123 exactly. .. code-block:: C const struct can_filter my_filter = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = 0x123, .mask = CAN_STD_ID_MASK }; @@ -226,7 +226,7 @@ The filter for this example is configured to match the extended identifier .. code-block:: C const struct can_filter my_filter = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = 0x1234567, .mask = CAN_EXT_ID_MASK }; diff --git a/drivers/can/Kconfig b/drivers/can/Kconfig index e4fd470950..d31d7b981b 100644 --- a/drivers/can/Kconfig +++ b/drivers/can/Kconfig @@ -54,6 +54,13 @@ config CAN_STATS help Enable CAN controller device statistics. +config CAN_ACCEPT_RTR + bool "Accept Remote Transmission Requests (RTR) frames" + help + Accept incoming Remote Transmission Request (RTR) frames matching CAN RX filters. Unless + enabled, all incoming Remote Transmission Request (RTR) frames are rejected at the driver + level. + config CAN_FD_MODE bool "CAN FD" help diff --git a/drivers/can/can_loopback.c b/drivers/can/can_loopback.c index 5eb88b163e..938002f597 100644 --- a/drivers/can/can_loopback.c +++ b/drivers/can/can_loopback.c @@ -77,6 +77,12 @@ static void tx_thread(void *arg1, void *arg2, void *arg3) continue; } +#ifndef CONFIG_CAN_ACCEPT_RTR + if ((frame.frame.flags & CAN_FRAME_RTR) != 0U) { + continue; + } +#endif /* !CONFIG_CAN_ACCEPT_RTR */ + k_mutex_lock(&data->mtx, K_FOREVER); for (int i = 0; i < CONFIG_CAN_MAX_FILTER; i++) { @@ -172,7 +178,7 @@ static int can_loopback_add_rx_filter(const struct device *dev, can_rx_callback_ LOG_DBG("Setting filter ID: 0x%x, mask: 0x%x", filter->id, filter->mask); - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA | CAN_FILTER_RTR)) != 0) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } diff --git a/drivers/can/can_mcan.c b/drivers/can/can_mcan.c index dc54bc1967..d777e325b8 100644 --- a/drivers/can/can_mcan.c +++ b/drivers/can/can_mcan.c @@ -615,7 +615,6 @@ static void can_mcan_get_message(const struct device *dev, uint16_t fifo_offset, struct can_frame frame = {0}; can_rx_callback_t cb; void *user_data; - uint8_t flags; uint32_t get_idx; uint32_t filt_idx; int data_length; @@ -666,20 +665,8 @@ static void can_mcan_get_message(const struct device *dev, uint16_t fifo_offset, if (hdr.xtd != 0) { frame.id = hdr.ext_id; frame.flags |= CAN_FRAME_IDE; - flags = cbs->ext[filt_idx].flags; } else { frame.id = hdr.std_id; - flags = cbs->std[filt_idx].flags; - } - - if (((frame.flags & CAN_FRAME_RTR) == 0U && (flags & CAN_FILTER_DATA) == 0U) || - ((frame.flags & CAN_FRAME_RTR) != 0U && (flags & CAN_FILTER_RTR) == 0U)) { - /* RTR bit does not match filter, drop frame */ - err = can_mcan_write_reg(dev, fifo_ack_reg, get_idx); - if (err != 0) { - return; - } - goto ack; } data_length = can_dlc_to_bytes(frame.dlc); @@ -718,7 +705,6 @@ static void can_mcan_get_message(const struct device *dev, uint16_t fifo_offset, LOG_ERR("Frame is too big"); } -ack: err = can_mcan_write_reg(dev, fifo_ack_reg, get_idx); if (err != 0) { return; @@ -1042,7 +1028,6 @@ int can_mcan_add_rx_filter_std(const struct device *dev, can_rx_callback_t callb __ASSERT_NO_MSG(filter_id <= cbs->num_std); cbs->std[filter_id].function = callback; cbs->std[filter_id].user_data = user_data; - cbs->std[filter_id].flags = filter->flags; return filter_id; } @@ -1095,7 +1080,6 @@ static int can_mcan_add_rx_filter_ext(const struct device *dev, can_rx_callback_ __ASSERT_NO_MSG(filter_id <= cbs->num_ext); cbs->ext[filter_id].function = callback; cbs->ext[filter_id].user_data = user_data; - cbs->ext[filter_id].flags = filter->flags; return filter_id; } @@ -1111,7 +1095,7 @@ int can_mcan_add_rx_filter(const struct device *dev, can_rx_callback_t callback, return -EINVAL; } - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA | CAN_FILTER_RTR)) != 0U) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0U) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } @@ -1430,6 +1414,9 @@ int can_mcan_init(const struct device *dev) } reg |= FIELD_PREP(CAN_MCAN_GFC_ANFE, 0x2) | FIELD_PREP(CAN_MCAN_GFC_ANFS, 0x2); + if (!IS_ENABLED(CONFIG_CAN_ACCEPT_RTR)) { + reg |= CAN_MCAN_GFC_RRFS | CAN_MCAN_GFC_RRFE; + } err = can_mcan_write_reg(dev, CAN_MCAN_GFC, reg); if (err != 0) { diff --git a/drivers/can/can_mcp2515.c b/drivers/can/can_mcp2515.c index 8d02db834f..255a8cdbe7 100644 --- a/drivers/can/can_mcp2515.c +++ b/drivers/can/can_mcp2515.c @@ -631,7 +631,7 @@ static int mcp2515_add_rx_filter(const struct device *dev, __ASSERT(rx_cb != NULL, "response_ptr can not be null"); - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA | CAN_FILTER_RTR)) != 0) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } @@ -693,6 +693,12 @@ static void mcp2515_rx_filter(const struct device *dev, can_rx_callback_t callback; struct can_frame tmp_frame; +#ifndef CONFIG_CAN_ACCEPT_RTR + if ((frame->flags & CAN_FRAME_RTR) != 0U) { + return; + } +#endif /* !CONFIG_CAN_ACCEPT_RTR */ + k_mutex_lock(&dev_data->mutex, K_FOREVER); for (; filter_id < CONFIG_CAN_MAX_FILTER; filter_id++) { diff --git a/drivers/can/can_mcp251xfd.c b/drivers/can/can_mcp251xfd.c index 1b90982d64..e029d54496 100644 --- a/drivers/can/can_mcp251xfd.c +++ b/drivers/can/can_mcp251xfd.c @@ -576,11 +576,6 @@ static int mcp251xfd_add_rx_filter(const struct device *dev, can_rx_callback_t r goto done; } - if ((filter->flags & CAN_FILTER_RTR) != 0) { - filter_idx = -ENOTSUP; - goto done; - } - reg = mcp251xfd_get_spi_buf_ptr(dev); if ((filter->flags & CAN_FILTER_IDE) != 0) { @@ -1267,6 +1262,12 @@ static void mcp251xfd_rx_fifo_handler(const struct device *dev, void *data) mcp251xfd_rxobj_to_canframe(rxobj, &dst); +#ifndef CONFIG_CAN_ACCEPT_RTR + if ((dst.flags & CAN_FRAME_RTR) != 0U) { + return; + } +#endif /* !CONFIG_CAN_ACCEPT_RTR */ + filhit = FIELD_GET(MCP251XFD_OBJ_FILHIT_MASK, rxobj->flags); if ((dev_data->filter_usage & BIT(filhit)) != 0) { LOG_DBG("Received msg CAN id: 0x%x", dst.id); diff --git a/drivers/can/can_mcux_flexcan.c b/drivers/can/can_mcux_flexcan.c index cb300bb753..f83b2ec212 100644 --- a/drivers/can/can_mcux_flexcan.c +++ b/drivers/can/can_mcux_flexcan.c @@ -625,8 +625,7 @@ static void mcux_flexcan_can_filter_to_mbconfig(const struct can_filter *src, uint32_t *mask) { static const uint32_t ide_mask = 1U; - uint32_t rtr_mask = (src->flags & (CAN_FILTER_DATA | CAN_FILTER_RTR)) != - (CAN_FILTER_DATA | CAN_FILTER_RTR) ? 1U : 0U; + static const uint32_t rtr_mask = !IS_ENABLED(CONFIG_CAN_ACCEPT_RTR); if ((src->flags & CAN_FILTER_IDE) != 0) { dest->format = kFLEXCAN_FrameFormatExtend; @@ -638,11 +637,7 @@ static void mcux_flexcan_can_filter_to_mbconfig(const struct can_filter *src, *mask = FLEXCAN_RX_MB_STD_MASK(src->mask, rtr_mask, ide_mask); } - if ((src->flags & CAN_FILTER_RTR) != 0) { - dest->type = kFLEXCAN_FrameTypeRemote; - } else { - dest->type = kFLEXCAN_FrameTypeData; - } + dest->type = kFLEXCAN_FrameTypeData; } static int mcux_flexcan_get_state(const struct device *dev, enum can_state *state, @@ -786,7 +781,6 @@ static int mcux_flexcan_add_rx_filter(const struct device *dev, void *user_data, const struct can_filter *filter) { - const uint8_t supported = CAN_FILTER_IDE | CAN_FILTER_DATA | CAN_FILTER_RTR; const struct mcux_flexcan_config *config = dev->config; struct mcux_flexcan_data *data = dev->data; status_t status; @@ -796,7 +790,7 @@ static int mcux_flexcan_add_rx_filter(const struct device *dev, __ASSERT_NO_MSG(callback); - if ((filter->flags & ~(supported)) != 0) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } diff --git a/drivers/can/can_native_linux.c b/drivers/can/can_native_linux.c index e5bd763f93..940123bdd9 100644 --- a/drivers/can/can_native_linux.c +++ b/drivers/can/can_native_linux.c @@ -105,6 +105,12 @@ static void rx_thread(void *arg1, void *arg2, void *arg3) socketcan_to_can_frame(&sframe, &frame); +#ifndef CONFIG_CAN_ACCEPT_RTR + if ((frame.flags & CAN_FRAME_RTR) != 0U) { + continue; + } +#endif /* !CONFIG_CAN_ACCEPT_RTR*/ + LOG_DBG("Received %d bytes. Id: 0x%x, ID type: %s %s", frame.dlc, frame.id, (frame.flags & CAN_FRAME_IDE) != 0 ? "extended" : "standard", @@ -197,7 +203,7 @@ static int can_native_linux_add_rx_filter(const struct device *dev, can_rx_callb LOG_DBG("Setting filter ID: 0x%x, mask: 0x%x", filter->id, filter->mask); - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA | CAN_FILTER_RTR)) != 0) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } diff --git a/drivers/can/can_nxp_s32_canxl.c b/drivers/can/can_nxp_s32_canxl.c index 6a2180867f..2ea854fad9 100644 --- a/drivers/can/can_nxp_s32_canxl.c +++ b/drivers/can/can_nxp_s32_canxl.c @@ -506,7 +506,8 @@ static int can_nxp_s32_add_rx_filter(const struct device *dev, uint32_t mask; __ASSERT_NO_MSG(callback != NULL); - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA)) != 0) { + + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } @@ -538,6 +539,10 @@ static int can_nxp_s32_add_rx_filter(const struct device *dev, mask = (filter->mask << CANXL_IP_ID_STD_SHIFT) & CANXL_IP_ID_STD_MASK; } +#ifndef CONFIG_CAN_ACCEPT_RTR + mask |= CANXL_MSG_DESCRIPTORS_MDFLT1FD_RTRMSK_MASK; +#endif /* !CONFIG_CAN_ACCEPT_RTR */ + Canexcel_Ip_EnterFreezeMode(config->instance); #ifdef CONFIG_CAN_NXP_S32_RX_FIFO diff --git a/drivers/can/can_rcar.c b/drivers/can/can_rcar.c index 35204b86c1..98c134ac50 100644 --- a/drivers/can/can_rcar.c +++ b/drivers/can/can_rcar.c @@ -362,6 +362,12 @@ static void can_rcar_rx_filter_isr(const struct device *dev, struct can_frame tmp_frame; uint8_t i; +#ifndef CONFIG_CAN_ACCEPT_RTR + if ((frame->flags & CAN_FRAME_RTR) != 0U) { + return; + } +#endif /* !CONFIG_CAN_ACCEPT_RTR */ + for (i = 0; i < CONFIG_CAN_RCAR_MAX_FILTER; i++) { if (data->rx_callback[i] == NULL) { continue; @@ -957,7 +963,7 @@ static int can_rcar_add_rx_filter(const struct device *dev, can_rx_callback_t cb struct can_rcar_data *data = dev->data; int filter_id; - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA)) != 0) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } diff --git a/drivers/can/can_shell.c b/drivers/can/can_shell.c index 3780e20e8d..3f193ae949 100644 --- a/drivers/can/can_shell.c +++ b/drivers/can/can_shell.c @@ -801,7 +801,7 @@ static int cmd_can_filter_add(const struct shell *sh, size_t argc, char **argv) /* Defaults */ max_id = CAN_MAX_STD_ID; - filter.flags = CAN_FILTER_DATA; + filter.flags = 0U; /* Parse options */ while (argidx < argc && strncmp(argv[argidx], "-", 1) == 0) { @@ -812,13 +812,6 @@ static int cmd_can_filter_add(const struct shell *sh, size_t argc, char **argv) filter.flags |= CAN_FILTER_IDE; max_id = CAN_MAX_EXT_ID; argidx++; - } else if (strcmp(argv[argidx], "-r") == 0) { - filter.flags |= CAN_FILTER_RTR; - argidx++; - } else if (strcmp(argv[argidx], "-R") == 0) { - filter.flags &= ~(CAN_FILTER_DATA); - filter.flags |= CAN_FILTER_RTR; - argidx++; } else { shell_error(sh, "unsupported argument %s", argv[argidx]); shell_help(sh); @@ -874,14 +867,11 @@ static int cmd_can_filter_add(const struct shell *sh, size_t argc, char **argv) return err; } - shell_print(sh, "adding filter with %s (%d-bit) CAN ID 0x%0*x, " - "CAN ID mask 0x%0*x, data frames %d, RTR frames %d", + shell_print(sh, "adding filter with %s (%d-bit) CAN ID 0x%0*x, CAN ID mask 0x%0*x", (filter.flags & CAN_FILTER_IDE) != 0 ? "extended" : "standard", (filter.flags & CAN_FILTER_IDE) != 0 ? 29 : 11, (filter.flags & CAN_FILTER_IDE) != 0 ? 8 : 3, filter.id, - (filter.flags & CAN_FILTER_IDE) != 0 ? 8 : 3, filter.mask, - (filter.flags & CAN_FILTER_DATA) != 0 ? 1 : 0, - (filter.flags & CAN_FILTER_RTR) != 0 ? 1 : 0); + (filter.flags & CAN_FILTER_IDE) != 0 ? 8 : 3, filter.mask); err = can_add_rx_filter_msgq(dev, &can_shell_rx_msgq, &filter); if (err < 0) { @@ -999,11 +989,9 @@ SHELL_DYNAMIC_CMD_CREATE(dsub_can_device_name_mode, cmd_can_device_name_mode); SHELL_STATIC_SUBCMD_SET_CREATE(sub_can_filter_cmds, SHELL_CMD_ARG(add, &dsub_can_device_name, "Add rx filter\n" - "Usage: can filter add [-e] [-r] [-R] [CAN ID mask]\n" - "-e use extended (29-bit) CAN ID/CAN ID mask\n" - "-r also match Remote Transmission Request (RTR) frames\n" - "-R only match Remote Transmission Request (RTR) frames", - cmd_can_filter_add, 3, 4), + "Usage: can filter add [-e] [CAN ID mask]\n" + "-e use extended (29-bit) CAN ID/CAN ID mask\n", + cmd_can_filter_add, 3, 2), SHELL_CMD_ARG(remove, &dsub_can_device_name, "Remove rx filter\n" "Usage: can filter remove ", diff --git a/drivers/can/can_sja1000.c b/drivers/can/can_sja1000.c index b0eac60063..c5ee7bebcf 100644 --- a/drivers/can/can_sja1000.c +++ b/drivers/can/can_sja1000.c @@ -427,7 +427,7 @@ int can_sja1000_add_rx_filter(const struct device *dev, can_rx_callback_t callba int filter_id = -ENOSPC; int i; - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA | CAN_FILTER_RTR)) != 0) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } @@ -569,18 +569,24 @@ static void can_sja1000_handle_receive_irq(const struct device *dev) do { can_sja1000_read_frame(dev, &frame); - for (i = 0; i < ARRAY_SIZE(data->filters); i++) { - if (!atomic_test_bit(data->rx_allocs, i)) { - continue; - } +#ifndef CONFIG_CAN_ACCEPT_RTR + if ((frame.flags & CAN_FRAME_RTR) == 0U) { +#endif /* !CONFIG_CAN_ACCEPT_RTR */ + for (i = 0; i < ARRAY_SIZE(data->filters); i++) { + if (!atomic_test_bit(data->rx_allocs, i)) { + continue; + } - if (can_frame_matches_filter(&frame, &data->filters[i].filter)) { - callback = data->filters[i].callback; - if (callback != NULL) { - callback(dev, &frame, data->filters[i].user_data); + if (can_frame_matches_filter(&frame, &data->filters[i].filter)) { + callback = data->filters[i].callback; + if (callback != NULL) { + callback(dev, &frame, data->filters[i].user_data); + } } } +#ifndef CONFIG_CAN_ACCEPT_RTR } +#endif /* !CONFIG_CAN_ACCEPT_RTR */ can_sja1000_write_reg(dev, CAN_SJA1000_CMR, CAN_SJA1000_CMR_RRB); sr = can_sja1000_read_reg(dev, CAN_SJA1000_SR); diff --git a/drivers/can/can_stm32_bxcan.c b/drivers/can/can_stm32_bxcan.c index 71ac02cb20..03f67e8667 100644 --- a/drivers/can/can_stm32_bxcan.c +++ b/drivers/can/can_stm32_bxcan.c @@ -886,8 +886,7 @@ static void can_stm32_set_filter_bank(int filter_id, CAN_FilterRegister_TypeDef static inline uint32_t can_stm32_filter_to_std_mask(const struct can_filter *filter) { - uint32_t rtr_mask = (filter->flags & (CAN_FILTER_DATA | CAN_FILTER_RTR)) != - (CAN_FILTER_DATA | CAN_FILTER_RTR) ? 1U : 0U; + uint32_t rtr_mask = !IS_ENABLED(CONFIG_CAN_ACCEPT_RTR); return (filter->mask << CAN_STM32_FIRX_STD_ID_POS) | (rtr_mask << CAN_STM32_FIRX_STD_RTR_POS) | @@ -896,8 +895,7 @@ static inline uint32_t can_stm32_filter_to_std_mask(const struct can_filter *fil static inline uint32_t can_stm32_filter_to_ext_mask(const struct can_filter *filter) { - uint32_t rtr_mask = (filter->flags & (CAN_FILTER_DATA | CAN_FILTER_RTR)) != - (CAN_FILTER_DATA | CAN_FILTER_RTR) ? 1U : 0U; + uint32_t rtr_mask = !IS_ENABLED(CONFIG_CAN_ACCEPT_RTR); return (filter->mask << CAN_STM32_FIRX_EXT_EXT_ID_POS) | (rtr_mask << CAN_STM32_FIRX_EXT_RTR_POS) | @@ -906,15 +904,12 @@ static inline uint32_t can_stm32_filter_to_ext_mask(const struct can_filter *fil static inline uint32_t can_stm32_filter_to_std_id(const struct can_filter *filter) { - return (filter->id << CAN_STM32_FIRX_STD_ID_POS) | - (((filter->flags & CAN_FILTER_RTR) != 0) ? (1U << CAN_STM32_FIRX_STD_RTR_POS) : 0U); + return (filter->id << CAN_STM32_FIRX_STD_ID_POS); } static inline uint32_t can_stm32_filter_to_ext_id(const struct can_filter *filter) { return (filter->id << CAN_STM32_FIRX_EXT_EXT_ID_POS) | - (((filter->flags & CAN_FILTER_RTR) != 0) ? - (1U << CAN_STM32_FIRX_EXT_RTR_POS) : 0U) | (1U << CAN_STM32_FIRX_EXT_IDE_POS); } @@ -995,7 +990,7 @@ static int can_stm32_add_rx_filter(const struct device *dev, can_rx_callback_t c struct can_stm32_data *data = dev->data; int filter_id; - if ((filter->flags & ~(CAN_FILTER_IDE | CAN_FILTER_DATA | CAN_FILTER_RTR)) != 0) { + if ((filter->flags & ~(CAN_FILTER_IDE)) != 0) { LOG_ERR("unsupported CAN filter flags 0x%02x", filter->flags); return -ENOTSUP; } diff --git a/include/zephyr/drivers/can.h b/include/zephyr/drivers/can.h index 7221df1ea5..dfcb62906d 100644 --- a/include/zephyr/drivers/can.h +++ b/include/zephyr/drivers/can.h @@ -203,11 +203,6 @@ struct can_frame { /** Filter matches frames with extended (29-bit) CAN IDs */ #define CAN_FILTER_IDE BIT(0) -/** Filter matches Remote Transmission Request (RTR) frames */ -#define CAN_FILTER_RTR BIT(1) - -/** Filter matches data frames */ -#define CAN_FILTER_DATA BIT(2) /** @} */ @@ -1259,7 +1254,7 @@ static inline int can_add_rx_filter(const struct device *dev, can_rx_callback_t { const struct can_driver_api *api = (const struct can_driver_api *)dev->api; - if (filter == NULL || (filter->flags & (CAN_FILTER_DATA | CAN_FILTER_RTR)) == 0) { + if (filter == NULL) { return -EINVAL; } @@ -1683,16 +1678,6 @@ static inline bool can_frame_matches_filter(const struct can_frame *frame, return false; } - if ((frame->flags & CAN_FRAME_RTR) == 0 && (filter->flags & CAN_FILTER_DATA) == 0) { - /* non-RTR frame, remote transmission request (RTR) filter */ - return false; - } - - if ((frame->flags & CAN_FRAME_RTR) != 0 && (filter->flags & CAN_FILTER_RTR) == 0) { - /* Remote transmission request (RTR) frame, non-RTR filter */ - return false; - } - if ((frame->id ^ filter->id) & filter->mask) { /* Masked ID mismatch */ return false; diff --git a/include/zephyr/drivers/can/can_mcan.h b/include/zephyr/drivers/can/can_mcan.h index 86d0ad6fd3..8e09d58983 100644 --- a/include/zephyr/drivers/can/can_mcan.h +++ b/include/zephyr/drivers/can/can_mcan.h @@ -1164,7 +1164,6 @@ struct can_mcan_tx_callback { struct can_mcan_rx_callback { can_rx_callback_t function; void *user_data; - uint8_t flags; }; /** diff --git a/include/zephyr/net/socketcan_utils.h b/include/zephyr/net/socketcan_utils.h index 9b00d6dadc..7db287ff84 100644 --- a/include/zephyr/net/socketcan_utils.h +++ b/include/zephyr/net/socketcan_utils.h @@ -95,14 +95,6 @@ static inline void socketcan_to_can_filter(const struct socketcan_filter *sfilte zfilter->flags |= (sfilter->can_id & BIT(31)) != 0 ? CAN_FILTER_IDE : 0; zfilter->id = sfilter->can_id & BIT_MASK(29); zfilter->mask = sfilter->can_mask & BIT_MASK(29); - - if ((sfilter->can_mask & BIT(30)) == 0) { - zfilter->flags |= CAN_FILTER_DATA | CAN_FILTER_RTR; - } else if ((sfilter->can_id & BIT(30)) == 0) { - zfilter->flags |= CAN_FILTER_DATA; - } else { - zfilter->flags |= CAN_FILTER_RTR; - } } /** @@ -118,13 +110,11 @@ static inline void socketcan_from_can_filter(const struct can_filter *zfilter, sfilter->can_id = zfilter->id; sfilter->can_id |= (zfilter->flags & CAN_FILTER_IDE) != 0 ? BIT(31) : 0; - sfilter->can_id |= (zfilter->flags & CAN_FILTER_RTR) != 0 ? BIT(30) : 0; sfilter->can_mask = zfilter->mask; sfilter->can_mask |= (zfilter->flags & CAN_FILTER_IDE) != 0 ? BIT(31) : 0; - if ((zfilter->flags & (CAN_FILTER_DATA | CAN_FILTER_RTR)) != - (CAN_FILTER_DATA | CAN_FILTER_RTR)) { + if (!IS_ENABLED(CONFIG_CAN_ACCEPT_RTR)) { sfilter->can_mask |= BIT(30); } } diff --git a/modules/canopennode/CO_driver.c b/modules/canopennode/CO_driver.c index e8892e080f..4d161a4940 100644 --- a/modules/canopennode/CO_driver.c +++ b/modules/canopennode/CO_driver.c @@ -96,6 +96,11 @@ static void canopen_rx_callback(const struct device *dev, struct can_frame *fram } if (((frame->id ^ buffer->ident) & buffer->mask) == 0U) { +#ifdef CONFIG_CAN_ACCEPT_RTR + if (buffer->rtr && ((frame->flags & CAN_FRAME_RTR) == 0U)) { + continue; + } +#endif /* CONFIG_CAN_ACCEPT_RTR */ rxMsg.ident = frame->id; rxMsg.DLC = frame->dlc; memcpy(rxMsg.data, frame->data, frame->dlc); @@ -310,7 +315,18 @@ CO_ReturnError_t CO_CANrxBufferInit(CO_CANmodule_t *CANmodule, uint16_t index, buffer->ident = ident; buffer->mask = mask; - filter.flags = (rtr ? CAN_FILTER_RTR : CAN_FILTER_DATA); +#ifndef CONFIG_CAN_ACCEPT_RTR + if (rtr) { + LOG_ERR("request for RTR frames, but RTR frames are rejected"); + CO_errorReport(CANmodule->em, CO_EM_GENERIC_SOFTWARE_ERROR, + CO_EMC_SOFTWARE_INTERNAL, 0); + return CO_ERROR_ILLEGAL_ARGUMENT; + } +#else /* !CONFIG_CAN_ACCEPT_RTR */ + buffer->rtr = rtr; +#endif /* CONFIG_CAN_ACCEPT_RTR */ + + filter.flags = 0U; filter.id = ident; filter.mask = mask; diff --git a/modules/canopennode/CO_driver_target.h b/modules/canopennode/CO_driver_target.h index 27b1f1caa9..5dba56ef80 100644 --- a/modules/canopennode/CO_driver_target.h +++ b/modules/canopennode/CO_driver_target.h @@ -69,6 +69,9 @@ typedef struct canopen_rx { CO_CANrxBufferCallback_t pFunct; uint16_t ident; uint16_t mask; +#ifdef CONFIG_CAN_ACCEPT_RTR + bool rtr; +#endif /* CONFIG_CAN_ACCEPT_RTR */ } CO_CANrx_t; typedef struct canopen_tx { diff --git a/samples/drivers/can/counter/src/main.c b/samples/drivers/can/counter/src/main.c index 78048308ca..2119069e43 100644 --- a/samples/drivers/can/counter/src/main.c +++ b/samples/drivers/can/counter/src/main.c @@ -63,7 +63,7 @@ void rx_thread(void *arg1, void *arg2, void *arg3) ARG_UNUSED(arg2); ARG_UNUSED(arg3); const struct can_filter filter = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = COUNTER_MSG_ID, .mask = CAN_EXT_ID_MASK }; @@ -76,6 +76,10 @@ void rx_thread(void *arg1, void *arg2, void *arg3) while (1) { k_msgq_get(&counter_msgq, &frame, K_FOREVER); + if (IS_ENABLED(CONFIG_CAN_ACCEPT_RTR) && (frame.flags & CAN_FRAME_RTR) != 0U) { + continue; + } + if (frame.dlc != 2U) { printf("Wrong data length: %u\n", frame.dlc); continue; @@ -92,6 +96,10 @@ void change_led_work_handler(struct k_work *work) int ret; while (k_msgq_get(&change_led_msgq, &frame, K_NO_WAIT) == 0) { + if (IS_ENABLED(CONFIG_CAN_ACCEPT_RTR) && (frame.flags & CAN_FRAME_RTR) != 0U) { + continue; + } + if (led.port == NULL) { printf("LED %s\n", frame.data[0] == SET_LED ? "ON" : "OFF"); } else { @@ -192,7 +200,7 @@ void state_change_callback(const struct device *dev, enum can_state state, int main(void) { const struct can_filter change_led_filter = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = LED_MSG_ID, .mask = CAN_STD_ID_MASK }; diff --git a/samples/net/sockets/can/src/main.c b/samples/net/sockets/can/src/main.c index 506157c6af..8bd51984a6 100644 --- a/samples/net/sockets/can/src/main.c +++ b/samples/net/sockets/can/src/main.c @@ -32,7 +32,7 @@ static struct k_thread rx_data; #define CLOSE_PERIOD 15 static const struct can_filter zfilter = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = 0x1, .mask = CAN_STD_ID_MASK }; diff --git a/subsys/canbus/isotp/isotp.c b/subsys/canbus/isotp/isotp.c index 233cc0b2ee..b36665a277 100644 --- a/subsys/canbus/isotp/isotp.c +++ b/subsys/canbus/isotp/isotp.c @@ -54,7 +54,7 @@ static inline void prepare_filter(struct can_filter *filter, struct isotp_msg_id { filter->id = addr->ext_id; filter->mask = mask; - filter->flags = CAN_FILTER_DATA | ((addr->flags & ISOTP_MSG_IDE) != 0 ? CAN_FILTER_IDE : 0); + filter->flags = (addr->flags & ISOTP_MSG_IDE) != 0 ? CAN_FILTER_IDE : 0; } /* @@ -557,6 +557,10 @@ static void receive_can_rx(const struct device *dev, struct can_frame *frame, vo ARG_UNUSED(dev); + if (IS_ENABLED(CONFIG_CAN_ACCEPT_RTR) && (frame->flags & CAN_FRAME_RTR) != 0U) { + return; + } + switch (rctx->state) { case ISOTP_RX_STATE_WAIT_FF_SF: __ASSERT_NO_MSG(rctx->buf); @@ -847,6 +851,10 @@ static void send_can_rx_cb(const struct device *dev, struct can_frame *frame, vo ARG_UNUSED(dev); + if (IS_ENABLED(CONFIG_CAN_ACCEPT_RTR) && (frame->flags & CAN_FRAME_RTR) != 0U) { + return; + } + if (sctx->state == ISOTP_TX_WAIT_FC) { k_timer_stop(&sctx->timer); send_process_fc(sctx, frame); diff --git a/tests/drivers/can/api/src/classic.c b/tests/drivers/can/api/src/classic.c index 34a9b42dc1..3259f300b2 100644 --- a/tests/drivers/can/api/src/classic.c +++ b/tests/drivers/can/api/src/classic.c @@ -314,6 +314,8 @@ static void send_receive(const struct can_filter *filter1, int err; filter_id_1 = add_rx_msgq(can_dev, filter1); + zassert_not_equal(filter_id_1, -ENOSPC, "no filters available"); + zassert_true(filter_id_1 >= 0, "negative filter number"); send_test_frame(can_dev, frame1); err = k_msgq_get(&can_msgq, &frame_buffer, TEST_RECEIVE_TIMEOUT); @@ -360,6 +362,12 @@ static void send_receive(const struct can_filter *filter1, } } + zassert_not_equal(filter_id_1, -ENOSPC, "no filters available"); + zassert_true(filter_id_1 >= 0, "negative filter number"); + + zassert_not_equal(filter_id_2, -ENOSPC, "no filters available"); + zassert_true(filter_id_2 >= 0, "negative filter number"); + err = k_sem_take(&rx_callback_sem, TEST_RECEIVE_TIMEOUT); zassert_equal(err, 0, "receive timeout"); @@ -385,8 +393,7 @@ static void send_receive(const struct can_filter *filter1, * @param data_frame CAN data frame * @param rtr_frame CAN RTR frame */ -void send_receive_rtr(const struct can_filter *data_filter, - const struct can_filter *rtr_filter, +void send_receive_rtr(const struct can_filter *filter, const struct can_frame *data_frame, const struct can_frame *rtr_frame) { @@ -394,36 +401,24 @@ void send_receive_rtr(const struct can_filter *data_filter, int filter_id; int err; - filter_id = can_add_rx_filter_msgq(can_dev, &can_msgq, rtr_filter); - if (filter_id == -ENOTSUP) { - /* Not all CAN controller drivers support remote transmission requests */ - ztest_test_skip(); - } - + filter_id = can_add_rx_filter_msgq(can_dev, &can_msgq, filter); zassert_not_equal(filter_id, -ENOSPC, "no filters available"); zassert_true(filter_id >= 0, "negative filter number"); - /* Verify that RTR filter does not match data frame */ - send_test_frame(can_dev, data_frame); - err = k_msgq_get(&can_msgq, &frame, TEST_RECEIVE_TIMEOUT); - zassert_equal(err, -EAGAIN, "Data frame passed RTR filter"); + /* Verify that filter matches RTR frame */ + err = can_send(can_dev, rtr_frame, TEST_SEND_TIMEOUT, NULL, NULL); + if (err == -ENOTSUP) { + /* Not all drivers support transmission of RTR frames */ + can_remove_rx_filter(can_dev, filter_id); + ztest_test_skip(); + } + zassert_equal(err, 0, "failed to send RTR frame (err %d)", err); - /* Verify that RTR filter matches RTR frame */ - send_test_frame(can_dev, rtr_frame); err = k_msgq_get(&can_msgq, &frame, TEST_RECEIVE_TIMEOUT); zassert_equal(err, 0, "receive timeout"); assert_frame_equal(&frame, rtr_frame, 0); - can_remove_rx_filter(can_dev, filter_id); - - filter_id = add_rx_msgq(can_dev, data_filter); - - /* Verify that data filter does not match RTR frame */ - send_test_frame(can_dev, rtr_frame); - err = k_msgq_get(&can_msgq, &frame, TEST_RECEIVE_TIMEOUT); - zassert_equal(err, -EAGAIN, "RTR frame passed data filter"); - - /* Verify that data filter matches data frame */ + /* Verify that filter matches data frame */ send_test_frame(can_dev, data_frame); err = k_msgq_get(&can_msgq, &frame, TEST_RECEIVE_TIMEOUT); zassert_equal(err, 0, "receive timeout"); @@ -573,7 +568,7 @@ static void add_remove_max_filters(bool ide) { uint32_t id_mask = ide ? CAN_EXT_ID_MASK : CAN_STD_ID_MASK; struct can_filter filter = { - .flags = CAN_FILTER_DATA | (ide ? CAN_FILTER_IDE : 0), + .flags = (ide ? CAN_FILTER_IDE : 0), .id = 0, .mask = id_mask, }; @@ -737,8 +732,9 @@ ZTEST_USER(can_classic, test_send_receive_msgq) */ ZTEST_USER(can_classic, test_send_receive_std_id_rtr) { - send_receive_rtr(&test_std_filter_1, &test_std_rtr_filter_1, - &test_std_frame_1, &test_std_rtr_frame_1); + Z_TEST_SKIP_IFNDEF(CONFIG_CAN_ACCEPT_RTR); + + send_receive_rtr(&test_std_filter_1, &test_std_frame_1, &test_std_rtr_frame_1); } /** @@ -746,8 +742,63 @@ ZTEST_USER(can_classic, test_send_receive_std_id_rtr) */ ZTEST_USER(can_classic, test_send_receive_ext_id_rtr) { - send_receive_rtr(&test_ext_filter_1, &test_ext_rtr_filter_1, - &test_ext_frame_1, &test_ext_rtr_frame_1); + Z_TEST_SKIP_IFNDEF(CONFIG_CAN_ACCEPT_RTR); + + send_receive_rtr(&test_ext_filter_1, &test_ext_frame_1, &test_ext_rtr_frame_1); +} + +/** + * @brief Test rejection of standard (11-bit) CAN IDs and remote transmission request (RTR). + */ +ZTEST_USER(can_classic, test_reject_std_id_rtr) +{ + struct can_frame frame_buffer; + int filter_id; + int err; + + Z_TEST_SKIP_IFDEF(CONFIG_CAN_ACCEPT_RTR); + + filter_id = add_rx_msgq(can_dev, &test_std_filter_1); + + err = can_send(can_dev, &test_std_rtr_frame_1, TEST_SEND_TIMEOUT, NULL, NULL); + if (err == -ENOTSUP) { + /* Not all drivers support transmission of RTR frames */ + can_remove_rx_filter(can_dev, filter_id); + ztest_test_skip(); + } + zassert_equal(err, 0, "failed to send RTR frame (err %d)", err); + + err = k_msgq_get(&can_msgq, &frame_buffer, TEST_RECEIVE_TIMEOUT); + zassert_equal(err, -EAGAIN, "received a frame that should be rejected"); + + can_remove_rx_filter(can_dev, filter_id); +} + +/** + * @brief Test rejection of extended (29-bit) CAN IDs and remote transmission request (RTR). + */ +ZTEST_USER(can_classic, test_reject_ext_id_rtr) +{ + struct can_frame frame_buffer; + int filter_id; + int err; + + Z_TEST_SKIP_IFDEF(CONFIG_CAN_ACCEPT_RTR); + + filter_id = add_rx_msgq(can_dev, &test_ext_filter_1); + + err = can_send(can_dev, &test_ext_rtr_frame_1, TEST_SEND_TIMEOUT, NULL, NULL); + if (err == -ENOTSUP) { + /* Not all drivers support transmission of RTR frames */ + can_remove_rx_filter(can_dev, filter_id); + ztest_test_skip(); + } + zassert_equal(err, 0, "failed to send RTR frame (err %d)", err); + + err = k_msgq_get(&can_msgq, &frame_buffer, TEST_RECEIVE_TIMEOUT); + zassert_equal(err, -EAGAIN, "received a frame that should be rejected"); + + can_remove_rx_filter(can_dev, filter_id); } /** @@ -764,7 +815,7 @@ ZTEST(can_classic, test_send_receive_wrong_id) send_test_frame(can_dev, &test_std_frame_2); err = k_msgq_get(&can_msgq, &frame_buffer, TEST_RECEIVE_TIMEOUT); - zassert_equal(err, -EAGAIN, "recevied a frame that should not pass the filter"); + zassert_equal(err, -EAGAIN, "received a frame that should not pass the filter"); can_remove_rx_filter(can_dev, filter_id); } diff --git a/tests/drivers/can/api/src/common.c b/tests/drivers/can/api/src/common.c index 759733d3e8..b38090690a 100644 --- a/tests/drivers/can/api/src/common.c +++ b/tests/drivers/can/api/src/common.c @@ -74,7 +74,7 @@ const struct can_frame test_std_rtr_frame_1 = { * @brief Extended (29-bit) CAN ID RTR frame 1. */ const struct can_frame test_ext_rtr_frame_1 = { - .flags = CAN_FRAME_IDE | CAN_FILTER_RTR, + .flags = CAN_FRAME_IDE | CAN_FRAME_RTR, .id = TEST_CAN_EXT_ID_1, .dlc = 0, .data = {0} @@ -113,7 +113,7 @@ const struct can_frame test_std_fdf_frame_2 = { * ``test_std_frame_1``. */ const struct can_filter test_std_filter_1 = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = TEST_CAN_STD_ID_1, .mask = CAN_STD_ID_MASK }; @@ -123,7 +123,7 @@ const struct can_filter test_std_filter_1 = { * ``test_std_frame_2``. */ const struct can_filter test_std_filter_2 = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = TEST_CAN_STD_ID_2, .mask = CAN_STD_ID_MASK }; @@ -133,7 +133,7 @@ const struct can_filter test_std_filter_2 = { * ``test_std_frame_1``. */ const struct can_filter test_std_masked_filter_1 = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = TEST_CAN_STD_MASK_ID_1, .mask = TEST_CAN_STD_MASK }; @@ -143,7 +143,7 @@ const struct can_filter test_std_masked_filter_1 = { * ``test_std_frame_2``. */ const struct can_filter test_std_masked_filter_2 = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = TEST_CAN_STD_MASK_ID_2, .mask = TEST_CAN_STD_MASK }; @@ -153,7 +153,7 @@ const struct can_filter test_std_masked_filter_2 = { * ``test_ext_frame_1``. */ const struct can_filter test_ext_filter_1 = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = TEST_CAN_EXT_ID_1, .mask = CAN_EXT_ID_MASK }; @@ -163,7 +163,7 @@ const struct can_filter test_ext_filter_1 = { * ``test_ext_frame_2``. */ const struct can_filter test_ext_filter_2 = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = TEST_CAN_EXT_ID_2, .mask = CAN_EXT_ID_MASK }; @@ -173,7 +173,7 @@ const struct can_filter test_ext_filter_2 = { * ``test_ext_frame_1``. */ const struct can_filter test_ext_masked_filter_1 = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = TEST_CAN_EXT_MASK_ID_1, .mask = TEST_CAN_EXT_MASK }; @@ -183,37 +183,17 @@ const struct can_filter test_ext_masked_filter_1 = { * ``test_ext_frame_2``. */ const struct can_filter test_ext_masked_filter_2 = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = TEST_CAN_EXT_MASK_ID_2, .mask = TEST_CAN_EXT_MASK }; -/** - * @brief Standard (11-bit) CAN ID RTR filter 1. This filter matches - * ``test_std_rtr_frame_1``. - */ -const struct can_filter test_std_rtr_filter_1 = { - .flags = CAN_FILTER_RTR, - .id = TEST_CAN_STD_ID_1, - .mask = CAN_STD_ID_MASK -}; - -/** - * @brief Extended (29-bit) CAN ID RTR filter 1. This filter matches - * ``test_ext_rtr_frame_1``. - */ -const struct can_filter test_ext_rtr_filter_1 = { - .flags = CAN_FILTER_RTR | CAN_FILTER_IDE, - .id = TEST_CAN_EXT_ID_1, - .mask = CAN_EXT_ID_MASK -}; - /** * @brief Standard (11-bit) CAN ID filter. This filter matches * ``TEST_CAN_SOME_STD_ID``. */ const struct can_filter test_std_some_filter = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = TEST_CAN_SOME_STD_ID, .mask = CAN_STD_ID_MASK }; diff --git a/tests/drivers/can/api/src/common.h b/tests/drivers/can/api/src/common.h index 374141a71f..af192f574c 100644 --- a/tests/drivers/can/api/src/common.h +++ b/tests/drivers/can/api/src/common.h @@ -144,18 +144,6 @@ extern const struct can_filter test_ext_masked_filter_1; */ extern const struct can_filter test_ext_masked_filter_2; -/** - * @brief Standard (11-bit) CAN ID RTR filter 1. This filter matches - * ``test_std_rtr_frame_1``. - */ -extern const struct can_filter test_std_rtr_filter_1; - -/** - * @brief Extended (29-bit) CAN ID RTR filter 1. This filter matches - * ``test_ext_rtr_frame_1``. - */ -extern const struct can_filter test_ext_rtr_filter_1; - /** * @brief Standard (11-bit) CAN ID filter. This filter matches * ``TEST_CAN_SOME_STD_ID``. diff --git a/tests/drivers/can/api/src/utilities.c b/tests/drivers/can/api/src/utilities.c index c2e037bcbc..5fd251f596 100644 --- a/tests/drivers/can/api/src/utilities.c +++ b/tests/drivers/can/api/src/utilities.c @@ -67,7 +67,7 @@ ZTEST(can_utilities, test_can_bytes_to_dlc) ZTEST(can_utilities, test_can_frame_matches_filter) { const struct can_filter test_ext_filter_std_id_1 = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = TEST_CAN_STD_ID_1, .mask = CAN_EXT_ID_MASK }; @@ -105,19 +105,9 @@ ZTEST(can_utilities, test_can_frame_matches_filter) zassert_false(can_frame_matches_filter(&test_ext_frame_1, &test_std_masked_filter_1)); zassert_false(can_frame_matches_filter(&test_ext_frame_2, &test_std_masked_filter_2)); - /* Remote transmission request (RTR) frames and filters */ - zassert_true(can_frame_matches_filter(&test_std_rtr_frame_1, &test_std_rtr_filter_1)); - zassert_true(can_frame_matches_filter(&test_ext_rtr_frame_1, &test_ext_rtr_filter_1)); - zassert_false(can_frame_matches_filter(&test_std_rtr_frame_1, &test_ext_rtr_filter_1)); - zassert_false(can_frame_matches_filter(&test_ext_rtr_frame_1, &test_std_rtr_filter_1)); - - /* Remote transmission request (RTR) frames and non-RTR filters */ - zassert_false(can_frame_matches_filter(&test_std_rtr_frame_1, &test_std_filter_1)); - zassert_false(can_frame_matches_filter(&test_ext_rtr_frame_1, &test_ext_filter_1)); - - /* Non-RTR frames and Remote transmission request (RTR) filters */ - zassert_false(can_frame_matches_filter(&test_std_frame_1, &test_std_rtr_filter_1)); - zassert_false(can_frame_matches_filter(&test_ext_frame_1, &test_ext_rtr_filter_1)); + /* Remote transmission request (RTR) frames */ + zassert_true(can_frame_matches_filter(&test_std_rtr_frame_1, &test_std_filter_1)); + zassert_true(can_frame_matches_filter(&test_ext_rtr_frame_1, &test_ext_filter_1)); /* CAN FD format frames and filters */ zassert_true(can_frame_matches_filter(&test_std_fdf_frame_1, &test_std_filter_1)); diff --git a/tests/drivers/can/api/testcase.yaml b/tests/drivers/can/api/testcase.yaml index f24a25c15a..11c72526a8 100644 --- a/tests/drivers/can/api/testcase.yaml +++ b/tests/drivers/can/api/testcase.yaml @@ -1,14 +1,16 @@ +common: + tags: + - drivers + - can + depends_on: can tests: drivers.can.api: - tags: - - drivers - - can - depends_on: can filter: dt_chosen_enabled("zephyr,canbus") and not dt_compat_enabled("kvaser,pcican") + drivers.can.api.rtr: + filter: dt_chosen_enabled("zephyr,canbus") and not dt_compat_enabled("kvaser,pcican") + extra_configs: + - CONFIG_CAN_ACCEPT_RTR=y drivers.can.api.twai: - tags: - - drivers - - can extra_args: DTC_OVERLAY_FILE=twai-enable.overlay filter: dt_compat_enabled("espressif,esp32-twai") platform_allow: diff --git a/tests/drivers/can/shell/src/main.c b/tests/drivers/can/shell/src/main.c index 61bd80ec72..35681839bd 100644 --- a/tests/drivers/can/shell/src/main.c +++ b/tests/drivers/can/shell/src/main.c @@ -487,7 +487,7 @@ static void can_shell_test_filter_add(const char *cmd, const struct can_filter * ZTEST(can_shell, test_can_filter_add_std_id) { struct can_filter expected = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = 0x010, .mask = CAN_STD_ID_MASK, }; @@ -498,7 +498,7 @@ ZTEST(can_shell, test_can_filter_add_std_id) ZTEST(can_shell, test_can_filter_add_std_id_mask) { struct can_filter expected = { - .flags = CAN_FILTER_DATA, + .flags = 0U, .id = 0x010, .mask = 0x020, }; @@ -509,7 +509,7 @@ ZTEST(can_shell, test_can_filter_add_std_id_mask) ZTEST(can_shell, test_can_filter_add_ext_id) { struct can_filter expected = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = 0x1024, .mask = CAN_EXT_ID_MASK, }; @@ -520,7 +520,7 @@ ZTEST(can_shell, test_can_filter_add_ext_id) ZTEST(can_shell, test_can_filter_add_ext_id_mask) { struct can_filter expected = { - .flags = CAN_FILTER_DATA | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = 0x1024, .mask = 0x2048, }; @@ -528,37 +528,15 @@ ZTEST(can_shell, test_can_filter_add_ext_id_mask) can_shell_test_filter_add("can filter add " FAKE_CAN_NAME " -e 1024 2048", &expected); } -ZTEST(can_shell, test_can_filter_add_rtr) -{ - struct can_filter expected = { - .flags = CAN_FILTER_DATA | CAN_FILTER_RTR, - .id = 0x022, - .mask = CAN_STD_ID_MASK, - }; - - can_shell_test_filter_add("can filter add " FAKE_CAN_NAME " -r 022", &expected); -} - -ZTEST(can_shell, test_can_filter_add_rtr_only) -{ - struct can_filter expected = { - .flags = CAN_FILTER_RTR, - .id = 0x322, - .mask = CAN_STD_ID_MASK, - }; - - can_shell_test_filter_add("can filter add " FAKE_CAN_NAME " -R 322", &expected); -} - ZTEST(can_shell, test_can_filter_add_all_options) { struct can_filter expected = { - .flags = CAN_FILTER_RTR | CAN_FILTER_IDE, + .flags = CAN_FILTER_IDE, .id = 0x2048, .mask = 0x4096, }; - can_shell_test_filter_add("can filter add " FAKE_CAN_NAME " -e -r -R 2048 4096", &expected); + can_shell_test_filter_add("can filter add " FAKE_CAN_NAME " -e 2048 4096", &expected); } ZTEST(can_shell, test_can_filter_remove_missing_value) diff --git a/tests/net/socket/can/src/main.c b/tests/net/socket/can/src/main.c index 02d209c022..040f03d1e3 100644 --- a/tests/net/socket/can/src/main.c +++ b/tests/net/socket/can/src/main.c @@ -101,10 +101,10 @@ ZTEST(socket_can, test_socketcan_filter_to_can_filter) struct can_filter expected = { 0 }; struct can_filter zfilter = { 0 }; - sfilter.can_id = BIT(31) | BIT(30) | 1234; - sfilter.can_mask = BIT(31) | BIT(30) | 1234; + sfilter.can_id = BIT(31) | 1234; + sfilter.can_mask = BIT(31) | 1234; - expected.flags = CAN_FILTER_IDE | CAN_FILTER_RTR; + expected.flags = CAN_FILTER_IDE; expected.id = 1234U; expected.mask = 1234U; @@ -128,10 +128,13 @@ ZTEST(socket_can, test_can_filter_to_socketcan_filter) struct socketcan_filter expected = { 0 }; struct can_filter zfilter = { 0 }; - expected.can_id = BIT(31) | BIT(30) | 1234; - expected.can_mask = BIT(31) | BIT(30) | 1234; + expected.can_id = BIT(31) | 1234; + expected.can_mask = BIT(31) | 1234; +#ifndef CONFIG_CAN_ACCEPT_RTR + expected.can_mask |= BIT(30); +#endif /* !CONFIG_CAN_ACCEPT_RTR */ - zfilter.flags = CAN_FILTER_IDE | CAN_FILTER_RTR; + zfilter.flags = CAN_FILTER_IDE; zfilter.id = 1234U; zfilter.mask = 1234U; diff --git a/tests/subsys/canbus/isotp/conformance/src/main.c b/tests/subsys/canbus/isotp/conformance/src/main.c index 926742da5d..bd4a7b52de 100644 --- a/tests/subsys/canbus/isotp/conformance/src/main.c +++ b/tests/subsys/canbus/isotp/conformance/src/main.c @@ -306,7 +306,7 @@ static int add_rx_msgq(uint32_t id, uint32_t mask) { int filter_id; struct can_filter filter = { - .flags = CAN_FILTER_DATA | ((id > 0x7FF) ? CAN_FILTER_IDE : 0), + .flags = (id > 0x7FF) ? CAN_FILTER_IDE : 0, .id = id, .mask = mask };