Skip to content

Add support ospi driver for Renesas RA8 devices #80799

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
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add support ospi driver for Renesas RA8 devices #80799

wants to merge 7 commits into from

Conversation

thaoluonguw
Copy link
Collaborator

Add support for ospi driver running on Renesas RA8 (RA8M1 and RA8D1)

  • boards: renesas: Add support for ospi on ek_ra8m1 and ek_ra8d1
  • drivers: flash: Add support for ospi_b driver
  • dts: arm: Add support for ospi
  • dts: bindings: Add support for ospi_b
  • tests: drivers: flash: common: Add support for ospi.
  • samples: drivers: jesd216: Add support for ospi
  • samples: drivers: spi_flash: Add support for ospi

Note: This PR requires #79766 is merged

@zephyrbot
Copy link
Collaborator

zephyrbot commented Nov 3, 2024 •
edited by github-actions bot
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 This PR should not be merged (Do Not Merge) labels Nov 3, 2024
@quytranpzz quytranpzz force-pushed the support_renesas_ra8_ospi branch from 8f49aa6 to ae8e589 Compare November 29, 2024 03:55
@thaoluonguw thaoluonguw marked this pull request as ready for review November 30, 2024 06:13
@zephyrbot zephyrbot added area: Flash platform: Renesas RA Renesas Electronics Corporation, RA area: Samples Samples labels Nov 30, 2024
@quytranpzz quytranpzz force-pushed the support_renesas_ra8_ospi branch from ae8e589 to a585f29 Compare December 6, 2024 09:26
@zephyrbot zephyrbot removed manifest manifest-hal_renesas DNM This PR should not be merged (Do Not Merge) labels Dec 6, 2024
@thaoluonguw thaoluonguw force-pushed the support_renesas_ra8_ospi branch from a585f29 to 7d172b6 Compare January 6, 2025 13:50
@thaoluonguw
Copy link
Collaborator Author

Rebase with main to solve conflicts.

@thaoluonguw
Copy link
Collaborator Author

Hello @GeorgeCGV : Could you please help us review again? Thank you so much.

@khoa-nguyen-18 khoa-nguyen-18 force-pushed the support_renesas_ra8_ospi branch from 5d1aa9e to a4db958 Compare April 3, 2025 09:15
@khoa-nguyen-18
Copy link
Contributor

rebase main to solve conflicts

@khoa-nguyen-18
Copy link
Contributor

@khoa-nguyen-18 please avoid extensive reformatting of spi_nor.h to ensure commit d150f56f2b3f7da718c317a11538a820fd8ef7ec reflects actual modifications and new lines.

I understand. I will remove the formatting for the entire file and only format our additions

khoa-nguyen-18 and others added 2 commits April 3, 2025 16:34
@khoa-nguyen-18
drivers: flash: Add additional define macro and reformat spi_nor.h
a294326 
Add additional define macro and reformat spi_nor.h

Signed-off-by: Khoa Nguyen <[email protected]>
@quytranpzz @khoa-nguyen-18
dts: renesas: Add devicetree property for OSPI support on RA8
2acdf76 
Add ospi node on Renesas RA8 devicetree to support QSPI flash driver

Signed-off-by: Quy Tran <[email protected]>
@khoa-nguyen-18 khoa-nguyen-18 force-pushed the support_renesas_ra8_ospi branch from a4db958 to b3ba2a9 Compare April 3, 2025 09:36
@quytranpzz @khoa-nguyen-18
boards: renesas: Add boards support for OSPI flash driver on RA8
e38c3fd 
Add support for OSPI flash driver on EK-RA8D1 and EK-RA8M1

Signed-off-by: Quy Tran <[email protected]>
@khoa-nguyen-18 khoa-nguyen-18 force-pushed the support_renesas_ra8_ospi branch 2 times, most recently from 4b87e5c to 896904e Compare April 4, 2025 01:40
GeorgeCGV
GeorgeCGV previously approved these changes Apr 6, 2025
View reviewed changes
@kartben kartben requested a review from Copilot April 6, 2025 15:28
Copilot
Copilot AI reviewed Apr 6, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 23 out of 37 changed files in this pull request and generated no comments.

Files not reviewed (14)
  • boards/renesas/ek_ra8d1/ek_ra8d1-pinctrl.dtsi: Language not supported
  • boards/renesas/ek_ra8d1/ek_ra8d1.dts: Language not supported
  • boards/renesas/ek_ra8m1/ek_ra8m1-pinctrl.dtsi: Language not supported
  • boards/renesas/ek_ra8m1/ek_ra8m1.dts: Language not supported
  • drivers/flash/CMakeLists.txt: Language not supported
  • drivers/flash/Kconfig: Language not supported
  • drivers/flash/Kconfig.renesas_ra_ospi: Language not supported
  • dts/arm/renesas/ra/ra8/ra8x1.dtsi: Language not supported
  • modules/Kconfig.renesas_fsp: Language not supported
  • tests/drivers/flash/common/boards/ek_ra4e2.conf: Language not supported
  • tests/drivers/flash/common/boards/ek_ra4m2.conf: Language not supported
  • tests/drivers/flash/common/boards/ek_ra4m3.conf: Language not supported
  • tests/drivers/flash/common/boards/ek_ra6e2.conf: Language not supported
  • tests/drivers/flash/common/boards/ek_ra6m1.conf: Language not supported
Comments suppressed due to low confidence (2)

samples/drivers/spi_flash/src/main.c:68

  • Please verify that the test’s expected data for CONFIG_FLASH_RENESAS_RA_OSPI_B accurately reflects the device’s actual flash memory behavior to avoid false negatives during testing.
const uint8_t expected[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};

drivers/flash/flash_renesas_ra_ospi_b.h:72

  • [nitpick] Consider declaring erase_command_list as a const array if these command definitions are not expected to change at runtime, to enforce immutability.
static spi_flash_erase_command_t erase_command_list[] = {

binhnguyen2434
binhnguyen2434 previously approved these changes Apr 8, 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!

thenguyenyf
thenguyenyf previously approved these changes Apr 10, 2025
View reviewed changes
@KhiemNguyenT KhiemNguyenT self-requested a review April 10, 2025 09:06
KhiemNguyenT
KhiemNguyenT previously approved these changes Apr 10, 2025
View reviewed changes
de-nordic
de-nordic reviewed Apr 10, 2025
View reviewed changes
drivers/flash/flash_renesas_ra_ospi_b.c Outdated
Comment on lines 567 to 583
if (data != NULL) {
p_src = data;
} else {
return -EINVAL;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not directly use data and get rid of the p_src?

This comment was marked as outdated.

Sign in to view

Copy link
Contributor

Choose a reason for hiding this comment

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

We use p_src to cast from const void * to const uint8_t * as input for the HAL API

drivers/flash/flash_renesas_ra_ospi_b.c Outdated
Comment on lines 574 to 588
LOG_ERR("address or size exceeds expected values: "
"addr 0x%lx, size %zu",
(long)offset, len);
Copy link
Collaborator

Choose a reason for hiding this comment

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

That is probably for vendor to decided, but I would put here LOG_DBG. Reason is that the message takes space, and chances that this will be logged, on well tested platform, is really low, which means that the log is only important for debugging the issue and it does not actually log any fault, from the perspective of hardware, that has randomly occurred during run-time, rather problem with software that used the device.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. After consideration, we use LOG_DBG in the child functions and LOG_ERR in the driver API.

drivers/flash/flash_renesas_ra_ospi_b.c Outdated
&ospi_b_data->ospi_b_ctrl, p_src,
(uint8_t *)(BSP_FEATURE_OSPI_B_DEVICE_1_START_ADDRESS + offset), size);
if (err != FSP_SUCCESS) {
err = -EIO;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you may consider LOG_DBG, again decision on vendor, but in here you have two places that return -EIO from different calls, so after user gets -EIO, it is not known which operation failed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, I have added a LOG_DBG in here

drivers/flash/flash_renesas_ra_ospi_b.c Outdated
Comment on lines 244 to 258
if (err != FSP_SUCCESS) {
return err;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you want to do LOG_DBG on each of these returns?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have updated the code to add LOG_DBG for all return err statements.

@khoa-nguyen-18
tests: drivers: flash: Add support "common" for Renesas OSPI_B
8430eaf 
Add support test app "flash/common" for testing Renesas RA
OSPI_B on ek_ra8m1, ek_ra8d1

Signed-off-by: Khoa Nguyen <[email protected]>
@khoa-nguyen-18
samples: drivers: jesd216: Add support OSPI for Renesas RA8
b2659d3 
Add support OSPI for Renesas ek_ra8m1, ek_ra8d1 to run
sample jesd216

Signed-off-by: Khoa Nguyen <[email protected]>
Signed-off-by: Tri Nguyen <[email protected]>
Signed-off-by: Thao Luong <[email protected]>
@khoa-nguyen-18
samples: drivers: spi_flash: Add support OSPI for Renesas RA8
cfe422d 
Add support OSPI for Renesas ek_ra8m1, ek_ra8d1 to run sample
spi_flash

Signed-off-by: Khoa Nguyen <[email protected]>
Signed-off-by: Tri Nguyen <[email protected]>
Signed-off-by: Thao Luong <[email protected]>
@khoa-nguyen-18 khoa-nguyen-18 force-pushed the support_renesas_ra8_ospi branch from 896904e to cfe422d Compare April 22, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@de-nordic de-nordic de-nordic left review comments

@quytranpzz quytranpzz quytranpzz left review comments

Copilot code review Copilot Copilot left review comments

@khoa-nguyen-18 khoa-nguyen-18 khoa-nguyen-18 left review comments

@GeorgeCGV GeorgeCGV GeorgeCGV left review comments

@KhiemNguyenT KhiemNguyenT KhiemNguyenT left review comments

@binhnguyen2434 binhnguyen2434 binhnguyen2434 left review comments

@thenguyenyf thenguyenyf thenguyenyf left review comments

@soburi soburi Awaiting requested review from soburi

@FRASTM FRASTM Awaiting requested review from FRASTM

@duynguyenxa duynguyenxa Awaiting requested review from duynguyenxa

@kartben kartben Awaiting requested review from kartben

@nashif nashif Awaiting requested review from nashif

@rghaddab rghaddab Awaiting requested review from rghaddab

At least 2 approving reviews are required to merge this pull request.

Assignees

@de-nordic de-nordic

Labels
area: Flash area: Samples Samples 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 @GeorgeCGV @khoa-nguyen-18 @KhiemNguyenT @de-nordic @binhnguyen2434 @thenguyenyf @quytranpzz