Skip to content

imx6ull: rngb driver#329

Open
DawidSzpejna wants to merge 1 commit into
masterfrom
DawidSzpejna/rngb
Open

imx6ull: rngb driver#329
DawidSzpejna wants to merge 1 commit into
masterfrom
DawidSzpejna/rngb

Conversation

@DawidSzpejna

@DawidSzpejna DawidSzpejna commented Jan 11, 2023

Copy link
Copy Markdown

Add driver for rngb on imx6ull to support TRNG

Description

Motivation and Context

For imx6ull, trng wans't supported

Types of changes

  • 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)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: (list targets here).

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • [] My changes generate no new compilation warnings for any of the targets.

Special treatment

  • This PR needs additional PRs to work (list the PRs, preferably in merge-order).
  • I will merge this PR by myself when appropriate.

@DawidSzpejna DawidSzpejna requested a review from nalajcie January 11, 2023 14:35
@DawidSzpejna DawidSzpejna self-assigned this Jan 11, 2023
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated

@nalajcie nalajcie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pointed out some things which might be improved ;)

Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/Makefile
@nalajcie nalajcie marked this pull request as ready for review January 11, 2023 17:06
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread rngb/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
@DawidSzpejna DawidSzpejna force-pushed the DawidSzpejna/rngb branch 2 times, most recently from aff855f to 6772360 Compare February 27, 2023 15:05

@gerard5 gerard5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Partial review, remarks for rngb_intr() function.

Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c
@nalajcie nalajcie requested a review from bswiercz July 13, 2023 11:16
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
Comment thread random/imx6ull-rngb.c Outdated
The driver starts by creating basic structures for a phonix driver.
Then it sets flags for tests of rngb and seeding, sequentially.
The last things is waiting for system messaga.

JIRA: RTOS-317
@bswiercz bswiercz force-pushed the DawidSzpejna/rngb branch from 4a0e286 to dcf8a36 Compare July 19, 2023 10:02

@ziemleszcz ziemleszcz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread random/imx6ull-rngb.c
/* Run self test */
*(common_rngb.base + rng_cmd) = 0x1;
mutexLock(common_rngb.lock);
while (common_rngb.intr_st == 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here we are waiting for whether the self-test has finished or failed and not for all statuses, so shouldn't it be?

Suggested change
while (common_rngb.intr_st == 0) {
while ((common_rngb.intr_st & (SELF_TEST_DONE | ERROR_OCCURRED)) == 0) {

Comment thread random/imx6ull-rngb.c
/* Run seeding */
*(common_rngb.base + rng_cmd) = 0x2;
mutexLock(common_rngb.lock);
while (common_rngb.intr_st == 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here we are waiting for whether the seeding has finished or failed and not for all statuses, so shouldn't it be?

Suggested change
while (common_rngb.intr_st == 0) {
while ((common_rngb.intr_st & (SEED_DONE | ERROR_OCCURRED)) == 0) {

Comment thread random/imx6ull-rngb.c
}

handleMsg(NULL);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

System cleanup is missing, any required resources freeing - if handleMsg() returns on msgRecv() error, also return 0 is misleading if handleMsg() terminates.

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.

6 participants