Skip to content

Conversation

degjorva
Copy link

@degjorva degjorva commented Sep 9, 2025

Add board files for nRF54LM20A/ns.
Update existing nRF54LM20A board files to support this.

Copy link

github-actions bot commented Sep 9, 2025

Hello @degjorva, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

Comment on lines 66 to 67
/* Get a node label for wi-fi spi to use in shield files */
wifi_spi: &spi22 {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move it from nrf54lm20dk_nrf54lm20a_cpuapp.dts to nrf54lm20dk_nrf54lm20a-common.dtsi?

tomi-font
tomi-font previously approved these changes Sep 29, 2025
};
};
};
/* TF-M partitions are defined in the NS board DTS file */
Copy link
Contributor

Choose a reason for hiding this comment

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

comment not needed and doesn't even apply to secure board target

Copy link
Author

Choose a reason for hiding this comment

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

Removed

Comment on lines 58 to 65
&bt_hci_controller {
status = "disabled";
};

&uart30 {
/* Disable so that TF-M can use this UART */
status = "disabled";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

why can't these definitions be moved from the common file to the normal cpuapp file so that they don't need to be disabled here?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

zephyr,entropy = &psa_rng;
};

/delete-node/ rng;
Copy link
Contributor

Choose a reason for hiding this comment

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

as other comment

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines +25 to +26
/* nRF54LM20A has 2036 kB of non-volatile memory (RRAM) but the last
* 96 kB are reserved for the FLPR MCU.
Copy link
Contributor

Choose a reason for hiding this comment

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

no, no no no, we agreed not to do this again @carlescufi

Copy link
Author

Choose a reason for hiding this comment

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

Right this comment contains no useful information. What did we agree not to do, with who, when, where? What is supposed to be done differently? Why? How is it supposed to be done instead? Where was that added as a requirement for the implementation of TF-m?

Copy link
Contributor

Choose a reason for hiding this comment

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

no flpr reservation in normal builds without flpr, use full RRAM for application core

Copy link
Author

Choose a reason for hiding this comment

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

That will mean the upstream TF-m flash map will cover the entire RRAM as well. I am no expert on the FLPR, but won't that cause issues with the reserved memory?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tomi-font for comment, for non-TF-M i.e. the normal cpuapp it has no impact from TF-M

Copy link
Contributor

Choose a reason for hiding this comment

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

? Honestly I have no idea, and here I believe @degjorva is just copying what's done for nRF54L15. What are you even asking @nordicjm ? + cc @Vge0rge @frkv

Copy link
Contributor

Choose a reason for hiding this comment

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

should a flpr section be removed from the end of NVM in the TF-M images i.e. is that what has been done with the upstream TF-M partition file?

Copy link
Author

Choose a reason for hiding this comment

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

In upstream TF-m we use the layout that is shown in this PR, it is the layout we have used for all 54L-series devices. As far as I can understand from how dts works that is also how it needs to be due to the FLPR core, but I could be wrong. I checked with @Vge0rge which did not have any more information.

Copy link
Author

@degjorva degjorva Oct 9, 2025

Choose a reason for hiding this comment

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

Since no one knows I will stick with this here and create a follow up task to investigate what the correct solution is in regards to the FLPR sections and if they are not needed to remove them from all 54L devices. @Vge0rge @tomi-font @nordicjm

Comment on lines +13 to +15
/* nRF54LM20A has 2036 kB of non-volatile memory (RRAM) but the last
* 96 kB are reserved for the FLPR MCU, so we have ~1940 kB available.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

as above, no

Copy link
Author

Choose a reason for hiding this comment

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

Same as above

Copy link

github-actions bot commented Oct 9, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
trusted-firmware-m zephyrproject-rtos/trusted-firmware-m@3e12b0c zephyrproject-rtos/trusted-firmware-m#150 zephyrproject-rtos/trusted-firmware-m#150/files

DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@github-actions github-actions bot added manifest manifest-trusted-firmware-m DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Oct 9, 2025
@degjorva degjorva requested a review from tomi-font October 9, 2025 14:27
@degjorva
Copy link
Author

Rebase

Update commit id for trusted-firmware-m
to bring in support for nRF54LM20A/ns.

Signed-off-by: Dag Erik Gjørvad <[email protected]>
Add board files for nRF54LM20A/ns.
Update existing nRF54LM20A board files to support this.

Signed-off-by: Dag Erik Gjørvad <[email protected]>
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards/SoCs area: TF-M ARM Trusted Firmware-M (TF-M) DNM (manifest) This PR should not be merged (controlled by action-manifest) manifest manifest-trusted-firmware-m platform: nRF Nordic nRFx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants