Introduce an Mbed config to enable XIP #11006

linlingao · 2019-07-09T21:44:37Z

Description

Introduce an Mbed config to enable XIP (Execute In Place) capability on some boards. XIP can be enabled by adding "target.xip-enable": true in mbed_app.json. It's disabled by default.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

Introduce xip-enable config to allow text/RO data to be loaded to QSPI flash where XIP (execute-in-place) is supported. This will allow programs with larger text/RO data section to run on a system with smaller on-chip flash.

This feature is disabled by default on all targets.

XIP can be enabled by adding target.xip-enable: true in mbed_app.json, then locate desired text/RO data sections to QSPI flash region in the linker script. In the boot sequence, QSPI flash needs to be configured to XIP mode.

ciarmcom · 2019-07-09T23:00:52Z

@linlingao, thank you for your changes.
@ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-ipcore please review.

40Grit · 2019-07-10T07:58:54Z

@AGlass0fMilk

mark-edgeworth · 2019-07-10T08:36:53Z

tools/toolchains/mbed_toolchain.py

@@ -957,6 +957,12 @@ def add_linker_defines(self):
                self.ld.append(define_string)
                self.flags["ld"].append(define_string)

+        if "XIP_ENABLE" in self.target.macros :


Please comment on what XIP is, and why this special case needs to be added here.

XIP stands for Execute In Place. Since the text/RO data gets too big to fit on on chip flash, we need to move some code to the QSPI to execute for bigger applications such as Pelion cloud client. This feature is designed to be enabled in mbed_app.json because each application usually has its own JSON. By default it's disabled, because if the text/RO data can fit on the internal flash, there's no need to enable this feature.

mark-edgeworth

This PR is enormous - 764 changed files!
I can only look at the tools changes; these look to be minor, but contribute to an increasing pool of special cases, each of which make maintaining the tools code more and more difficult.
I would like to see some comments to indicate what this changed code is actually doing and why.

0xc0170 · 2019-07-10T10:32:32Z

This PR is enormous - 764 changed files!

+1, would be good to indicate which commits are actually for review here (range of them if multiple). Otherwise most of us just wait until the rebase is done once PR dependency is in

0xc0170 · 2019-07-10T10:34:35Z

This is the commit: b075f76 for review.

What does this bring? Are there any implications one enabled?
Why XIP_ENABLE, is this part of QSPI, should it be addition to the current driver ? We rather should use driver config or somehow differently than using "macro" to enable it. Isn't this QSPI feature?

linlingao · 2019-07-10T15:40:09Z

Yes, only b075f76 needs to be reviewed. It's fine to wait till the rebase.

0xc0170 · 2019-07-11T08:06:08Z

Can you introduce XIP - answer my questions above.

linlingao · 2019-07-11T14:18:26Z

Hope this helps:
https://en.wikipedia.org/wiki/Execute_in_place
XIP is a mode on the QSPI. I don't see how this can be done in the QSPI driver alone since this is really a systematic feature.
We need to relocate some code to QSPI, the only place to do that is via the linker.

artokin · 2019-07-15T13:09:50Z

@linlingao , #10692 is now merged, would you please rebase?

maclobdell · 2019-07-16T19:29:41Z

@ARMmbed/team-cypress - please review after this is rebased

linlingao · 2019-07-17T15:39:50Z

Rebased. Reviewers, please review.

bulislaw

Could you please connect it to Mbed config system, similarly to MBED_CONF_TARGET_LSE_AVAILABLE which can be found in target.json as lse_available in config section.

linlingao · 2019-07-30T16:49:25Z

@bulislaw I looked at how lse_available is used in targets.json. lse_available only applies to FAMILY_STM32. The default is "1", but on some targets, it's overridden to 0. That is, lse_available is the same setting on a certain target for all applications.

I'm not seeing close resemblance between lse_available and XIP_ENABLE. There're two differentiating points with respect to the intended usage of XIP_ENABLE:

(1) XIP_ENABLE can be used for ALL targets. For example, on the Nordic boards with XIP, if someone wants to enable XIP, he/she doesn't need to introduce another variable.
(2) XIP_ENABLE is application specific setting. Which means, on a board with XIP capability, one can choose in enable XIP in one application such as cloud client example when the on-chip flash is not big enough, but disable XIP in smaller applications when on-chip flash is more than enough.

bulislaw · 2019-07-31T10:34:34Z

(1) XIP_ENABLE can be used for ALL targets. For example, on the Nordic boards with XIP, if someone wants to enable XIP, he/she doesn't need to introduce another variable.
(2) XIP_ENABLE is application specific setting. Which means, on a board with XIP capability, one can choose in enable XIP in one application such as cloud client example when the on-chip flash is not big enough, but disable XIP in smaller applications when on-chip flash is more than enough.

Even more reasons to have it as system config rather than a 'random' flag.

linlingao · 2019-08-30T14:09:38Z

I had to wait for other PRs to go in first. It's fine to push this out to a minor release.

…P_ENABLE in mbed_app.json. It's disabled by default.

linlingao · 2019-09-03T21:22:36Z

XIP feature has already been added by Cypress in other PRs. Reduce the scope of this PR to just add the Mbed config.

0xc0170 · 2019-09-04T08:44:22Z

To understand the scope now - this provides a config option to enable/disable it. There're Cypress targets already providing all needed, and other targets can follow. This PR should be ready then ?

linlingao · 2019-09-04T14:23:06Z

@0xc0170 Yes

linlingao · 2019-09-04T14:30:24Z

@morser499 Please review the change in mbed_override.c

morser499 · 2019-09-04T14:34:39Z

Looks good to me

0xc0170 · 2019-09-06T13:53:21Z

CI started

mbed-ci · 2019-09-06T15:25:56Z

Test run: FAILED

Summary: 2 of 4 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM

mbed-ci · 2019-09-06T16:35:12Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 7
Build artifacts

ciarmcom requested review from a team July 9, 2019 23:00

ciarmcom added the needs: review label Jul 9, 2019

mark-edgeworth reviewed Jul 10, 2019

View reviewed changes

0xc0170 requested review from a team and removed request for a team July 10, 2019 10:33

0xc0170 requested a review from a team July 11, 2019 07:59

maclobdell mentioned this pull request Jul 16, 2019

Cypress QSPI and QSPIF removed #11061

Closed

SeppoTakalo added needs: work and removed needs: review labels Jul 17, 2019

linlingao force-pushed the pr10692 branch from b075f76 to 26da29f Compare July 17, 2019 15:38

bulislaw suggested changes Jul 24, 2019

View reviewed changes

linlingao added 4 commits September 3, 2019 11:54

Add XIP capability, enable QSPI. XIP can be enable by adding macro XI…

ea032be

…P_ENABLE in mbed_app.json. It's disabled by default.

Refactored code to not use macro. Created config xip-enable

2a78a9b

Remove QSPI for Future targets

b9f8e2a

Remove keep keyword for .heap section

f1813e5

linlingao force-pushed the pr10692 branch from f92d3f9 to f1813e5 Compare September 3, 2019 20:36

removed xip unrelated changes

67c6bdb

linlingao changed the title ~~Add XIP capability~~ Add Mbed config to enable XIP Sep 3, 2019

linlingao changed the title ~~Add Mbed config to enable XIP~~ Introduce an Mbed config to enable XIP Sep 3, 2019

0xc0170 approved these changes Sep 4, 2019

View reviewed changes

morser499 approved these changes Sep 4, 2019

View reviewed changes

0xc0170 added needs: CI release-version: 5.15.0-rc1 and removed needs: review labels Sep 5, 2019

0xc0170 added ready for merge and removed needs: CI labels Sep 9, 2019

0xc0170 merged commit 4585232 into ARMmbed:master Sep 9, 2019

0xc0170 removed the ready for merge label Sep 9, 2019

adbridge added release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed release-version: 5.15.0-rc1 labels Oct 3, 2019

0xc0170 added release-version: 5.15.0-rc1 and removed release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Nov 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce an Mbed config to enable XIP #11006

Introduce an Mbed config to enable XIP #11006

linlingao commented Jul 9, 2019 •

edited

Loading

ciarmcom commented Jul 9, 2019

40Grit commented Jul 10, 2019

mark-edgeworth Jul 10, 2019 •

edited

Loading

linlingao Jul 10, 2019

mark-edgeworth left a comment •

edited

Loading

0xc0170 commented Jul 10, 2019

0xc0170 commented Jul 10, 2019 •

edited

Loading

linlingao commented Jul 10, 2019

0xc0170 commented Jul 11, 2019

linlingao commented Jul 11, 2019

artokin commented Jul 15, 2019

maclobdell commented Jul 16, 2019 •

edited

Loading

linlingao commented Jul 17, 2019 •

edited

Loading

bulislaw left a comment

linlingao commented Jul 30, 2019

bulislaw commented Jul 31, 2019

linlingao commented Aug 30, 2019

linlingao commented Sep 3, 2019

0xc0170 commented Sep 4, 2019

linlingao commented Sep 4, 2019

linlingao commented Sep 4, 2019

morser499 commented Sep 4, 2019

0xc0170 commented Sep 6, 2019

mbed-ci commented Sep 6, 2019

mbed-ci commented Sep 6, 2019

Introduce an Mbed config to enable XIP #11006

Introduce an Mbed config to enable XIP #11006

Conversation

linlingao commented Jul 9, 2019 • edited Loading

Description

Pull request type

Reviewers

Release Notes

ciarmcom commented Jul 9, 2019

40Grit commented Jul 10, 2019

mark-edgeworth Jul 10, 2019 • edited Loading

Choose a reason for hiding this comment

linlingao Jul 10, 2019

Choose a reason for hiding this comment

mark-edgeworth left a comment • edited Loading

Choose a reason for hiding this comment

0xc0170 commented Jul 10, 2019

0xc0170 commented Jul 10, 2019 • edited Loading

linlingao commented Jul 10, 2019

0xc0170 commented Jul 11, 2019

linlingao commented Jul 11, 2019

artokin commented Jul 15, 2019

maclobdell commented Jul 16, 2019 • edited Loading

linlingao commented Jul 17, 2019 • edited Loading

bulislaw left a comment

Choose a reason for hiding this comment

linlingao commented Jul 30, 2019

bulislaw commented Jul 31, 2019

linlingao commented Aug 30, 2019

linlingao commented Sep 3, 2019

0xc0170 commented Sep 4, 2019

linlingao commented Sep 4, 2019

linlingao commented Sep 4, 2019

morser499 commented Sep 4, 2019

0xc0170 commented Sep 6, 2019

mbed-ci commented Sep 6, 2019

Test run: FAILED

mbed-ci commented Sep 6, 2019

Test run: SUCCESS

linlingao commented Jul 9, 2019 •

edited

Loading

mark-edgeworth Jul 10, 2019 •

edited

Loading

mark-edgeworth left a comment •

edited

Loading

0xc0170 commented Jul 10, 2019 •

edited

Loading

maclobdell commented Jul 16, 2019 •

edited

Loading

linlingao commented Jul 17, 2019 •

edited

Loading