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

Add Support renesas ra sci i2c #86627

Merged
merged 4 commits into from
Mar 24, 2025
Merged

Add Support renesas ra sci i2c #86627

merged 4 commits into from
Mar 24, 2025

Conversation

thaoluonguw
Copy link
Collaborator

  • Add I2C driver use SCI_B I2C
  • Add support and test for I2C driver use SCI_B I2C on EK-RA8M1, EK-RA8D1 and MCK-RA8T1

@zephyrbot
Copy link
Collaborator

zephyrbot commented Mar 4, 2025 •
edited
Loading

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff

✅ All manifest checks OK

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

@zephyrbot zephyrbot added manifest manifest-hal_renesas DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Mar 4, 2025
@thaoluonguw thaoluonguw force-pushed the support_renesas_ra_sci_i2c branch from e28b8aa to 0c7b10c Compare March 13, 2025 15:35
@zephyrbot zephyrbot removed the DNM (manifest) This PR should not be merged (controlled by action-manifest) label Mar 13, 2025
@thaoluonguw thaoluonguw added platform: Renesas RA Renesas Electronics Corporation, RA area: I2C labels Mar 13, 2025
@thaoluonguw thaoluonguw marked this pull request as ready for review March 13, 2025 15:49
@zephyrbot zephyrbot requested review from soburi and teburd March 13, 2025 15:50
teburd
teburd previously approved these changes Mar 13, 2025
View reviewed changes
Copy link
Collaborator

@teburd teburd left a comment

Choose a reason for hiding this comment

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

The driver looks ok to me though honestly I wonder if it'd be simpler to avoid the HAL stuff here, it seems to add a lot of code for what is almost certainly a few register reads/writes underneath all of it.

drivers/i2c/i2c_renesas_ra_sci_b.c
uint16_t sda_output_delay;
};

struct sci_b_i2c_data {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is quite a struct to do i2c... somewhat surprising we need all of this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@teburd : Thank you so much for your feedback. Actually, all fields are necessary. We used DTC (Data Transfer Controller) in transferring, DTC only supports in HAL layer, so, there are some additional fields...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I get it, not a blocker from me

duynguyenxa
duynguyenxa previously approved these changes Mar 14, 2025
View reviewed changes
binhnguyen2434
binhnguyen2434 previously approved these changes Mar 14, 2025
View reviewed changes
Copy link
Collaborator

@binhnguyen2434 binhnguyen2434 left a comment

Choose a reason for hiding this comment

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

LGTM!

soburi
soburi previously approved these changes Mar 14, 2025
View reviewed changes
nordicjm
nordicjm requested changes Mar 17, 2025
View reviewed changes
drivers/i2c/Kconfig.renesas_ra Outdated
Comment on lines 24 to 33
if I2C_RENESAS_RA_SCI_B
config I2C_RENESAS_RA_SCI_B_DTC
bool "DTC on Transmission and Reception"
default y
select USE_RA_FSP_DTC
help
Enable DTC on transmission and reception
endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if I2C_RENESAS_RA_SCI_B
config I2C_RENESAS_RA_SCI_B_DTC
bool "DTC on Transmission and Reception"
default y
select USE_RA_FSP_DTC
help
Enable DTC on transmission and reception
endif
if I2C_RENESAS_RA_SCI_B
config I2C_RENESAS_RA_SCI_B_DTC
bool "DTC on Transmission and Reception"
default y
select USE_RA_FSP_DTC
help
Enable DTC on transmission and reception
endif

Copy link
Collaborator Author

@thaoluonguw thaoluonguw Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@nordicjm : Thank you for your feedback. I updated it. Could you please re-check it again?

drivers/i2c/Kconfig.renesas_ra Outdated
@@ -1,6 +1,6 @@
# Renesas RA Family

# Copyright (c) 2024 Renesas Electronics Corporation
# Copyright (c) 2024 - 2025 Renesas Electronics Corporation
Copy link
Collaborator

Choose a reason for hiding this comment

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

usually done as # Copyright (c) 2024-2025 Renesas Electronics Corporation without space (non-blocking)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for your feedback. I updated it.

@thaoluonguw thaoluonguw force-pushed the support_renesas_ra_sci_i2c branch 2 times, most recently from 8e4a97a to d1ea929 Compare March 18, 2025 17:18
@thaoluonguw
Copy link
Collaborator Author

Rebase to re-confirm for twister test.

KhiemNguyenT
KhiemNguyenT previously approved these changes Mar 19, 2025
View reviewed changes
binhnguyen2434
binhnguyen2434 previously approved these changes Mar 19, 2025
View reviewed changes
Khoa Tran added 4 commits March 19, 2025 10:34
@thaoluonguw
drivers: i2c: initial support for i2c sci-b driver on Renesas RA family
90c67b2 
First commit to add support for Renesas RA i2c sci-b driver

Signed-off-by: Khoa Tran <khoa.tran.yj@bp.renesas.com>
Signed-off-by: Thao Luong <thao.luong.uw@renesas.com>
@thaoluonguw
dts: arm: add i2c device node to sci device for Renesas RA family
3fc8731 
This commit to add i2c device node to support i2c sci-b driver
on Renesas RA SoCs

Signed-off-by: Khoa Tran <khoa.tran.yj@bp.renesas.com>
Signed-off-by: Thao Luong <thao.luong.uw@renesas.com>
@thaoluonguw
boards: renesas: add support for i2c sci-b driver on Renesas RA boards
9e2078e 
Enable support of i2c sci-b driver on these boards: ek_ra8m1,
ek_ra8d1, mck_ra8t1

Signed-off-by: Khoa Tran <khoa.tran.yj@bp.renesas.com>
Signed-off-by: Thao Luong <thao.luong.uw@renesas.com>
@thaoluonguw
tests: i2c: add support for i2c sci-b on Renesas RA boards
26a9294 
Add Renesas RA board support for i2c test use sci_b_i2c:
- tests/drivers/i2c/i2c_api

Example for i2c use sci_b_i2c:

west build -b ek_ra8m1 tests/drivers/i2c/i2c_api/ -p always \
 -DDTC_OVERLAY_FILE=boards/ek_ra8m1_sci_b_i2c.overlay \
 -DCONF_FILE="boards/ek_ra8m1_sci_b_i2c.conf"

Signed-off-by: Khoa Tran <khoa.tran.yj@bp.renesas.com>
Signed-off-by: Thao Luong <thao.luong.uw@renesas.com>
@thaoluonguw thaoluonguw dismissed stale reviews from binhnguyen2434 and KhiemNguyenT via 26a9294 March 19, 2025 03:34
@thaoluonguw thaoluonguw force-pushed the support_renesas_ra_sci_i2c branch from d1ea929 to 26a9294 Compare March 19, 2025 03:34
@thaoluonguw
Copy link
Collaborator Author

Rebase to solve conflict.

@KhiemNguyenT KhiemNguyenT self-requested a review March 19, 2025 04:36
@thaoluonguw thaoluonguw requested a review from nordicjm March 21, 2025 03:31
@kartben kartben merged commit e1b90bf into zephyrproject-rtos:main Mar 24, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teburd teburd teburd approved these changes

@soburi soburi soburi approved these changes

@KhiemNguyenT KhiemNguyenT KhiemNguyenT approved these changes

@nordicjm nordicjm nordicjm approved these changes

@binhnguyen2434 binhnguyen2434 binhnguyen2434 approved these changes

@duynguyenxa duynguyenxa duynguyenxa approved these changes

@henrikbrixandersen henrikbrixandersen Awaiting requested review from henrikbrixandersen

@nhutnguyenkc nhutnguyenkc Awaiting requested review from nhutnguyenkc

Assignees

@teburd teburd

Labels
area: I2C platform: Renesas RA Renesas Electronics Corporation, RA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@thaoluonguw @zephyrbot @teburd @soburi @KhiemNguyenT @nordicjm @binhnguyen2434 @duynguyenxa @kartben