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

Idiomatic way of enabling the cpu-net core on nRF5340 #3325

Open
igiona opened this issue Sep 10, 2024 · 5 comments
Open

Idiomatic way of enabling the cpu-net core on nRF5340 #3325

igiona opened this issue Sep 10, 2024 · 5 comments

Comments

@igiona
Copy link
Contributor

igiona commented Sep 10, 2024

In order to start the cpu-net core on the nRF5340, the cpu-app core must clear the bit FORCEOFF of the register APPLICATION.RESET.NETWORK.

See
image
https://infocenter.nordicsemi.com/pdf/nRF5340_PS_v1.3.1.pdf

Currently I didn't find a way to cleanly access the RESET peripheral.
I found the generated PAC types, but not how to access them as they are not (seems not to be?) exposed in HAL Peripheral object.

Am I missing something?

Or the implementation of this register is simply missing in the HAL?
If yes, I would be happy to help. Is there some resource on how registers should be exposed in the HAL?

I was able to access the register as follow:

unsafe {
    ((0x5000_5000 + 0x614) as *mut u32).write_volatile(0);
}

...but this is hardly the idiomatic way of doing things 😝

@Dirbaio
Copy link
Member

Dirbaio commented Sep 10, 2024

use unsafe to get access to the registerblock.

unsafe { &*embassy_nrf::pac::RESET::PTR }.network.forceoff.write(...);

I found the generated PAC types, but not how to access them as they are not (seems not to be?) exposed in HAL Peripheral object.

the reason is embassy HALs don't use the PAC owned singletons, they create their own. See here for the reasoning.

RP2x and STM32 are using PACs generated by chiptool so you don't need unsafe, but embassy-nrf is still using the svd2rust-generated PACs from nrf-rs.

@igiona
Copy link
Contributor Author

igiona commented Sep 10, 2024

use unsafe to get access to the registerblock.

unsafe { &*embassy_nrf::pac::RESET::PTR }.network.forceoff.write(...);

I found the generated PAC types, but not how to access them as they are not (seems not to be?) exposed in HAL Peripheral object.

the reason is embassy HALs don't use the PAC owned singletons, they create their own. See here for the reasoning.

RP2x and STM32 are using PACs generated by chiptool so you don't need unsafe, but embassy-nrf is still using the svd2rust-generated PACs from nrf-rs.

Thank you for the explanation and input, definitely a cleaner way to access the register 👍

I'm still a bit confused though, why then e.g. SERIAL0 or NVMC are available in Peripherals ?
Is it because RESET is not consider to be peripheral somehow, or because embassy-hal doesn't have an "abstraction" for such kind of peripheral?
I couldn't find where a peripheral kind of implements an embassy-HAL trait, I got lost in macros :)

Thank you @Dirbaio 🙏 for your time and I hope this will be helpful to others as well :)

@Dirbaio
Copy link
Member

Dirbaio commented Sep 10, 2024

I'm still a bit confused though, why then e.g. SERIAL0 or NVMC are available in Peripherals ?

they are, but they're defined by the HAL, they're not the PAC ones.

the reason RESET is not there is because there's no HAL driver for it, yes. But even if we add one it might make more sense to not make it owned. RESET is just a bunch of registers, the "driver" could be just a few functions to set them with no ownership required. In that case there would be no RESET in Peripherals.

The design of embassy HALs is to not use PAC owned singletons at all, for these reasons and for usability reasons. For example, the PAC has a single singleton for the whole PPI, and two singletons for GPIO port 0 and port 1. That doesn't match how you as a user want to manage ownership, you want to own individual pins or PPI channels. Using the PAC singletons forces the HAL to require useless boilerplate like "take the GPIO P0 peripheral, split it in pins, and then you can own individual pins". If the HAL defines its own singletons, everything can come pre-split nicely in Peripherals, no boilerplate.

If you look at the rp or stm32 PACs they don't have singletons at all, you can just go and do a register write, no unsafe needed. For nrf for historical reasons we're still using a PAC with singletons, just that embassy-nrf pretends they don't exist and bypasses them with unsafe. This has the unfortunate consequence that the user must also use unsafe to access registers. This will go away when we switch to a nRF PAC generated by chiptool. I've sort of started work on it here, but it needs porting the entire HAL to the new PAC which is annoying.

@igiona
Copy link
Contributor Author

igiona commented Sep 11, 2024

Can you point out to the bit that allows you to state

RESET is not there is because there's no HAL driver for it

Where can I see that for SERIAL0 there is a HAL driver available? I imagine that there is somewhere a HAL trait which is implemented, but I couldn't track it down yet...

Yes I get that, the embassy-HAL decision on not using owned singletons makes sense to me.

Maybe I could build on the work you started to port the nRF PAC from svd2rust to chiptool, it could be a good way to get deeper into some implementation details.
I would prob. need some guide to be able to effectively start with it though.

@Dirbaio
Copy link
Member

Dirbaio commented Sep 11, 2024

"SERIAL" is a peripheral that can do UART, SPI or TWI (i2c), master or slave, so it's usable with all 5 drivers: https://github.com/embassy-rs/embassy/blob/main/embassy-nrf/src/chips/nrf5340_app.rs#L395-L418 . It's a bit confusing because Nordic rarely refers to it as "SERIAL" in their docs, they call it "UART" or "SPIM" or whatever but if you look at the peripheral address it's the same, ie they're all actually a single peripheral that has different "modes".

Maybe I could build on the work you started to port the nRF PAC from svd2rust to chiptool, it could be a good way to get deeper into some implementation details.

There's mainly two things to do:

  1. Make nrf-pac support all nRF chips. The PAC in the nrf-pac repo currently supports only nrf52840. The best way to do it is probably same as we did in rp-pac to support rp2040 and rp235x: generate the PACs into a subfolder and manually write a lib.rs that exports the enabled one.
  2. Port embassy-nrf to the new PAC. This is mostly mechanical changes, the syntax for chiptool register access is slightly different than svd2rust's.

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

No branches or pull requests

2 participants