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

MPSL flash sync (with SDC driver intact) #3067

Merged
merged 1 commit into from
Oct 9, 2020
Merged

MPSL flash sync (with SDC driver intact) #3067

merged 1 commit into from
Oct 9, 2020

Conversation

eugmes
Copy link
Contributor

@eugmes eugmes commented Oct 7, 2020

This PR adds flash sync support for the soc_flash_nrf driver using MPSL timeslot API.

Ref: DRGN-14778

This is a version of #3003 with SDC flash driver intact.

@github-actions github-actions bot added the doc-required PR must not be merged without tech writer approval. label Oct 7, 2020
@eugmes eugmes changed the title MPSL flash sync with sdc driver (with SDC driver) MPSL flash sync (with SDC driver) Oct 7, 2020
@eugmes eugmes marked this pull request as ready for review October 8, 2020 13:29
@eugmes eugmes changed the title MPSL flash sync (with SDC driver) MPSL flash sync (with SDC driver intact) Oct 8, 2020
@eugmes eugmes requested a review from nvlsianpu October 8, 2020 13:45
@rugeGerritsen rugeGerritsen added this to the 1.4.0 milestone Oct 8, 2020
@eugmes eugmes requested a review from tomchy October 8, 2020 14:46
@edmont
Copy link
Contributor

edmont commented Oct 9, 2020

Hi, I've been running quite many Thread Certification tests on this PR and so far so good, no issues found.

Copy link
Contributor

@edmont edmont left a comment

Choose a reason for hiding this comment

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

LGTM, it works well with most demanding flash + radio Thread tests (Commissioning cases).

Copy link
Contributor

@rugeGerritsen rugeGerritsen left a comment

Choose a reason for hiding this comment

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

Nice! I've added some suggested improvements, and how to avoid asserts when using partial erase.

Copy link
Contributor

@rugeGerritsen rugeGerritsen left a comment

Choose a reason for hiding this comment

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

Thanks!

@rugeGerritsen rugeGerritsen removed the doc-required PR must not be merged without tech writer approval. label Oct 9, 2020
Copy link
Contributor

@tomchy tomchy left a comment

Choose a reason for hiding this comment

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

Commit 1b3d896dc6fab49b1c3e9a303989df718ccab990 verified using Zigbee FOTA tests.

@joerchan
Copy link
Contributor

joerchan commented Oct 9, 2020

@eugmes Please rebase your PR to fix the CI issue.

@eugmes eugmes requested a review from joerchan October 9, 2020 09:47
@eugmes
Copy link
Contributor Author

eugmes commented Oct 9, 2020

@tomchy: The second commit has changed timings slightly. I would suggest rerunning the tests.

@tomchy
Copy link
Contributor

tomchy commented Oct 9, 2020

@eugmes I've started CI while writing the first approval 🙂
Both commits are now verified using Zigbee FOTA tests!

@eugmes
Copy link
Contributor Author

eugmes commented Oct 9, 2020

This and #3003 have passed CI.

@edmont
Copy link
Contributor

edmont commented Oct 9, 2020

Thread Commissioning tests passed successfully with current changes.

Comment on lines 147 to 151
uint32_t ret = mpsl_timeslot_session_open(timeslot_callback,
&_context.session_id);
MULTITHREADING_LOCK_RELEASE();

if (ret < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How a uint32_t value could be < 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should be int32_t

@eugmes eugmes requested a review from anangl October 9, 2020 11:49
Add flash sync support for the soc_flash_nrf driver using MPSL
timeslot API.

Signed-off-by: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@nordicsemi.no>
@eugmes
Copy link
Contributor Author

eugmes commented Oct 9, 2020

Squashed everything.

@eugmes
Copy link
Contributor Author

eugmes commented Oct 9, 2020

The CI is green again

@rlubos rlubos merged commit f8b6ab4 into nrfconnect:master Oct 9, 2020
@eugmes eugmes deleted the mpsl-flash-sync-with-sdc-driver branch October 9, 2020 15:44
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.

7 participants