Skip to content

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

Merged
merged 7 commits into from May 31, 2018
Merged

Add porting guide for Hardware CRC HAL module #505

merged 7 commits into from May 31, 2018

Conversation

ghost
Copy link

@ghost ghost commented Apr 27, 2018

No description provided.

## 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,
Copy link
Member

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.
Copy link
Member

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
Copy link
Member

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:

[![View code](https://www.mbed.com/embed/?type=library)](http://os-doc-builder.test.mbed.com/docs/development/feature-hal-spec-crc-doxy/classmbed_1_1_crc.html)
Copy link
Member

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:

[![View code](https://www.mbed.com/embed/?type=library)](http://os-doc-builder.test.mbed.com/docs/development/feature-hal-spec-crc-doxy/classreset__reason_1_1_crc_test.html)
Copy link
Member

Choose a reason for hiding this comment

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

Internal link?

Steven Cartmell and others added 2 commits April 30, 2018 12:07
Copy edit file for consistent tense across docs, formatting and U.S. spelling.
@AnotherButler
Copy link
Contributor

@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?

@ghost
Copy link
Author

ghost commented May 9, 2018

ARMmbed/mbed-os#6708

## 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.

Copy link

@deepikabhavnani deepikabhavnani May 9, 2018

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.
@AnotherButler
Copy link
Contributor

@deepikabhavnani I've added that note. Does it look OK?

@AnotherButler
Copy link
Contributor

@bulislaw Have your concerns been addressed? If so, is this OK to merge once the code merges?

@deepikabhavnani
Copy link

@AnotherButler - Should it be "The hardware CRC API's are not thread safe" ?

Update note to make subject plural.
@AnotherButler
Copy link
Contributor

I'm getting unicorns. Can anyone tell me if the code dependency merged?

@bulislaw
Copy link
Member

bulislaw commented May 23, 2018 via email

Add link to Doxygen.
You can read more about the test cases here:

```
Fix link after code is merged.
Copy link
Contributor

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

Copy link
Author

@ghost ghost May 30, 2018

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

@MarceloSalazar
Copy link
Contributor

Can we please finalize and merge this for OOB?

@AnotherButler AnotherButler merged commit dc12def into ARMmbed:new_engine May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants