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

[nrf noup] platform: nordic_nrf: nRF54L15 does not have UICR #173

Conversation

MarkusLassila
Copy link
Contributor

@MarkusLassila MarkusLassila commented Oct 30, 2024

Do not attempt to lock the UICR.APPROTECT or UICR.SECUREAPPROTECT for nRF54L15. These registers do not exist.

TF-M provisioning with nRF54L15 will still result in compilation failure. As it should at this point.

Copy link
Contributor

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

No Change-Id in noups.

@MarkusLassila MarkusLassila force-pushed the noup-tfm-disable-uicr-locking-for-nRF54L15 branch from 944c9a3 to d9c8a50 Compare October 30, 2024 09:00
@@ -814,7 +814,7 @@ enum tfm_plat_err_t init_debug(void)
#error "Debug access controlled by NRF_APPROTECT and NRF_SECURE_APPROTECT."
#endif

#if defined(NRF_APPROTECT)
#if defined(NRF_APPROTECT) && !defined(NRF54L15_XXAA)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about checking e.g. for NRF_UICR_S instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or some register/define in particular under UICR (APPROTECT, SECUREAPPROTECT).

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 think that we should use the devices as we are using them elsewhere. This is maybe something that could be changed when we know how the nRF54L15 is going to look. We want it to lock debugger permanently, but it is not supported in 2.8.

Copy link
Contributor

Choose a reason for hiding this comment

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

though we are using defined(NRF_APPROTECT) here already 🤔

Do not attempt to lock the UICR.APPROTECT or UICR.SECUREAPPROTECT
for nRF54L15. These registers do not exist.

TF-M provisioning with nRF54L15 will still result in compilation
failure. As it should at this point.

Signed-off-by: Markus Lassila <[email protected]>
@MarkusLassila MarkusLassila force-pushed the noup-tfm-disable-uicr-locking-for-nRF54L15 branch from d9c8a50 to a68f3e6 Compare October 30, 2024 09:02
Copy link
Contributor

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

TF-M provisioning with nRF54L15 will still result in compilation
failure. As it should at this point.

I don't see what's the link between this and what you're doing here?

@MarkusLassila
Copy link
Contributor Author

TF-M provisioning with nRF54L15 will still result in compilation
failure. As it should at this point.

I don't see what's the link between this and what you're doing here?

nrf_provisioning.c checks that these UICR registers are set. This will still cause compilation error for nRF54L15.

@nika-nordic
Copy link
Contributor

let me check this change with my PR: nrfconnect/sdk-zephyr#2162 before merging this

@nika-nordic
Copy link
Contributor

LGTM, but I will approve once CI on nrfconnect/sdk-nrf#18370 is green.

Testing done locally went fine and @MarkusLassila took a look at that as well

@MarkusLassila
Copy link
Contributor Author

@nordicjm: When the nrfconnect/sdk-zephyr#2162 is backported to v2.8 branch, do we need to have a corresponding branch for this as well? I am not sure how that should be achieved.

@nordicjm
Copy link
Contributor

@nordicjm: When the nrfconnect/sdk-zephyr#2162 is backported to v2.8 branch, do we need to have a corresponding branch for this as well? I am not sure how that should be achieved.

You should have a branch already where the tag for rc2 was made from

@MarkusLassila
Copy link
Contributor Author

MarkusLassila commented Oct 30, 2024

@nordicjm: When the nrfconnect/sdk-zephyr#2162 is backported to v2.8 branch, do we need to have a corresponding branch for this as well? I am not sure how that should be achieved.

You should have a branch already where the tag for rc2 was made from

So, likely this one: v2.1.1-ncs1-branch

Copy link
Contributor

@nika-nordic nika-nordic left a comment

Choose a reason for hiding this comment

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

sdk-nrf PR is green with APPROTECT changes

@rlubos
Copy link
Contributor

rlubos commented Oct 31, 2024

@MarkusLassila @nika-nordic This will need to be manually backported

@rlubos rlubos merged commit 5ae4c7f into nrfconnect:main Oct 31, 2024
@MarkusLassila
Copy link
Contributor Author

@MarkusLassila @nika-nordic This will need to be manually backported

@rlubos @nika-nordic: 2.8 available in here: #174

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.

6 participants