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.

Sign up for GitHub

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

Jump to bottom

Support Renesas ra8 canfd driver #76134

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Support Renesas ra8 canfd driver #76134

merged 1 commit into from
Oct 25, 2024

Conversation

duynguyenxa
Copy link
Member

@duynguyenxa duynguyenxa commented Jul 21, 2024 •
edited
Loading

This PR added support for CANFD driver on Renesas EK RA8M1 board

@zephyrbot
Copy link
Collaborator

zephyrbot commented Jul 21, 2024 •
edited
Loading

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hal_renesas DNM This PR should not be merged (Do Not Merge) labels Jul 21, 2024
henrikbrixandersen
henrikbrixandersen requested changes Jul 21, 2024
View reviewed changes
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

  • Please split this PR into one PR for adding the new clock control support and one for adding CAN support.
  • Please justify the addition of a board-specific CAN test suite.

@henrikbrixandersen
Copy link
Member

This PR added support for ADC driver on Renesas EK RA8M1 board

I take it you mean CAN, not ADC.

@duynguyenxa
Copy link
Member Author

This PR added support for ADC driver on Renesas EK RA8M1 board

I take it you mean CAN, not ADC.

@henrikbrixandersen , Yes, sorry I mistype the comment.

@duynguyenxa duynguyenxa force-pushed the support_renesas_ra8_canfd branch from 484649a to faa6adb Compare July 22, 2024 08:52
@ydamigos ydamigos assigned KhiemNguyenT and unassigned nordic-krch Jul 25, 2024
@ydamigos ydamigos added platform: Renesas RA Renesas Electronics Corporation, RA and removed platforms: Renesas RA labels Jul 25, 2024
@duynguyenxa
Copy link
Member Author

  • Please split this PR into one PR for adding the new clock control support and one for adding CAN support.
  • Please justify the addition of a board-specific CAN test suite.

I added a separated PR clock control driver here, it's expected that clock control driver to be merge first
#76134

@thenguyenyf thenguyenyf force-pushed the support_renesas_ra8_canfd branch from faa6adb to 6500b24 Compare August 8, 2024 11:21
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Aug 8, 2024
@henrikbrixandersen
Copy link
Member

Please rebase on main and resolve any merge conflicts.

@thaoluonguw
Copy link
Collaborator

I rebase the main branch and add 1 more commit 4fdbfda to skip the test for samples/net/prometheus due to CI pipeline build failed (description in #80121)

This is unrelated to this PR and needs to be addressed elsewhere.

@henrikbrixandersen : Thank you so much for your feedback. Could you share with us the correct action in this case?
"needs to be addressed elsewhere" mean that we have a separated PR for it. Is it correct?

This PR is failed for twister test due to new support for samples/net/prometheus, we created issue to report this at #80121 and create a commit to skip testing samples/net/prometheus for RA boards.
If current action isn't correct, we need create a new PR to skip testing samples/net/prometheus, then wait for it merge to continue with this PR. Is our understanding correct?

str4t0m
str4t0m requested changes Oct 22, 2024
View reviewed changes
Copy link
Collaborator

@str4t0m str4t0m left a comment

Choose a reason for hiding this comment

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

A few nits.

boards/renesas/ek_ra8m1/ek_ra8m1.dts Outdated Show resolved Hide resolved
drivers/can/Kconfig.renesas_ra Outdated Show resolved Hide resolved
drivers/can/can_renesas_ra.c Outdated Show resolved Hide resolved
drivers/can/can_renesas_ra.c Outdated Show resolved Hide resolved
drivers/can/can_renesas_ra.c Outdated Show resolved Hide resolved
drivers/can/can_renesas_ra.c Outdated Show resolved Hide resolved
drivers/can/can_renesas_ra.c Show resolved Hide resolved
dts/bindings/misc/renesas,ra-canfd-global.yaml Outdated Show resolved Hide resolved
@thenguyenyf thenguyenyf force-pushed the support_renesas_ra8_canfd branch from 4fdbfda to 6d7c4d4 Compare October 22, 2024 06:25
@thenguyenyf
Copy link
Contributor

Please rebase on main and resolve any merge conflicts.

Hi @henrikbrixandersen, I rebased main and solved conflicts

@thenguyenyf thenguyenyf force-pushed the support_renesas_ra8_canfd branch from 6d7c4d4 to a504e3f Compare October 22, 2024 07:54
@henrikbrixandersen
Copy link
Member

I rebase the main branch and add 1 more commit 4fdbfda to skip the test for samples/net/prometheus due to CI pipeline build failed (description in #80121)

This is unrelated to this PR and needs to be addressed elsewhere.

@henrikbrixandersen : Thank you so much for your feedback. Could you share with us the correct action in this case? "needs to be addressed elsewhere" mean that we have a separated PR for it. Is it correct?

This PR is failed for twister test due to new support for samples/net/prometheus, we created issue to report this at #80121 and create a commit to skip testing samples/net/prometheus for RA boards. If current action isn't correct, we need create a new PR to skip testing samples/net/prometheus, then wait for it merge to continue with this PR. Is our understanding correct?

Reporting it in #80121 is good, but it needs to be fixed outside of this PR - as the reviewers are different from those reviewing CAN related PRs. Thus, please remove the excluding commit from this PR.

@thenguyenyf thenguyenyf force-pushed the support_renesas_ra8_canfd branch 2 times, most recently from 5c9107b to 49b29db Compare October 24, 2024 01:31
@thenguyenyf
Copy link
Contributor

Hi @henrikbrixandersen , @str4t0m , @thaoluonguw , @KhiemNguyenT . It seems that there is no remain issue. Could you help to take a look to make this PR can be merged?

str4t0m
str4t0m reviewed Oct 24, 2024
View reviewed changes
Copy link
Collaborator

@str4t0m str4t0m left a comment

Choose a reason for hiding this comment

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

I found two more things, do you think you address them before the merge window closes?

drivers/can/can_renesas_ra.c Outdated Show resolved Hide resolved
drivers/can/can_renesas_ra.c Outdated Show resolved Hide resolved
@thenguyenyf thenguyenyf force-pushed the support_renesas_ra8_canfd branch from 49b29db to 5a9d2cb Compare October 24, 2024 08:29
str4t0m
str4t0m previously approved these changes Oct 24, 2024
View reviewed changes
Copy link
Collaborator

@str4t0m str4t0m left a comment

Choose a reason for hiding this comment

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

Thanks for the quick follow-up

@thenguyenyf thenguyenyf force-pushed the support_renesas_ra8_canfd branch from 5a9d2cb to 69e3365 Compare October 24, 2024 14:24
@thenguyenyf
drivers: can: initial support for Renesas RA CANFD
9bfe5fc 
Add support for CAN driver running on Renesas RA CANFD

Signed-off-by: The Nguyen <[email protected]>
@thenguyenyf thenguyenyf force-pushed the support_renesas_ra8_canfd branch from 69e3365 to 9bfe5fc Compare October 24, 2024 14:38
henrikbrixandersen
henrikbrixandersen approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Awesome work! 👍

@KhiemNguyenT KhiemNguyenT self-requested a review October 25, 2024 04:37
@henrikbrixandersen henrikbrixandersen merged commit 95cc5f5 into zephyrproject-rtos:main Oct 25, 2024
27 checks passed
This was referenced Oct 25, 2024
Merged
Merged
@thenguyenyf thenguyenyf deleted the support_renesas_ra8_canfd branch November 7, 2024 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thenguyenyf thenguyenyf thenguyenyf left review comments

@thaoluonguw thaoluonguw thaoluonguw approved these changes

@str4t0m str4t0m str4t0m approved these changes

@KhiemNguyenT KhiemNguyenT KhiemNguyenT approved these changes

@henrikbrixandersen henrikbrixandersen henrikbrixandersen approved these changes

@alexanderwachter alexanderwachter Awaiting requested review from alexanderwachter

@decsny decsny Awaiting requested review from decsny

@galak galak Awaiting requested review from galak

@martinjaeger martinjaeger Awaiting requested review from martinjaeger

@nordic-krch nordic-krch Awaiting requested review from nordic-krch

@jukkar jukkar Awaiting requested review from jukkar

@pdgendt pdgendt Awaiting requested review from pdgendt

@rlubos rlubos Awaiting requested review from rlubos

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@soburi soburi Awaiting requested review from soburi

Assignees

@KhiemNguyenT KhiemNguyenT

Labels
area: CAN area: Clock Control area: Devicetree Binding PR modifies or adds a Device Tree binding area: Networking area: Samples Samples platform: Renesas RA Renesas Electronics Corporation, RA
Projects
None yet
Milestone
v4.0.0
Development

Successfully merging this pull request may close these issues.
10 participants
@duynguyenxa @zephyrbot @henrikbrixandersen @thenguyenyf @thaoluonguw @soburi @KhiemNguyenT @str4t0m @ydamigos @nordic-krch