-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add support for Xilinx CANFD #97341
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
base: main
Are you sure you want to change the base?
Add support for Xilinx CANFD #97341
Conversation
The Xilinx CANFD enhances traditional CAN communication by supporting higher data rates and improved efficiency, making it suitable for applications that require real-time data transfer, such as automotive and industrial systems. Xilinx CANFD supports data rates of up to 8 Mbps, allowing for faster communication compared to standard CAN. The protocol supports longer data frames, enabling the transmission of larger data packets and compatible with both classical CAN and CANFD protocols. Signed-off-by: Sai Priya M <[email protected]>
Add support for Xilinx CANFD with CAN and CANFD functionality. Features: -Classic CAN 2.0A/B support (11-bit and 29-bit IDs). -CANFD support with up to 64-byte data frames. -Multiple operating modes: Normal, Loopback, Listen-only, Sleep. -Configurable bit timing for arbitration and data phases. -Single TX mailbox with interrupt-driven transmission. -RX FIFO with multi-frame buffering. -Comprehensive error handling and state management. -Thread-safe operation with mutex protection. -Power management with sleep/wake-up support. Signed-off-by: Sai Priya M <[email protected]>
Add CANFD nodes to device tree for Versal-NET platform. Signed-off-by: Sai Priya M <[email protected]>
Add support for CANFD in Versal-NET RPU platform. Signed-off-by: Sai Priya M <[email protected]>
Add conditional compilation to handle both classic CAN and CANFD frames when running on Xilinx CANFD. Add CONFIG_CAN_FD_MODE and CONFIG_LOOPBACK_MODE to can_xilinx_canfd.conf Signed-off-by: Sai Priya M <[email protected]>
Add CONFIG_XILINX_CANFD guards to disable test cases for features not supported by the Xilinx CANFD driver: - Acceptance filter tests. - Recovery API tests. Signed-off-by: Sai Priya M <[email protected]>
Hello @smuthoju, and thank you very much for your first pull request to the Zephyr project! |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not read the entire code so far... just too many issues.
|
||
#include "common.h" | ||
|
||
#ifndef CONFIG_CAN_XILINX_CANFD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not modify any tests, but make the driver pass the test.
The tests are verifying if the driver fulfills the contract of the api. If the test does not pass, the driver cannot go in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Please address this and the remaining comments from @alexanderwachter.
filter_id = can_add_rx_filter_msgq(can_dev, &counter_msgq, &filter); | ||
printf("Counter filter id: %d\n", filter_id); | ||
|
||
#if CONFIG_CAN_XILINX_CANFD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not modify the sample code. This code needs to be able to run on any platform!
@@ -0,0 +1,2 @@ | |||
CONFIG_CAN_FD_MODE=y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No device specific config for samples!
*/ | ||
|
||
/ { | ||
soc: soc { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only can specific changes in this PR
|
||
config CAN_XILINX_CANFD | ||
bool "Xilinx CANFD driver" | ||
default y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on dts node okay missing
(dwindex * XCANFD_DW_BYTES); | ||
reg_data = xcanfd_read32(config, dw_offset); | ||
reg_data = BSWAP_32(reg_data); | ||
copy_len = (len - i) > 4 ? 4 : (len - i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MAX()?
|
||
if (len > 0 && len <= sizeof(frame.data)) { | ||
for (int i = 0; i < len; i += 4) { | ||
dw_offset = XCANFD_FRAME_DW_OFFSET(offset) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why such a long calculation?
Just enhance i with the DW size?
|
||
if (data->rx_callback != NULL) { | ||
data->rx_callback(dev, &frame, data->rx_callback_arg); | ||
frame_consumed = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for frame_consumed
struct xcanfd_data *data = dev->data; | ||
uint32_t isr; | ||
|
||
if (dev == NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How should this ever happen?
|
||
if (isr & XCANFD_IXR_ARBLST_MASK) { | ||
xcanfd_write32(config, XCANFD_ICR_OFFSET, XCANFD_IXR_ARBLST_MASK); | ||
k_mutex_lock(&data->inst_mutex, K_FOREVER); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mutex in an isr?
This patch set does the following: