-
Notifications
You must be signed in to change notification settings - Fork 8.2k
NXP: CAN: Add LPC MCAN support #35832
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
NXP: CAN: Add LPC MCAN support #35832
Conversation
6eff5dc to
5cc5ba5
Compare
5cc5ba5 to
c1998e2
Compare
|
This pull request has been marked as stale because it has been open (more than) 60 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 14 days. Note, that you can always re-open a closed pull request at any time. |
drivers/can/can_mcan.c
Outdated
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.
So much magic ;-). Just curious but why is this done?
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.
@henrikbrixandersen what will it take to move this PR out of draft state?
I will need to revisit it. As I recall it, there were some issues with CAN-FD with the current solution.
Also, github added an STM32 tag because of the one dts file you touched. Was that your intention because it doesn't really align with the title of the PR.
One of the commits is touching the generic Bosch M-CAN driver, and thus needed to make a change to the STM32 front-end. I still need to decide if this is the right fix.
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 found the bug in the generic driver, that I was trying to work-around with the commit that touched the STM32 front-end. I have fixed it properly and removed the STM32 changes + label.
The "magic" above is to calculate the relative addresses of the various elements of the Message RAM.
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.
The addressing part is getting tricky as more front-ends are dropping in.
I propose to merge this as is (and also the new STM32H7) and refactor it.
My idea is to have a function ptr in the config structure that is called somewhere during the init.
Somehow a vendor-specific config.
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 agree.
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 propose to merge this as is (and also the new STM32H7) and refactor it.
@alexanderwachter Does that mean you approve this PR?
|
@henrikbrixandersen what will it take to move this PR out of draft state? Also, github added an STM32 tag because of the one dts file you touched. Was that your intention because it doesn't really align with the title of the PR. |
|
This pull request has been marked as stale because it has been open (more than) 60 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 14 days. Note, that you can always re-open a closed pull request at any time. |
eed70c1 to
4caa0c4
Compare
9606c4c to
dd0fa45
Compare
fc34fb2 to
f340363
Compare
alexanderwachter
left a comment
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.
LGTM. Just a little comment. Feel free to ignore it :-)
drivers/can/can_mcan.h
Outdated
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.
Can we do something like
#define MCAN_DT_PATH DT_COMPAT_GET_ANY_STATUS_OKAY(DT_DRV_COMPAT)?
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.
The current approach does not allow for different devicetree configurations of the M_CAN message RAM for multiple M_CAN frontend instances, so it needs to be reworked.
I'd prefer to leave this for the general refactoring of the M_CAN driver once the other frontend drivers are in (#41549 and #41674).
The Event FIFO start address (EFSA) field within the Tx event FIFO configuration register (TXEFC) occupies bit 15:2. Change the CAN_MCAN_TXEFC_EFSA_POS definition to reflect this. Signed-off-by: Henrik Brix Andersen <[email protected]>
Add Kconfig option for indicating that a given SoC contains the NXP MCAN CAN FD controller. Signed-off-by: Henrik Brix Andersen <[email protected]>
Add support for the LPC MCAN clock to the LPC SYSCON clock controller driver. Signed-off-by: Henrik Brix Andersen <[email protected]>
Add devicetree binding for the NXP LPC MCAN CAN-FD controller. Signed-off-by: Henrik Brix Andersen <[email protected]>
daf81de to
25ab5a5
Compare
Add a NXP LPC MCAN-specific front-end for the generic Bosch MCAN driver. Signed-off-by: Henrik Brix Andersen <[email protected]>
Add devicetree node for the NXP LPC MCAN. Signed-off-by: Henrik Brix Andersen <[email protected]>
Add support for the NXP LPC MCAN CAN-FD controller. Signed-off-by: Henrik Brix Andersen <[email protected]>
Add NXP LPC MCAN support for the NXP lpcxpresso55s16 board definition. Fixes zephyrproject-rtos#35437 Signed-off-by: Henrik Brix Andersen <[email protected]>
25ab5a5 to
21cff9b
Compare
Add a NXP LPC MCAN-specific front-end for the generic Bosch MCAN driver and enable it for the lpcxpresso55s16 board.