Skip to content

CRC Thread safety #6546

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

Closed
daniel-cesarini-tridonic-com opened this issue Apr 5, 2018 · 7 comments
Closed

CRC Thread safety #6546

daniel-cesarini-tridonic-com opened this issue Apr 5, 2018 · 7 comments

Comments

@daniel-cesarini-tridonic-com

Description

  • Type: Enhancement | Question
  • Priority: Major

Enhancement/Question

Reason to enhance or problem with existing solution
In the current implementation of CRC in mbed-os thread safety is not guaranteed:
#5911 (comment)

How to?
How do you envision to solve the issue?

Suggested enhancement
We implemented some time ago a CRC Class (https://github.com/ARMmbed/mbed-os/pull/3443/files) that deals with Thread safety by storing intermediate results in a local variables, such that no "lower level" context is needed, and only the time of accessing the lower layers should be mutexed / protected.
Is that approach fitting your needs?

@deepikabhavnani @sg- @MarceloSalazar

FYI: @markus-becker-tridonic-com

@deepikabhavnani
Copy link

@daniel-cesarini-tridonic-com Thanks for the proposed solution. We will definitely look into it during HW CRC implementation. Thread safety will be for sure required to support hardware CRC, and we have not finalized the HAL API's for that.

Also I suppose we will have mutex locking for low level access just like other drivers. https://github.com/ARMmbed/mbed-os/blob/master/drivers/AnalogIn.cpp#L23

@bulislaw

@daniel-cesarini-tridonic-com
Copy link
Author

The solution is actually not handling the mutual access, it is only providing a "multiplexed" access to the CRC lower level in case of usage of "CRC accumulation" functionality by storing the intermediate results in a local variable in order to free the resource of CRC lower level in the meanwhile.

@deepikabhavnani
Copy link

deepikabhavnani commented Apr 6, 2018

Storing intermediate results in the class will be bit tricky for different devices, some of them maintain the states and store the intermediate values themselves and some do not support partial CRC. Our API's are designed to return the intermediate values, and the application/thread using it has to make sure it passes the correct intermediate value back for rest of the calculation.

Sorry I didn't get the chance to study your solution fully, we will have a look during HW CRC implementation which is not planned in near future.

@bulislaw
Copy link
Member

@scartmell-arm fyi

@ciarmcom
Copy link
Member

ciarmcom commented Jun 1, 2018

ARM Internal Ref: MBOTRIAGE-98

@daniel-cesarini-tridonic-com
Copy link
Author

daniel-cesarini-tridonic-com commented Sep 25, 2018

Would this MR (already merged) #7781 close this issue?

@deepikabhavnani
Copy link

Yes. Thanks

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

No branches or pull requests

5 participants