Skip to content
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

STM32 HRNG driver #642

Merged
merged 2 commits into from
Mar 10, 2018
Merged

STM32 HRNG driver #642

merged 2 commits into from
Mar 10, 2018

Conversation

networkfusion
Copy link
Member

@networkfusion networkfusion commented Mar 9, 2018

Description

  • Most STM32's support a hardware random number generator. This PR enables this functionality

Motivation and Context

How Has This Been Tested?

On an STM32F769I (Be aware, this is my first driver so it needs careful review against the reference implementation.

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@nfbot
Copy link
Member

nfbot commented Mar 9, 2018

Hi @networkfusion,

I'm nanoFramework bot.
Thank you for your contribution!

A human will be reviewing it shortly. 😉

@networkfusion networkfusion requested a review from josesimoes March 9, 2018 16:23
Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

Looks good! Almost only cosmetic changes requested. 👏

On the code and header files, if you could tidy up the tabs... there 2 and 4 spaces... 😉


#endif /* HAL_USE_STM32_RNG */

#endif /* HAL_STM32_RNG_H_ */
Copy link
Member

Choose a reason for hiding this comment

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

missing empty line

Copy link
Member Author

Choose a reason for hiding this comment

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

done...

@@ -0,0 +1,8 @@
# RNG v1

This driver supports all STM32 series.
Copy link
Member

Choose a reason for hiding this comment

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

this is contradicted bellow... something like all but F0 seems more accurate...

Copy link
Member Author

Choose a reason for hiding this comment

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

done..



uint32_t rng_lld_GetLastRandomNumber() {
return RNGD1.RandomNumber;
Copy link
Member

Choose a reason for hiding this comment

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

missing tab

Copy link
Member Author

Choose a reason for hiding this comment

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

done...



// RNG Interrupt definitions
#define RNG_IT_DRDY RNG_SR_DRDY /*!< Data Ready interrupt */
Copy link
Member

Choose a reason for hiding this comment

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

RNG_SR_DRDY doesn't seem to be used anywhere

Copy link
Member Author

Choose a reason for hiding this comment

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

see below...

#define RNG_IT_SEI RNG_SR_SEIS /*!< Seed error interrupt */

// RNG Flag definitions
#define RNG_FLAG_DRDY RNG_SR_DRDY /*!< Data ready */
Copy link
Member

Choose a reason for hiding this comment

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

RNG_SR_DRDY doesn't seem to be used anywhere

Copy link
Member Author

Choose a reason for hiding this comment

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

That is used...


// RNG Flag definitions
#define RNG_FLAG_DRDY RNG_SR_DRDY /*!< Data ready */
#define RNG_FLAG_CECS RNG_SR_CECS /*!< Clock error current status */
Copy link
Member

Choose a reason for hiding this comment

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

this one too

Copy link
Member Author

Choose a reason for hiding this comment

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

see below...

// RNG Flag definitions
#define RNG_FLAG_DRDY RNG_SR_DRDY /*!< Data ready */
#define RNG_FLAG_CECS RNG_SR_CECS /*!< Clock error current status */
#define RNG_FLAG_SECS RNG_SR_SECS /*!< Seed error current status */
Copy link
Member

Choose a reason for hiding this comment

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

and this one too

Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to keep these 3 as one is used and the other 2 are options (although not currently used...

/* Driver macros. */
/*===========================================================================*/
// Reset RNG handle state
//#define __RNG_RESET_HANDLE_STATE(__HANDLE__) ((__HANDLE__).State = HAL_RNG_STATE_RESET)
Copy link
Member

Choose a reason for hiding this comment

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

can be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

return randomNumber;
}

#endif /* HAL_USE_STM32_RNG */
Copy link
Member

Choose a reason for hiding this comment

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

missing empty line

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

/*===========================================================================*/


// RNG Interrupt definitions
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't seem to be using interrupts so this block can go away, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@josesimoes
Copy link
Member

One last request: on the halconf_nf.h (booter and CLR) for the F091 you have to add that negative RNG driver or the build will break

@josesimoes josesimoes added Type: enhancement Series: STM32xx Everything related specifically with STM32 targets labels Mar 10, 2018
@josesimoes josesimoes merged commit 142c36b into nanoframework:develop Mar 10, 2018
@networkfusion networkfusion deleted the develop-rng branch July 24, 2018 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Series: STM32xx Everything related specifically with STM32 targets Type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants