-
Notifications
You must be signed in to change notification settings - Fork 178
Add porting guide for Hardware CRC HAL module #505
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
Conversation
## Hardware CRC - Porting Guide | ||
|
||
The Hardware CRC HAL API provides a low-level interface to the Hardware CRC | ||
module of a target platform. Implementing the Hardware CRC API is not mandatory, |
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.
Lets not mention that it's not mandatory, if the platform supports it we want it implemented.
module of a target platform. Implementing the Hardware CRC API is not mandatory, | ||
but implementing it you can gain the performance benefits of using hardware | ||
acceleration for CRC calculations, if the API is not implemented the CRC API | ||
will fallback to using table and bitwise CRC implementations. |
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.
Here we can word it something like "for platforms not having CRC module capabilities"
this function is called. | ||
|
||
* Calling the hal_crc_compute_partial() function before calling | ||
hal_crc_partial_start() is undefined. The Hardware CRC module must be |
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.
What about calling get result before partial start?
You can find the API and specification for the Hardware CRC API in the following | ||
header file: | ||
|
||
[](http://os-doc-builder.test.mbed.com/docs/development/feature-hal-spec-crc-doxy/classmbed_1_1_crc.html) |
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 think that's an internal link.
|
||
You can read more about the test cases here: | ||
|
||
[](http://os-doc-builder.test.mbed.com/docs/development/feature-hal-spec-crc-doxy/classreset__reason_1_1_crc_test.html) |
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.
Internal link?
Copy edit file for consistent tense across docs, formatting and U.S. spelling.
@scartmell-arm Fantastic work on this. It looks really good 👍 Could you please link the PR we're waiting on for the testing and class reference links? |
## Hardware CRC | ||
|
||
The hardware CRC HAL API provides a low-level interface to the hardware CRC module of a target platform. Implementing the hardware CRC API allows you to gain the performance benefits of using hardware acceleration for CRC calculations. For platforms without hardware CRC capabilities, the API falls back to using the table and bitwise CRC implementations. | ||
|
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.
Note here is required for Hardware CRC - not thread safe, else can add it to undefined behavior section
Add note as requested in comments.
@deepikabhavnani I've added that note. Does it look OK? |
@bulislaw Have your concerns been addressed? If so, is this OK to merge once the code merges? |
@AnotherButler - Should it be "The hardware CRC API's are not thread safe" ? |
Update note to make subject plural.
I'm getting unicorns. Can anyone tell me if the code dependency merged? |
Not yet, but it passed Cis so should be merged very soon.
From: Amanda Butler <[email protected]>
Reply-To: ARMmbed/Handbook <[email protected]>
Date: Wednesday, 23 May 2018 at 10:23
To: ARMmbed/Handbook <[email protected]>
Cc: Bartek Szatkowski <[email protected]>, Mention <[email protected]>
Subject: Re: [ARMmbed/Handbook] Add porting guide for Hardware CRC HAL module (#505)
I'm getting unicorns. Can anyone tell me if the code dependency merged?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#505 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAGi9cJnSoamOcLyx7IYTNFzQjgi-ak8ks5t1X75gaJpZM4TqLT3>.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
Add link to Doxygen.
You can read more about the test cases here: | ||
|
||
``` | ||
Fix link after code is merged. |
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.
Query: Where do we find the tests? I think they should be on this page? https://os-doc-builder.test.mbed.com/docs/development/mbed-os-api-doxy/modules.html
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 tests still haven't been merged. The PR is here: ARMmbed/mbed-os#6831
Can we please finalize and merge this for OOB? |
No description provided.