From 3f88bf6f9b998e209c6bfe860930fc82516f3f9c Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 23 Jan 2023 01:48:35 +0100 Subject: [PATCH 1/2] nrf: add support for UICR configuration. - APPROTECT enable/disable. Notably this fixes issues with nrf52-rev3 and nrf53 from locking itself at reset. - Use NFC pins as GPIO. - Use RESET pin as GPIO. NFC and RESET pins singletons are made available only when usable as GPIO, for compile-time checking. --- ci.sh | 4 +- embassy-nrf/Cargo.toml | 35 ++++-- embassy-nrf/src/chips/nrf52805.rs | 4 + embassy-nrf/src/chips/nrf52810.rs | 4 + embassy-nrf/src/chips/nrf52811.rs | 4 + embassy-nrf/src/chips/nrf52820.rs | 4 + embassy-nrf/src/chips/nrf52832.rs | 8 ++ embassy-nrf/src/chips/nrf52833.rs | 8 ++ embassy-nrf/src/chips/nrf52840.rs | 8 ++ embassy-nrf/src/chips/nrf5340_app.rs | 4 + embassy-nrf/src/lib.rs | 156 +++++++++++++++++++++++++++ embassy-nrf/src/nvmc.rs | 12 +-- 12 files changed, 232 insertions(+), 19 deletions(-) diff --git a/ci.sh b/ci.sh index 4199f91d..417937d0 100755 --- a/ci.sh +++ b/ci.sh @@ -46,8 +46,8 @@ cargo batch \ --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv7em-none-eabi --features nightly,nrf52810,gpiote,time-driver-rtc1 \ --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv7em-none-eabi --features nightly,nrf52811,gpiote,time-driver-rtc1 \ --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv7em-none-eabi --features nightly,nrf52820,gpiote,time-driver-rtc1 \ - --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv7em-none-eabi --features nightly,nrf52832,gpiote,time-driver-rtc1 \ - --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv7em-none-eabi --features nightly,nrf52833,gpiote,time-driver-rtc1,unstable-traits \ + --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv7em-none-eabi --features nightly,nrf52832,gpiote,time-driver-rtc1,reset-pin-as-gpio \ + --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv7em-none-eabi --features nightly,nrf52833,gpiote,time-driver-rtc1,unstable-traits,nfc-pins-as-gpio \ --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv8m.main-none-eabihf --features nightly,nrf9160-s,gpiote,time-driver-rtc1 \ --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv8m.main-none-eabihf --features nightly,nrf9160-ns,gpiote,time-driver-rtc1,unstable-traits \ --- build --release --manifest-path embassy-nrf/Cargo.toml --target thumbv8m.main-none-eabihf --features nightly,nrf5340-app-s,gpiote,time-driver-rtc1,unstable-traits \ diff --git a/embassy-nrf/Cargo.toml b/embassy-nrf/Cargo.toml index 6b06d5d0..c31ce199 100644 --- a/embassy-nrf/Cargo.toml +++ b/embassy-nrf/Cargo.toml @@ -34,22 +34,30 @@ unstable-pac = [] # Implement embedded-hal-async traits if `nightly` is set as well. unstable-traits = ["embedded-hal-1"] -nrf52805 = ["nrf52805-pac", "_ppi"] -nrf52810 = ["nrf52810-pac", "_ppi"] -nrf52811 = ["nrf52811-pac", "_ppi"] -nrf52820 = ["nrf52820-pac", "_ppi"] -nrf52832 = ["nrf52832-pac", "_ppi"] -nrf52833 = ["nrf52833-pac", "_ppi", "_gpio-p1"] -nrf52840 = ["nrf52840-pac", "_ppi", "_gpio-p1"] -nrf5340-app-s = ["_nrf5340-app"] -nrf5340-app-ns = ["_nrf5340-app"] +nrf52805 = ["nrf52805-pac", "_nrf52"] +nrf52810 = ["nrf52810-pac", "_nrf52"] +nrf52811 = ["nrf52811-pac", "_nrf52"] +nrf52820 = ["nrf52820-pac", "_nrf52"] +nrf52832 = ["nrf52832-pac", "_nrf52"] +nrf52833 = ["nrf52833-pac", "_nrf52", "_gpio-p1"] +nrf52840 = ["nrf52840-pac", "_nrf52", "_gpio-p1"] +nrf5340-app-s = ["_nrf5340-app", "_s"] +nrf5340-app-ns = ["_nrf5340-app", "_ns"] nrf5340-net = ["_nrf5340-net"] -nrf9160-s = ["_nrf9160"] -nrf9160-ns = ["_nrf9160"] +nrf9160-s = ["_nrf9160", "_s"] +nrf9160-ns = ["_nrf9160", "_ns"] gpiote = [] time-driver-rtc1 = ["_time-driver"] +# Allow using the NFC pins as regular GPIO pins (P0_09/P0_10 on nRF52, P0_02/P0_03 on nRF53) +nfc-pins-as-gpio = [] + +# Allow using the RST pin as a regular GPIO pin. +# nrf52805, nrf52810, nrf52811, nrf52832: P0_21 +# nrf52820, nrf52833, nrf52840: P0_18 +reset-pin-as-gpio = [] + # Features starting with `_` are for internal use only. They're not intended # to be enabled by other crates, and are not covered by semver guarantees. @@ -57,9 +65,14 @@ _nrf5340-app = ["_nrf5340", "nrf5340-app-pac"] _nrf5340-net = ["_nrf5340", "nrf5340-net-pac"] _nrf5340 = ["_gpio-p1", "_dppi"] _nrf9160 = ["nrf9160-pac", "_dppi"] +_nrf52 = ["_ppi"] _time-driver = ["dep:embassy-time", "embassy-time?/tick-hz-32_768"] +# trustzone state. +_s = [] +_ns = [] + _ppi = [] _dppi = [] _gpio-p1 = [] diff --git a/embassy-nrf/src/chips/nrf52805.rs b/embassy-nrf/src/chips/nrf52805.rs index bf4019c1..3c74a2a6 100644 --- a/embassy-nrf/src/chips/nrf52805.rs +++ b/embassy-nrf/src/chips/nrf52805.rs @@ -6,6 +6,8 @@ pub const FORCE_COPY_BUFFER_SIZE: usize = 256; pub const FLASH_SIZE: usize = 192 * 1024; +pub const RESET_PIN: u32 = 21; + embassy_hal_common::peripherals! { // RTC RTC0, @@ -108,6 +110,7 @@ embassy_hal_common::peripherals! { P0_18, P0_19, P0_20, + #[cfg(feature="reset-pin-as-gpio")] P0_21, P0_22, P0_23, @@ -162,6 +165,7 @@ impl_pin!(P0_17, 0, 17); impl_pin!(P0_18, 0, 18); impl_pin!(P0_19, 0, 19); impl_pin!(P0_20, 0, 20); +#[cfg(feature = "reset-pin-as-gpio")] impl_pin!(P0_21, 0, 21); impl_pin!(P0_22, 0, 22); impl_pin!(P0_23, 0, 23); diff --git a/embassy-nrf/src/chips/nrf52810.rs b/embassy-nrf/src/chips/nrf52810.rs index 6c28a3be..6b5c134b 100644 --- a/embassy-nrf/src/chips/nrf52810.rs +++ b/embassy-nrf/src/chips/nrf52810.rs @@ -6,6 +6,8 @@ pub const FORCE_COPY_BUFFER_SIZE: usize = 256; pub const FLASH_SIZE: usize = 192 * 1024; +pub const RESET_PIN: u32 = 21; + embassy_hal_common::peripherals! { // RTC RTC0, @@ -111,6 +113,7 @@ embassy_hal_common::peripherals! { P0_18, P0_19, P0_20, + #[cfg(feature="reset-pin-as-gpio")] P0_21, P0_22, P0_23, @@ -170,6 +173,7 @@ impl_pin!(P0_17, 0, 17); impl_pin!(P0_18, 0, 18); impl_pin!(P0_19, 0, 19); impl_pin!(P0_20, 0, 20); +#[cfg(feature = "reset-pin-as-gpio")] impl_pin!(P0_21, 0, 21); impl_pin!(P0_22, 0, 22); impl_pin!(P0_23, 0, 23); diff --git a/embassy-nrf/src/chips/nrf52811.rs b/embassy-nrf/src/chips/nrf52811.rs index e7214cf5..c5de9a44 100644 --- a/embassy-nrf/src/chips/nrf52811.rs +++ b/embassy-nrf/src/chips/nrf52811.rs @@ -6,6 +6,8 @@ pub const FORCE_COPY_BUFFER_SIZE: usize = 256; pub const FLASH_SIZE: usize = 192 * 1024; +pub const RESET_PIN: u32 = 21; + embassy_hal_common::peripherals! { // RTC RTC0, @@ -111,6 +113,7 @@ embassy_hal_common::peripherals! { P0_18, P0_19, P0_20, + #[cfg(feature="reset-pin-as-gpio")] P0_21, P0_22, P0_23, @@ -172,6 +175,7 @@ impl_pin!(P0_17, 0, 17); impl_pin!(P0_18, 0, 18); impl_pin!(P0_19, 0, 19); impl_pin!(P0_20, 0, 20); +#[cfg(feature = "reset-pin-as-gpio")] impl_pin!(P0_21, 0, 21); impl_pin!(P0_22, 0, 22); impl_pin!(P0_23, 0, 23); diff --git a/embassy-nrf/src/chips/nrf52820.rs b/embassy-nrf/src/chips/nrf52820.rs index 21d1d16c..81b07f32 100644 --- a/embassy-nrf/src/chips/nrf52820.rs +++ b/embassy-nrf/src/chips/nrf52820.rs @@ -6,6 +6,8 @@ pub const FORCE_COPY_BUFFER_SIZE: usize = 512; pub const FLASH_SIZE: usize = 256 * 1024; +pub const RESET_PIN: u32 = 18; + embassy_hal_common::peripherals! { // USB USBD, @@ -106,6 +108,7 @@ embassy_hal_common::peripherals! { P0_15, P0_16, P0_17, + #[cfg(feature="reset-pin-as-gpio")] P0_18, P0_19, P0_20, @@ -168,6 +171,7 @@ impl_pin!(P0_14, 0, 14); impl_pin!(P0_15, 0, 15); impl_pin!(P0_16, 0, 16); impl_pin!(P0_17, 0, 17); +#[cfg(feature = "reset-pin-as-gpio")] impl_pin!(P0_18, 0, 18); impl_pin!(P0_19, 0, 19); impl_pin!(P0_20, 0, 20); diff --git a/embassy-nrf/src/chips/nrf52832.rs b/embassy-nrf/src/chips/nrf52832.rs index 152dad4e..c2b23fc5 100644 --- a/embassy-nrf/src/chips/nrf52832.rs +++ b/embassy-nrf/src/chips/nrf52832.rs @@ -10,6 +10,8 @@ pub const FORCE_COPY_BUFFER_SIZE: usize = 255; // nrf52832xxAB = 256kb pub const FLASH_SIZE: usize = 512 * 1024; +pub const RESET_PIN: u32 = 21; + embassy_hal_common::peripherals! { // RTC RTC0, @@ -109,7 +111,9 @@ embassy_hal_common::peripherals! { P0_06, P0_07, P0_08, + #[cfg(feature = "nfc-pins-as-gpio")] P0_09, + #[cfg(feature = "nfc-pins-as-gpio")] P0_10, P0_11, P0_12, @@ -121,6 +125,7 @@ embassy_hal_common::peripherals! { P0_18, P0_19, P0_20, + #[cfg(feature="reset-pin-as-gpio")] P0_21, P0_22, P0_23, @@ -178,7 +183,9 @@ impl_pin!(P0_05, 0, 5); impl_pin!(P0_06, 0, 6); impl_pin!(P0_07, 0, 7); impl_pin!(P0_08, 0, 8); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_09, 0, 9); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_10, 0, 10); impl_pin!(P0_11, 0, 11); impl_pin!(P0_12, 0, 12); @@ -190,6 +197,7 @@ impl_pin!(P0_17, 0, 17); impl_pin!(P0_18, 0, 18); impl_pin!(P0_19, 0, 19); impl_pin!(P0_20, 0, 20); +#[cfg(feature = "reset-pin-as-gpio")] impl_pin!(P0_21, 0, 21); impl_pin!(P0_22, 0, 22); impl_pin!(P0_23, 0, 23); diff --git a/embassy-nrf/src/chips/nrf52833.rs b/embassy-nrf/src/chips/nrf52833.rs index a99ca634..95f71ade 100644 --- a/embassy-nrf/src/chips/nrf52833.rs +++ b/embassy-nrf/src/chips/nrf52833.rs @@ -6,6 +6,8 @@ pub const FORCE_COPY_BUFFER_SIZE: usize = 512; pub const FLASH_SIZE: usize = 512 * 1024; +pub const RESET_PIN: u32 = 18; + embassy_hal_common::peripherals! { // USB USBD, @@ -111,7 +113,9 @@ embassy_hal_common::peripherals! { P0_06, P0_07, P0_08, + #[cfg(feature = "nfc-pins-as-gpio")] P0_09, + #[cfg(feature = "nfc-pins-as-gpio")] P0_10, P0_11, P0_12, @@ -120,6 +124,7 @@ embassy_hal_common::peripherals! { P0_15, P0_16, P0_17, + #[cfg(feature="reset-pin-as-gpio")] P0_18, P0_19, P0_20, @@ -207,7 +212,9 @@ impl_pin!(P0_05, 0, 5); impl_pin!(P0_06, 0, 6); impl_pin!(P0_07, 0, 7); impl_pin!(P0_08, 0, 8); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_09, 0, 9); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_10, 0, 10); impl_pin!(P0_11, 0, 11); impl_pin!(P0_12, 0, 12); @@ -216,6 +223,7 @@ impl_pin!(P0_14, 0, 14); impl_pin!(P0_15, 0, 15); impl_pin!(P0_16, 0, 16); impl_pin!(P0_17, 0, 17); +#[cfg(feature = "reset-pin-as-gpio")] impl_pin!(P0_18, 0, 18); impl_pin!(P0_19, 0, 19); impl_pin!(P0_20, 0, 20); diff --git a/embassy-nrf/src/chips/nrf52840.rs b/embassy-nrf/src/chips/nrf52840.rs index 4f7463be..5e7479e8 100644 --- a/embassy-nrf/src/chips/nrf52840.rs +++ b/embassy-nrf/src/chips/nrf52840.rs @@ -6,6 +6,8 @@ pub const FORCE_COPY_BUFFER_SIZE: usize = 512; pub const FLASH_SIZE: usize = 1024 * 1024; +pub const RESET_PIN: u32 = 18; + embassy_hal_common::peripherals! { // USB USBD, @@ -117,7 +119,9 @@ embassy_hal_common::peripherals! { P0_06, P0_07, P0_08, + #[cfg(feature = "nfc-pins-as-gpio")] P0_09, + #[cfg(feature = "nfc-pins-as-gpio")] P0_10, P0_11, P0_12, @@ -126,6 +130,7 @@ embassy_hal_common::peripherals! { P0_15, P0_16, P0_17, + #[cfg(feature="reset-pin-as-gpio")] P0_18, P0_19, P0_20, @@ -212,7 +217,9 @@ impl_pin!(P0_05, 0, 5); impl_pin!(P0_06, 0, 6); impl_pin!(P0_07, 0, 7); impl_pin!(P0_08, 0, 8); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_09, 0, 9); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_10, 0, 10); impl_pin!(P0_11, 0, 11); impl_pin!(P0_12, 0, 12); @@ -221,6 +228,7 @@ impl_pin!(P0_14, 0, 14); impl_pin!(P0_15, 0, 15); impl_pin!(P0_16, 0, 16); impl_pin!(P0_17, 0, 17); +#[cfg(feature = "reset-pin-as-gpio")] impl_pin!(P0_18, 0, 18); impl_pin!(P0_19, 0, 19); impl_pin!(P0_20, 0, 20); diff --git a/embassy-nrf/src/chips/nrf5340_app.rs b/embassy-nrf/src/chips/nrf5340_app.rs index c600fcbf..2b8b55e7 100644 --- a/embassy-nrf/src/chips/nrf5340_app.rs +++ b/embassy-nrf/src/chips/nrf5340_app.rs @@ -304,7 +304,9 @@ embassy_hal_common::peripherals! { // GPIO port 0 P0_00, P0_01, + #[cfg(feature = "nfc-pins-as-gpio")] P0_02, + #[cfg(feature = "nfc-pins-as-gpio")] P0_03, P0_04, P0_05, @@ -393,7 +395,9 @@ impl_timer!(TIMER2, TIMER2, TIMER2); impl_pin!(P0_00, 0, 0); impl_pin!(P0_01, 0, 1); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_02, 0, 2); +#[cfg(feature = "nfc-pins-as-gpio")] impl_pin!(P0_03, 0, 3); impl_pin!(P0_04, 0, 4); impl_pin!(P0_05, 0, 5); diff --git a/embassy-nrf/src/lib.rs b/embassy-nrf/src/lib.rs index 20e70a24..0fa1c0f6 100644 --- a/embassy-nrf/src/lib.rs +++ b/embassy-nrf/src/lib.rs @@ -24,6 +24,12 @@ )))] compile_error!("No chip feature activated. You must activate exactly one of the following features: nrf52810, nrf52811, nrf52832, nrf52833, nrf52840"); +#[cfg(all(feature = "reset-pin-as-gpio", not(feature = "_nrf52")))] +compile_error!("feature `reset-pin-as-gpio` is only valid for nRF52 series chips."); + +#[cfg(all(feature = "nfc-pins-as-gpio", not(any(feature = "_nrf52", feature = "_nrf5340-app"))))] +compile_error!("feature `nfc-pins-as-gpio` is only valid for nRF52, or nRF53's application core."); + // This mod MUST go first, so that the others see its macros. pub(crate) mod fmt; pub(crate) mod util; @@ -139,6 +145,19 @@ pub mod config { ExternalFullSwing, } + /// SWD access port protection setting. + #[non_exhaustive] + pub enum Debug { + /// Debugging is allowed (APPROTECT is disabled). Default. + Allowed, + /// Debugging is not allowed (APPROTECT is enabled). + Disallowed, + /// APPROTECT is not configured (neither to enable it or disable it). + /// This can be useful if you're already doing it by other means and + /// you don't want embassy-nrf to touch UICR. + NotConfigured, + } + /// Configuration for peripherals. Default configuration should work on any nRF chip. #[non_exhaustive] pub struct Config { @@ -152,6 +171,8 @@ pub mod config { /// Time driver interrupt priority. Should be lower priority than softdevice if used. #[cfg(feature = "_time-driver")] pub time_interrupt_priority: crate::interrupt::Priority, + /// Enable or disable the debug port. + pub debug: Debug, } impl Default for Config { @@ -166,17 +187,152 @@ pub mod config { gpiote_interrupt_priority: crate::interrupt::Priority::P0, #[cfg(feature = "_time-driver")] time_interrupt_priority: crate::interrupt::Priority::P0, + + // In NS mode, default to NotConfigured, assuming the S firmware will do it. + #[cfg(feature = "_ns")] + debug: Debug::NotConfigured, + #[cfg(not(feature = "_ns"))] + debug: Debug::Allowed, } } } } +#[cfg(feature = "_nrf9160")] +#[allow(unused)] +mod consts { + pub const UICR_APPROTECT: *mut u32 = 0x00FF8000 as *mut u32; + pub const UICR_SECUREAPPROTECT: *mut u32 = 0x00FF802C as *mut u32; + pub const APPROTECT_ENABLED: u32 = 0x0000_0000; +} + +#[cfg(feature = "_nrf5340-app")] +#[allow(unused)] +mod consts { + pub const UICR_APPROTECT: *mut u32 = 0x00FF8000 as *mut u32; + pub const UICR_SECUREAPPROTECT: *mut u32 = 0x00FF801C as *mut u32; + pub const UICR_NFCPINS: *mut u32 = 0x00FF8028 as *mut u32; + pub const APPROTECT_ENABLED: u32 = 0x0000_0000; + pub const APPROTECT_DISABLED: u32 = 0x50FA50FA; +} + +#[cfg(feature = "_nrf5340-net")] +#[allow(unused)] +mod consts { + pub const UICR_APPROTECT: *mut u32 = 0x01FF8000 as *mut u32; + pub const APPROTECT_ENABLED: u32 = 0x0000_0000; + pub const APPROTECT_DISABLED: u32 = 0x50FA50FA; +} + +#[cfg(feature = "_nrf52")] +#[allow(unused)] +mod consts { + pub const UICR_PSELRESET1: *mut u32 = 0x10001200 as *mut u32; + pub const UICR_PSELRESET2: *mut u32 = 0x10001204 as *mut u32; + pub const UICR_NFCPINS: *mut u32 = 0x1000120C as *mut u32; + pub const UICR_APPROTECT: *mut u32 = 0x10001208 as *mut u32; + pub const APPROTECT_ENABLED: u32 = 0x0000_0000; + pub const APPROTECT_DISABLED: u32 = 0x0000_005a; +} + +unsafe fn uicr_write(address: *mut u32, value: u32) -> bool { + let curr_val = address.read_volatile(); + if curr_val == value { + return false; + } + + // Writing to UICR can only change `1` bits to `0` bits. + // If this write would change `0` bits to `1` bits, we can't do it. + // It is only possible to do when erasing UICR, which is forbidden if + // APPROTECT is enabled. + if (!curr_val) & value != 0 { + panic!("Cannot write UICR address={:08x} value={:08x}", address as u32, value) + } + + let nvmc = &*pac::NVMC::ptr(); + nvmc.config.write(|w| w.wen().wen()); + while nvmc.ready.read().ready().is_busy() {} + address.write_volatile(value); + while nvmc.ready.read().ready().is_busy() {} + nvmc.config.reset(); + while nvmc.ready.read().ready().is_busy() {} + + true +} + /// Initialize peripherals with the provided configuration. This should only be called once at startup. pub fn init(config: config::Config) -> Peripherals { // Do this first, so that it panics if user is calling `init` a second time // before doing anything important. let peripherals = Peripherals::take(); + let mut needs_reset = false; + + // Setup debug protection. + match config.debug { + config::Debug::Allowed => { + #[cfg(feature = "_nrf52")] + unsafe { + let variant = (0x1000_0104 as *mut u32).read_volatile(); + // Get the letter for the build code (b'A' .. b'F') + let build_code = (variant >> 8) as u8; + + if build_code >= b'F' { + // Chips with build code F and higher (revision 3 and higher) have an + // improved APPROTECT ("hardware and software controlled access port protection") + // which needs explicit action by the firmware to keep it unlocked + + // UICR.APPROTECT = SwDisabled + needs_reset |= uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_DISABLED); + // APPROTECT.DISABLE = SwDisabled + (0x4000_0558 as *mut u32).write_volatile(consts::APPROTECT_DISABLED); + } else { + // nothing to do on older chips, debug is allowed by default. + } + } + + #[cfg(feature = "_nrf5340")] + unsafe { + let p = &*pac::CTRLAP::ptr(); + + needs_reset |= uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_DISABLED); + p.approtect.disable.write(|w| w.bits(consts::APPROTECT_DISABLED)); + + #[cfg(feature = "_nrf5340-app")] + { + needs_reset |= uicr_write(consts::UICR_SECUREAPPROTECT, consts::APPROTECT_DISABLED); + p.secureapprotect.disable.write(|w| w.bits(consts::APPROTECT_DISABLED)); + } + } + + // nothing to do on the nrf9160, debug is allowed by default. + } + config::Debug::Disallowed => unsafe { + // UICR.APPROTECT = Enabled + needs_reset |= uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_ENABLED); + #[cfg(any(feature = "_nrf5340-app", feature = "_nrf9160"))] + { + needs_reset |= uicr_write(consts::UICR_SECUREAPPROTECT, consts::APPROTECT_ENABLED); + } + }, + config::Debug::NotConfigured => {} + } + + #[cfg(all(feature = "_nrf52", not(feature = "reset-pin-as-gpio")))] + unsafe { + needs_reset |= uicr_write(consts::UICR_PSELRESET1, chip::RESET_PIN); + needs_reset |= uicr_write(consts::UICR_PSELRESET2, chip::RESET_PIN); + } + + #[cfg(all(any(feature = "_nrf52", feature = "_nrf5340-app"), feature = "nfc-pins-as-gpio"))] + unsafe { + needs_reset |= uicr_write(consts::UICR_NFCPINS, 0); + } + + if needs_reset { + cortex_m::peripheral::SCB::sys_reset(); + } + let r = unsafe { &*pac::CLOCK::ptr() }; // Start HFCLK. diff --git a/embassy-nrf/src/nvmc.rs b/embassy-nrf/src/nvmc.rs index c1ffa31a..6f48853d 100644 --- a/embassy-nrf/src/nvmc.rs +++ b/embassy-nrf/src/nvmc.rs @@ -85,23 +85,23 @@ impl<'d> Nvmc<'d> { } fn enable_erase(&self) { - #[cfg(not(any(feature = "nrf5340-app-ns", feature = "nrf9160-ns")))] + #[cfg(not(feature = "_ns"))] Self::regs().config.write(|w| w.wen().een()); - #[cfg(any(feature = "nrf5340-app-ns", feature = "nrf9160-ns"))] + #[cfg(feature = "_ns")] Self::regs().configns.write(|w| w.wen().een()); } fn enable_read(&self) { - #[cfg(not(any(feature = "nrf5340-app-ns", feature = "nrf9160-ns")))] + #[cfg(not(feature = "_ns"))] Self::regs().config.write(|w| w.wen().ren()); - #[cfg(any(feature = "nrf5340-app-ns", feature = "nrf9160-ns"))] + #[cfg(feature = "_ns")] Self::regs().configns.write(|w| w.wen().ren()); } fn enable_write(&self) { - #[cfg(not(any(feature = "nrf5340-app-ns", feature = "nrf9160-ns")))] + #[cfg(not(feature = "_ns"))] Self::regs().config.write(|w| w.wen().wen()); - #[cfg(any(feature = "nrf5340-app-ns", feature = "nrf9160-ns"))] + #[cfg(feature = "_ns")] Self::regs().configns.write(|w| w.wen().wen()); } } From 7fa478358ac117557af778d12d7812da5521fdfd Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 20 Feb 2023 01:29:12 +0100 Subject: [PATCH 2/2] nrf: warn if uicr configuration could not be written. If the user requests some configuration, but UICR is already programmed to something else, detect this and warn the user. We don't do it for the debug port settings, because if they are wrong then the user will simply not be able to read debug logs. --- embassy-nrf/src/lib.rs | 78 +++++++++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 13 deletions(-) diff --git a/embassy-nrf/src/lib.rs b/embassy-nrf/src/lib.rs index 0fa1c0f6..a9683df4 100644 --- a/embassy-nrf/src/lib.rs +++ b/embassy-nrf/src/lib.rs @@ -235,10 +235,26 @@ mod consts { pub const APPROTECT_DISABLED: u32 = 0x0000_005a; } -unsafe fn uicr_write(address: *mut u32, value: u32) -> bool { +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +enum WriteResult { + /// Word was written successfully, needs reset. + Written, + /// Word was already set to the value we wanted to write, nothing was done. + Noop, + /// Word is already set to something else, we couldn't write the desired value. + Failed, +} + +unsafe fn uicr_write(address: *mut u32, value: u32) -> WriteResult { let curr_val = address.read_volatile(); if curr_val == value { - return false; + return WriteResult::Noop; + } + + // We can only change `1` bits to `0` bits. + if curr_val & value != value { + return WriteResult::Failed; } // Writing to UICR can only change `1` bits to `0` bits. @@ -257,7 +273,7 @@ unsafe fn uicr_write(address: *mut u32, value: u32) -> bool { nvmc.config.reset(); while nvmc.ready.read().ready().is_busy() {} - true + WriteResult::Written } /// Initialize peripherals with the provided configuration. This should only be called once at startup. @@ -283,7 +299,8 @@ pub fn init(config: config::Config) -> Peripherals { // which needs explicit action by the firmware to keep it unlocked // UICR.APPROTECT = SwDisabled - needs_reset |= uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_DISABLED); + let res = uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_DISABLED); + needs_reset |= res == WriteResult::Written; // APPROTECT.DISABLE = SwDisabled (0x4000_0558 as *mut u32).write_volatile(consts::APPROTECT_DISABLED); } else { @@ -295,12 +312,14 @@ pub fn init(config: config::Config) -> Peripherals { unsafe { let p = &*pac::CTRLAP::ptr(); - needs_reset |= uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_DISABLED); + let res = uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_DISABLED); + needs_reset |= res == WriteResult::Written; p.approtect.disable.write(|w| w.bits(consts::APPROTECT_DISABLED)); #[cfg(feature = "_nrf5340-app")] { - needs_reset |= uicr_write(consts::UICR_SECUREAPPROTECT, consts::APPROTECT_DISABLED); + let res = uicr_write(consts::UICR_SECUREAPPROTECT, consts::APPROTECT_DISABLED); + needs_reset |= res == WriteResult::Written; p.secureapprotect.disable.write(|w| w.bits(consts::APPROTECT_DISABLED)); } } @@ -309,24 +328,57 @@ pub fn init(config: config::Config) -> Peripherals { } config::Debug::Disallowed => unsafe { // UICR.APPROTECT = Enabled - needs_reset |= uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_ENABLED); + let res = uicr_write(consts::UICR_APPROTECT, consts::APPROTECT_ENABLED); + needs_reset |= res == WriteResult::Written; #[cfg(any(feature = "_nrf5340-app", feature = "_nrf9160"))] { - needs_reset |= uicr_write(consts::UICR_SECUREAPPROTECT, consts::APPROTECT_ENABLED); + let res = uicr_write(consts::UICR_SECUREAPPROTECT, consts::APPROTECT_ENABLED); + needs_reset |= res == WriteResult::Written; } }, config::Debug::NotConfigured => {} } - #[cfg(all(feature = "_nrf52", not(feature = "reset-pin-as-gpio")))] + #[cfg(feature = "_nrf52")] unsafe { - needs_reset |= uicr_write(consts::UICR_PSELRESET1, chip::RESET_PIN); - needs_reset |= uicr_write(consts::UICR_PSELRESET2, chip::RESET_PIN); + let value = if cfg!(feature = "reset-pin-as-gpio") { + !0 + } else { + chip::RESET_PIN + }; + let res1 = uicr_write(consts::UICR_PSELRESET1, value); + let res2 = uicr_write(consts::UICR_PSELRESET2, value); + needs_reset |= res1 == WriteResult::Written || res2 == WriteResult::Written; + if res1 == WriteResult::Failed || res2 == WriteResult::Failed { + #[cfg(not(feature = "reset-pin-as-gpio"))] + warn!( + "You have requested enabling chip reset functionality on the reset pin, by not enabling the Cargo feature `reset-pin-as-gpio`.\n\ + However, UICR is already programmed to some other setting, and can't be changed without erasing it.\n\ + To fix this, erase UICR manually, for example using `probe-rs-cli erase` or `nrfjprog --eraseuicr`." + ); + #[cfg(feature = "reset-pin-as-gpio")] + warn!( + "You have requested using the reset pin as GPIO, by enabling the Cargo feature `reset-pin-as-gpio`.\n\ + However, UICR is already programmed to some other setting, and can't be changed without erasing it.\n\ + To fix this, erase UICR manually, for example using `probe-rs-cli erase` or `nrfjprog --eraseuicr`." + ); + } } - #[cfg(all(any(feature = "_nrf52", feature = "_nrf5340-app"), feature = "nfc-pins-as-gpio"))] + #[cfg(any(feature = "_nrf52", feature = "_nrf5340-app"))] unsafe { - needs_reset |= uicr_write(consts::UICR_NFCPINS, 0); + let value = if cfg!(feature = "nfc-pins-as-gpio") { 0 } else { !0 }; + let res = uicr_write(consts::UICR_NFCPINS, value); + needs_reset |= res == WriteResult::Written; + if res == WriteResult::Failed { + // with nfc-pins-as-gpio, this can never fail because we're writing all zero bits. + #[cfg(not(feature = "nfc-pins-as-gpio"))] + warn!( + "You have requested to use P0.09 and P0.10 pins for NFC, by not enabling the Cargo feature `nfc-pins-as-gpio`.\n\ + However, UICR is already programmed to some other setting, and can't be changed without erasing it.\n\ + To fix this, erase UICR manually, for example using `probe-rs-cli erase` or `nrfjprog --eraseuicr`." + ); + } } if needs_reset {