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.

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

RFC: Breaking API Change: drivers: can: remove run-time RTR filtering, add build-time RTR filter #67127

Conversation

henrikbrixandersen
Copy link
Member

@henrikbrixandersen henrikbrixandersen commented Jan 2, 2024

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.

RFC: #67128

@henrikbrixandersen henrikbrixandersen added RFC Request For Comments: want input from the community area: CAN Breaking API Change Breaking changes to stable, public APIs labels Jan 2, 2024
@henrikbrixandersen henrikbrixandersen force-pushed the can_remove_rtr_runtime_filter branch 2 times, most recently from 930a20a to 728ae88 Compare January 2, 2024 22:25
@henrikbrixandersen henrikbrixandersen self-assigned this Jan 3, 2024
@henrikbrixandersen henrikbrixandersen changed the title RFC: drivers: can: remove run-time RTR filtering, add build-time RTR filter RFC: Breaking API Change: drivers: can: remove run-time RTR filtering, add build-time RTR filter Jan 4, 2024
@henrikbrixandersen henrikbrixandersen added area: API Changes to public APIs Release Notes To be mentioned in the release notes labels Jan 5, 2024
@henrikbrixandersen henrikbrixandersen added this to the v3.6.0 milestone Jan 10, 2024
@henrikbrixandersen henrikbrixandersen force-pushed the can_remove_rtr_runtime_filter branch 2 times, most recently from 7ba4763 to a422b39 Compare January 15, 2024 21:23
@henrikbrixandersen henrikbrixandersen marked this pull request as ready for review January 15, 2024 21:26
@henrikbrixandersen henrikbrixandersen added the Architecture Review Discussion in the Architecture WG required label Jan 15, 2024
@carlescufi
Copy link
Member

Architecture WG:

  • No objections

@henrikbrixandersen henrikbrixandersen removed the Architecture Review Discussion in the Architecture WG required label Jan 16, 2024
@henrikbrixandersen
Copy link
Member Author

This PR is ready for review.

@henrikbrixandersen
Copy link
Member Author

Rebased to fix merge conflict after 9eb0a55.

martinjaeger
martinjaeger previously approved these changes Jan 18, 2024
Copy link
Member

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks a lot for your effort to fix this!

@henrikbrixandersen
Copy link
Member Author

Rebased on main to resolve a bunch of merge conflicts from #65108 and #67743.

tests/drivers/can/api/src/common.c Outdated Show resolved Hide resolved
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 <hebad@vestas.com>
Add a note about the removal of the CAN_FILTER_DATA and CAN_FILTER_RTR
flags from the CAN controller driver API.

Signed-off-by: Henrik Brix Andersen <hebad@vestas.com>
@henrikbrixandersen henrikbrixandersen merged commit 53da5f6 into zephyrproject-rtos:main Jan 21, 2024
22 checks passed
@henrikbrixandersen henrikbrixandersen deleted the can_remove_rtr_runtime_filter branch January 21, 2024 10:04
sasataku added a commit to sasataku/libcsp that referenced this pull request Jan 24, 2024
In Zephyr, with PR below, `CAN_FILTER_DATA` and `CAN_FILTER_RTR`
have been removed.

  - zephyrproject-rtos/zephyr#67127

Originally, `CAN_FILTER_DATA` was specified explicitly to filter CAN
Data Frames, but in reality, in the Zephyr driver, it is possible
to filter CAN Data Frames without specifying `CAN_FILTER_DATA`.
Therefore, in this commit, `CAN_FILTER_DATA` is removed.

Signed-off-by: Takuya Sasaki <takuya.sasaki@spacecubics.com>
sasataku added a commit to sasataku/libcsp that referenced this pull request Jan 24, 2024
In Zephyr, with PR below, `CAN_FILTER_DATA` and `CAN_FILTER_RTR`
have been removed.

  - zephyrproject-rtos/zephyr#67127

Originally, `CAN_FILTER_DATA` was specified explicitly to filter CAN
Data Frames, but in reality, in the Zephyr driver, it is possible
to filter CAN Data Frames without specifying `CAN_FILTER_DATA`.
Therefore, in this commit, `CAN_FILTER_DATA` is removed.

Signed-off-by: Takuya Sasaki <takuya.sasaki@spacecubics.com>
yashi pushed a commit to libcsp/libcsp that referenced this pull request Jan 25, 2024
In Zephyr, with PR below, `CAN_FILTER_DATA` and `CAN_FILTER_RTR`
have been removed.

  - zephyrproject-rtos/zephyr#67127

Originally, `CAN_FILTER_DATA` was specified explicitly to filter CAN
Data Frames, but in reality, in the Zephyr driver, it is possible
to filter CAN Data Frames without specifying `CAN_FILTER_DATA`.
Therefore, in this commit, `CAN_FILTER_DATA` is removed.

Signed-off-by: Takuya Sasaki <takuya.sasaki@spacecubics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: CAN Breaking API Change Breaking changes to stable, public APIs Release Notes To be mentioned in the release notes RFC Request For Comments: want input from the community
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants