drivers: can: add Realtek Bee series support#105411
Conversation
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
2 times, most recently
from
d124069 to
aba1b77
Compare
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
aba1b77 to
ab33cff
Compare
|
The following west manifest projects have changed revision in this Pull Request:
Additional metadata changed:
⛔ DNM label due to: 1 project with PR revision, 1 project with metadata changes and 3 blob changes Note: This message is automatically posted and updated by the Manifest GitHub Action. |
github-actions
Bot
added
manifest
manifest-hal_realtek
DNM (manifest)
|
|
This pull request has been marked as stale because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 7 days. Note, that you can always re-open a closed pull request at any time. |
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
eb4489c to
7197893
Compare
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
7197893 to
492973a
Compare
zephyrbot
requested review from
57300,
JiafeiPan,
JordanYates,
Vge0rge,
axelnxp,
blauret,
karstenkoenig,
kl-cruz,
lauracarlesso,
manuargue,
nhutnguyenkc,
ssekar15,
str4t0m and
wmrsouza
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
2 times, most recently
from
481f5af to
5e06cb1
Compare
|
Hello, @henrikbrixandersen if you have some time, I’d really appreciate it if you could review this PR. Thanks! |
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
5e06cb1 to
a2f8097
Compare
|
Rebased to resolve conflicts. |
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
a2f8097 to
286b4f4
Compare
henrikbrixandersen
left a comment
henrikbrixandersen
left a comment
There was a problem hiding this comment.
Please add an entry for this driver to https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/build_all/can/testcase.yaml
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
286b4f4 to
d4d2104
Compare
|
Added. Please continue the review, and feel free to let me know if any further changes are needed. Thanks! |
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
d4d2104 to
683a046
Compare
|
Rebased to resolve conflicts. |
|
|
Hi, @henrikbrixandersen Sorry to ping you again. This PR has been open for a while. If you have time, could you please continue reviewing this PR? Thanks! |
Hi, yes - my apologies for the delay. I have set aside time for reviewing this and a bunch of other CAN driver additions early next week. |
No worries at all. Thanks a lot for the update. |
| default y | ||
| depends on DT_HAS_REALTEK_BEE_CAN_ENABLED | ||
| select HAL_REALTEK_BEE_CAN | ||
| select CAN_ACCEPT_RTR |
There was a problem hiding this comment.
This should not be selected by the driver. It should be selected by the application if needed.
There was a problem hiding this comment.
Removed, thanks~
| config CAN_BEE_MAX_STD_ID_FILTER | ||
| int "Maximum number of standard (11-bit) ID filters" | ||
| default 5 | ||
| range 0 CAN_BEE_RX_MSG_BUF_NUM | ||
| help | ||
| Defines the maximum number of filters with standard ID (11-bit) | ||
| that can be added by the application. | ||
| The tx and rx share 16 message buffers in total. | ||
| Each rx message buffer can filter one standard or extended ID. | ||
| The following equation determines the maximum total number of | ||
| filters: | ||
| CAN_BEE_MAX_STD_ID_FILTER + CAN_BEE_MAX_EXT_ID_FILTER <= CAN_BEE_RX_MSG_BUF_NUM | ||
|
|
||
| config CAN_BEE_MAX_EXT_ID_FILTER | ||
| int "Maximum number of extended (29-bit) ID filters" | ||
| default 5 | ||
| range 0 CAN_BEE_RX_MSG_BUF_NUM | ||
| help | ||
| Defines the maximum number of filters with extended ID (29-bit) | ||
| that can be added by the application. | ||
| The tx and rx share 16 message buffers in total. | ||
| Each rx message buffer can filter one standard or extended ID. | ||
| The following equation determines the maximum total number of | ||
| filters: | ||
| CAN_BEE_MAX_STD_ID_FILTER + CAN_BEE_MAX_EXT_ID_FILTER <= CAN_BEE_RX_MSG_BUF_NUM |
There was a problem hiding this comment.
Are these two really needed? Isn't CONFIG_CAN_BEE_RX_MSG_BUF_NUM sufficient?
There was a problem hiding this comment.
This is indeed redundant and has been removed. Thanks~
There was a problem hiding this comment.
Perhaps name this can_realtek_bee.c instead?
There was a problem hiding this comment.
Renamed, thanks!
| bitrate = <500000>; | ||
| sample-point = <875>; |
There was a problem hiding this comment.
These should not be set in the DTSI. Instead, rely on the defaults set via Kconfig.
There was a problem hiding this comment.
Yeah I noticed the CONFIG_CAN_DEFAULT_BITRATE in Kconfig. Removed here, thanks!
| description: | | ||
| Prescaler value for computing the rx timestamps of received messages. | ||
| The timestamp counter is derived from the CAN source clock divided by this value. | ||
| Valid range is [1, 256]. |
There was a problem hiding this comment.
Please use min and max for this.
There was a problem hiding this comment.
Updated, thanks~
| return 0; | ||
|
|
||
| fail: | ||
| LOG_ERR("CAN send failed: %d", ret); |
There was a problem hiding this comment.
You just logged the reason above.
There was a problem hiding this comment.
Removed the redundant log here, thanks~
| } | ||
| } | ||
|
|
||
| static int can_bee_start(const struct device *dev) |
There was a problem hiding this comment.
You need to reset the CAN statistics here. Check the other drivers for inspiration.
| { | ||
| ARG_UNUSED(dev); | ||
|
|
||
| if (SET == CAN_GetErrorStatus(CAN_ERROR_ACK)) { |
There was a problem hiding this comment.
Where is SET declared?
There was a problem hiding this comment.
typedef enum
{
RESET = 0,
SET = !RESET
} FlagStatus, ITStatus;
SET is defined as 1 in the HAL. In practice, true / false can also meet the requirement.
| return 0; | ||
| } | ||
|
|
||
| static DEVICE_API(can, can_bee_driver_api) = { |
There was a problem hiding this comment.
How does this core handle CAN bus recovery?
There was a problem hiding this comment.
Since the bus on/off state of this CAN controller depends on the external PHY device, manual recovery is not supported.
| .clkid = DT_INST_CLOCKS_CELL(index, id), \ | ||
| .pcfg = PINCTRL_DT_INST_DEV_CONFIG_GET(index), \ | ||
| .irq_config_func = config_can_##index##_irq, \ | ||
| .rx_timestamp_prescaler = DT_INST_PROP(index, rx_timestamp_prescaler), \ |
There was a problem hiding this comment.
You could opt to use DT_INST_PROP_OR() here and remove the default in the binding.
There was a problem hiding this comment.
Yeah I use DT_INST_PROP_OR to set the default value and removed in the binding file. Thanks!
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
2 times, most recently
from
9fefab6 to
b28cfb1
Compare
|
Hi, @henrikbrixandersen , since the SOC does not directly bind the CAN function to specific pins, users can pinmux CAN to any pins. Therefore, we did not directly enable CAN bus or specify its pins in the board DTS. This causes the CI for the “build all tests” check to fail, so I removed CAN from the board supported feature list. |
This needs to be integrated and build on at least one board in the tree. |
Okay, I will add CAN on one board, thanks! |
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
b28cfb1 to
0d465d5
Compare
|
Hi, @henrikbrixandersen ， I have updated the code and tested locally. The CI alse passed. Could you please take another look when you have time? Thanks! |
Update hal realtek drivers and add ADC/CAN driver support. Signed-off-by: Yuzhuo Liu <yuzhuo_liu@realsil.com.cn>
Add CAN driver support for Realtek Bee series SoCs, including RTL87x2G. This driver supports: - Classic CAN 2.0A/B - Standard and Extended identifiers - Remote and data frames - Transmission and Reception Signed-off-by: Yuzhuo Liu <yuzhuo_liu@realsil.com.cn>
Add build-only test for Realtek Bee CAN controller in the tree. Signed-off-by: Yuzhuo Liu <yuzhuo_liu@realsil.com.cn>
YuzhuoLiuRTK
force-pushed
the
upstream/drivers/can/add-driver
branch
from
0d465d5 to
7f6404b
Compare
|
Rebased to resolce conflicts. |
|
3 participants
Important Note: This PR depends on hal_realtek repo PR for CAN lib(zephyrproject-rtos/hal_realtek#21).
Description
This PR adds the CAN driver support for Realtek Bee series SoCs. The driver is implemented based on the standard Zephyr CAN API and supports the following series:
Key features implemented:
Testing
I have verified the driver functionality using the standard Zephyr CAN API test suite as well as a manual shell test with an external PHY and a CAN analyzer on the following hardware platform:
rtl87x2g_evb_a_rtl8762gkutests/drivers/can/apiLogs
CAN API Test Logs
Shell Test Logs