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

LPC55S69: Update Flash driver to set clock frequency #10246

Merged
merged 1 commit into from Apr 2, 2019

Conversation

Projects
None yet
7 participants
@mmahadevan108
Copy link
Contributor

commented Mar 27, 2019

Description

This is to ensure the flash access time is set correctly

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change
@mmahadevan108

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

@orenc17
Copy link
Contributor

left a comment

Just discard the binary change

@cmonr cmonr added the needs: work label Mar 27, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

@orenc17 Why is the binary change not needed?

@cmonr cmonr added needs: review and removed needs: work labels Mar 27, 2019

@cmonr cmonr requested review from cmonr and mikisch81 Mar 27, 2019

@orenc17

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

It has been agreed that binary updates will occur only on releases

The CI is already compiling the secure binaries for each PR

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

Making sure I understand then (completely possible I missed this), is there no guarantee that binaries on master are up to date?

CC @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-psa?

@mmahadevan108

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

@orenc17 . If the binary is not updated then developers working on the master branch will not get this change, am I understanding this correctly?

@cmonr cmonr requested a review from bulislaw Mar 28, 2019

@mmahadevan108

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

@orenc17 . If the binary is not updated then developers working on the master branch will not get this change, am I understanding this correctly?

@orenc17

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2019

@alzix can update on this better

@alzix

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2019

There are prebuilt secure image binaries committed to mbed-os master, which require manual rebuild and commit.
I'm not fond of this approach, but it is here for providing backward compatibility (in term of build instructions) for application developers working with default secure image.
This technique has its own drawbacks, request for rebuild&commit secure images upon any change is one of them.
We will work to provide better user experience in the upcoming releases.

@NirSonnenschein

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2019

note that when running CI, the CI will build updated secure images prior to testing (but these images are not pushed into master). This means that any developer developing on a device that requires secure images must build these images before running as a the default ones provided as part of the OS tree are usually out of date (they will get the change if they rebuild the secure image). This approach is not the most friendly as Alex said above but it does prevent issues with changes being overrun when edited by multiple PRs at once(secure images are non-mergable). before creating a release the secure images will be updated, so that at least official releases should always have updated binaries,

@0xc0170
Copy link
Member

left a comment

@mmahadevan108 tfm.bin should be removed as requested above

@0xc0170 0xc0170 added needs: work and removed needs: review labels Apr 1, 2019

LPC55S69: Update Flash driver to set clock frequency
This is to ensure the flash access time is set correctly

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

@mmahadevan108 mmahadevan108 force-pushed the NXPmicro:Fix_LPC55S69_Flash_ClockSpeed branch from 0871214 to 1b9531d Apr 1, 2019

@mmahadevan108

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2019

PR updated to remove binary

@orenc17

orenc17 approved these changes Apr 1, 2019

Binary removed.

@cmonr cmonr added needs: CI and removed needs: work labels Apr 1, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Apr 2, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 1
Build artifacts

@cmonr cmonr merged commit 4950178 into ARMmbed:master Apr 2, 2019

28 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARMC5 Success
Details
jenkins-ci/mbed2-build-ARMC6 Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR8 Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9197 cycles (-794 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Apr 2, 2019

@mmahadevan108 mmahadevan108 deleted the NXPmicro:Fix_LPC55S69_Flash_ClockSpeed branch Apr 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.