Skip to content

drivers: crc: initial support for renesas,ra-crc driver #87557

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

thenguyenyf
Copy link
Collaborator

@thenguyenyf thenguyenyf commented Mar 24, 2025

This PR is inherited from #74977 to add a CRC device driver with hardware accelerator. We'd like to introduce:

  • New CRC device drivers: include/zephyr/drivers/crc.h
  • New CRC subsystem: using the same interface with include/zephyr/sys/crc.h

The crc subsys implementation can be chosen as:

  • A CRC device driver, get by zephyr,crc alias if it's defined
  • CRC software-based library, that migrated from old lib/crc to new location: subsys/crc/crc*_sw.c

@zephyrbot
Copy link
Collaborator

zephyrbot commented Mar 24, 2025

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

Name Old Revision New Revision Diff
hal_renesas zephyrproject-rtos/hal_renesas@15c3f05 zephyrproject-rtos/hal_renesas#89 zephyrproject-rtos/hal_renesas#89/files

DNM label due to: 1 project with PR revision

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 24, 2025
@henrikbrixandersen henrikbrixandersen self-requested a review March 24, 2025 20:32
@henrikbrixandersen henrikbrixandersen added the Architecture Review Discussion in the Architecture WG required label Mar 24, 2025
@henrikbrixandersen
Copy link
Member

@thenguyenyf Nice, thanks! Are you able to give a short presentation of this in an Architecture Working Group meeting?

@thenguyenyf
Copy link
Collaborator Author

@thenguyenyf Nice, thanks! Are you able to give a short presentation of this in an Architecture Working Group meeting?

Hello @henrikbrixandersen . I'm willing to join and give a presentation for the new CRC subsystem. How about next week meeting? I may not be able to join this week.

@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch 3 times, most recently from bce01eb to 6b4d778 Compare March 28, 2025 03:30
@thaoluonguw
Copy link
Collaborator

Please rebase to slove conflict.

@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch 2 times, most recently from 5e366bc to 6c12cb7 Compare March 31, 2025 06:30
@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch 2 times, most recently from d9db36b to ea016fb Compare April 1, 2025 11:43
@thenguyenyf
Copy link
Collaborator Author

Last push just to rebase and solve conflict

@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch from 0c100b8 to f342368 Compare April 10, 2025 08:30
@thenguyenyf
Copy link
Collaborator Author

Last push just to rebase and solve conflict

@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch from f342368 to 40751ca Compare April 14, 2025 07:12
@carlescufi carlescufi moved this from Todo to In Progress in Architecture Review Apr 14, 2025
Duy Vo added 4 commits April 16, 2025 09:45
Update rev of hal_renesas to add support for crc driver

Signed-off-by: Duy Vo <[email protected]>
- Implement CRC syscall.
- Add CRC driver API.
- Introduce support for the CRC driver.

Signed-off-by: Duy Vo <[email protected]>
Add device tree node for CRC driver on all Renesas MCU

Signed-off-by: Duy Vo <[email protected]>
Enable and add CRC node to aliases, chosen for all
Renesas MCU

Signed-off-by: Duy Vo <[email protected]>
@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch from 40751ca to 19ee23d Compare April 16, 2025 02:52
@thenguyenyf
Copy link
Collaborator Author

thenguyenyf commented Apr 16, 2025

Hello @henrikbrixandersen , @pdgendt , @kartben . I've updated according to WG's review items. Could you please take a look?


config CRC4
bool "CRC-4 (Generic)"
depends on CRC_DRIVER_HAS_CRC4 || CRC_SW_HANDLER
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like an "all or nothing" for being able to select HW or SW CRC handlers. You can't have HW capable CRC drivers for some 32-bit variant, while having the SW CRC handlers for the others as a fallback.

Not sure how this could be achieved easily, maybe a chosen per CRC type? Or add boolean dts binding properties for hardware to describe which variants are supported?

Copy link
Collaborator Author

@thenguyenyf thenguyenyf Apr 16, 2025

Choose a reason for hiding this comment

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

Not sure how this could be achieved easily, maybe a chosen per CRC type?

Do you mean that for each CRC type we should have a specific chosen alias for it (ex: zephyr,crc4, zephyr,crc4ti, zephyr,crc7be, ...)? There are more than 10 type of CRC currently and the number could be increased. So, defining lots of aliases just to use one driver may not provide a good user experience.

Or add boolean dts binding properties for hardware to describe which variants are supported?

There is a large of number of properties should be added. So, if it's simply to add a way to check the hardware capability, how about adding the crc_device_get_capabilities as CRC driver API?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah splitting all of them isn't ideal, but don't really have other suggestions.

how about adding the crc_device_get_capabilities as CRC driver API?

You'd probably want to set these as bit flags, but you'd also run quickly out of available bits, maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah splitting all of them isn't ideal, but don't really have other suggestions.

Okay. If there is no better choice, let me follow your idea.
@henrikbrixandersen , @nashif , @kartben . Do you have any comment?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another option would be to do something similar to the hwinfo driver subsystem, where only a single driver can implement a given CRC variant, and leave the SW handlers as __weak?
Just spitballing here, this might not be considered a clean solution either.

@kartben kartben requested a review from Copilot April 16, 2025 19:57
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 90 out of 100 changed files in this pull request and generated no comments.

Files not reviewed (10)
  • boards/renesas/ek_ra2a1/ek_ra2a1.dts: Language not supported
  • boards/renesas/ek_ra2l1/ek_ra2l1.dts: Language not supported
  • boards/renesas/ek_ra4e2/ek_ra4e2.dts: Language not supported
  • boards/renesas/ek_ra4l1/ek_ra4l1.dts: Language not supported
  • boards/renesas/ek_ra4m1/ek_ra4m1.dts: Language not supported
  • boards/renesas/ek_ra4m2/ek_ra4m2.dts: Language not supported
  • boards/renesas/ek_ra4m3/ek_ra4m3.dts: Language not supported
  • boards/renesas/ek_ra4w1/ek_ra4w1.dts: Language not supported
  • boards/renesas/ek_ra6e2/ek_ra6e2.dts: Language not supported
  • boards/renesas/ek_ra6m1/ek_ra6m1.dts: Language not supported

@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch from 19ee23d to 9a77e1f Compare April 17, 2025 06:31
@kartben kartben removed their assignment Apr 23, 2025
@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch from 9a77e1f to 6af150c Compare April 25, 2025 11:21
Duy Vo and others added 6 commits April 25, 2025 18:38
Migrate support from crc library to new crc subsystem
Add hardware acclerator backend for crc subsystem

Signed-off-by: Duy Vo <[email protected]>
Add samples for CRC driver

Signed-off-by: Duy Vo <[email protected]>
Add samples for CRC subsystem

Signed-off-by: Duy Vo <[email protected]>
Add ztests for CRC driver

Signed-off-by: Duy Vo <[email protected]>
Add ztests for CRC subsystem

Signed-off-by: Duy Vo <[email protected]>
Due to the move of lib/crc to subsys/crc, the include path of CRC handler
in this test should be updated:
- tests/unit/crc

Signed-off-by: The Nguyen <[email protected]>
@thenguyenyf thenguyenyf force-pushed the renesas_ra_crc_driver_support branch from 6af150c to 578b46a Compare April 25, 2025 11:39
@thenguyenyf thenguyenyf requested a review from pdgendt April 25, 2025 11:39
@kartben kartben requested a review from Copilot April 25, 2025 18:40
@kartben kartben removed their assignment Apr 25, 2025
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.

Pull Request Overview

This PR introduces initial support for the Renesas RA CRC driver by adding hardware-accelerated CRC functionality and updating board configuration files and maintainers accordingly.

  • Added "crc" as a supported feature in multiple Renesas board YAML files.
  • Updated the MAINTAINERS file with new CRC subsystem source file paths.

Reviewed Changes

Copilot reviewed 90 out of 100 changed files in this pull request and generated no comments.

Show a summary per file
File Description
boards/renesas/ek_ra6e2/ek_ra6e2.yaml Added "crc" to the supported list
boards/renesas/ek_ra4w1/ek_ra4w1.yaml Added "crc" to the supported list
boards/renesas/ek_ra4m3/ek_ra4m3.yaml Added "crc" to the supported list
boards/renesas/ek_ra4m2/ek_ra4m2.yaml Added "crc" to the supported list
boards/renesas/ek_ra4m1/ek_ra4m1.yaml Added "crc" to the supported list
boards/renesas/ek_ra4l1/ek_ra4l1.yaml Added "crc" to the supported list
boards/renesas/ek_ra4e2/ek_ra4e2.yaml Added "crc" to the supported list
boards/renesas/ek_ra2l1/ek_ra2l1.yaml Added "crc" to the supported list
boards/renesas/ek_ra2a1/ek_ra2a1.yaml Added "crc" to the supported list
MAINTAINERS.yml Updated file paths to include new CRC subsystem files
Files not reviewed (10)
  • boards/renesas/ek_ra2a1/ek_ra2a1.dts: Language not supported
  • boards/renesas/ek_ra2l1/ek_ra2l1.dts: Language not supported
  • boards/renesas/ek_ra4e2/ek_ra4e2.dts: Language not supported
  • boards/renesas/ek_ra4l1/ek_ra4l1.dts: Language not supported
  • boards/renesas/ek_ra4m1/ek_ra4m1.dts: Language not supported
  • boards/renesas/ek_ra4m2/ek_ra4m2.dts: Language not supported
  • boards/renesas/ek_ra4m3/ek_ra4m3.dts: Language not supported
  • boards/renesas/ek_ra4w1/ek_ra4w1.dts: Language not supported
  • boards/renesas/ek_ra6e2/ek_ra6e2.dts: Language not supported
  • boards/renesas/ek_ra6m1/ek_ra6m1.dts: Language not supported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Review Discussion in the Architecture WG required area: Process area: Samples Samples area: Utilities DNM (manifest) This PR should not be merged (controlled by action-manifest) manifest manifest-hal_renesas platform: Renesas RA Renesas Electronics Corporation, RA
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

7 participants