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

dts: arm: nxp: RT10xx/11xx .dtsi Files: Remove flexram,bank-spec prop #84889

Conversation

jacob-wienecke-nxp
Copy link
Contributor

@jacob-wienecke-nxp jacob-wienecke-nxp commented Jan 29, 2025

This commit removes the flexram,bank-spec property from dtsi files. The property causes flexram to be dynamically configured based on the configuration in flexram,bank-spec.

This is a problem for 2 reasons: 1) The FlexRAM will always be dynamically reconfigured to default fuse configuration. This is unnecessary if using default fuses. 2) If a user decides to program fuses. The FlexRAM will still be reconfigured to the default fuse configuration.

Modify description in the binding to show how to use the property at:
dts/bindings/memory-controllers/nxp,flexram.yaml

Added board overlay to mimxrt1170_evk_cm7 magic_addr:
samples/boards/nxp/mimxrt1170_evk_cm7/magic_addr/boards

@decsny
Copy link
Member

decsny commented Jan 30, 2025

Hey @jacob-wienecke-nxp, I have a question regarding this PR. I have manually resized my ITCM and DTCM regions on RT1061 to make my code and data fit in those internal TCMs, as @DerekSnell suggested me here, using the following configuration:

&itcm {
        reg = < 0x0 DT_SIZE_K(160) >;
};
&dtcm {
        reg = < 0x20000000 DT_SIZE_K(288) >;
};
&ocram {
        reg = < 0x20280000 DT_SIZE_K(64) >;
};
&flexram {
        flexram,bank-spec = <FLEXRAM_OCRAM>,
                            <FLEXRAM_OCRAM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_DTCM>,
                            <FLEXRAM_ITCM>,
                            <FLEXRAM_ITCM>,
                            <FLEXRAM_ITCM>,
                            <FLEXRAM_ITCM>,
                            <FLEXRAM_ITCM>;
};

So my question is after the merge of this PR, how this resize should be facilitated ?

Thanks.

The PR doesn't remove the property from the binding, just the property usage from the SOC DTSI files, so you don't need to change anything

Copy link
Member

@decsny decsny left a comment

Choose a reason for hiding this comment

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

change request: add description to DT binding for flexram about how to use it

also, maybe add an overlay to the magic-addr sample to have CI for at least one example of dynamic reconfigure in tree, otherwise we have no CI coverage of this feature at all

@decsny decsny added this to the v4.1.0 milestone Feb 3, 2025
@zephyrbot zephyrbot added area: MEMC platform: NXP Drivers NXP Semiconductors, drivers labels Feb 17, 2025
@jacob-wienecke-nxp jacob-wienecke-nxp force-pushed the remove_flexram,bank-spec_property branch 2 times, most recently from 72319ab to 1792915 Compare February 17, 2025 22:11
…erty

This commit removes the flexram,bank-spec property from dtsi files.
The property causes flexram to be dynamically configured based on
the configuration in flexram,bank-spec. This is a problem for 2 reasons:
1) The FlexRAM will always be dynamically reconfigured to default
fuse configuration. This is unnecessary if using default fuses.
2) If a user decides to program fuses. The FlexRAM will still be
reconfigured to the default fuse configuration.

Modify description in the binding to show how to use the property at:
dts/bindings/memory-controllers/nxp,flexram.yaml

Added board overlay to mimxrt1170_evk_cm7 magic_addr:
samples/boards/nxp/mimxrt1170_evk_cm7/magic_addr/boards

Signed-off-by: Jacob Wienecke <[email protected]>
@jacob-wienecke-nxp jacob-wienecke-nxp force-pushed the remove_flexram,bank-spec_property branch from 1792915 to de92e22 Compare February 18, 2025 15:47
@zephyrbot zephyrbot added the area: Samples Samples label Feb 18, 2025
@zephyrbot zephyrbot requested review from kartben and nashif February 18, 2025 15:48
@decsny
Copy link
Member

decsny commented Feb 19, 2025

@kartben is it possible we can still add this to 4.1 without TSC approval (once it gets the approvals)?

It's not fixing a bug, but it's:

  • improving the documentation of the binding
  • reducing the usage of this feature in the tree, so that it is not used by default (the platform boot is significantly more robust if this feature is not enabled, because then it relies on the chip metal ROM instead of the zephyr image to configure RAM banks)
  • Adding an overlay to a relevant sample so CI still covers the feature

@kartben
Copy link
Collaborator

kartben commented Feb 19, 2025

It actually sounds to me like it's fixing a bug—point #2 in the PR description—with #1 also being something that could be considered a bug (unnecessary code being pulled in / executed) so fine with me!

cc @fabiobaltieri

@kartben kartben merged commit 9e1bf0b into zephyrproject-rtos:main Feb 20, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants