nrf: expose regs on all peripherals

[0e4ef622]

This makes it easier to implement generic drivers externally. Mainly interested in timer but I did it for all of them.

Related: #2948

[i509VCB]

I do think exposing regs on types is a good idea. Although I'm not sure if we want to make this require unstable-pac (not just for nRF, but all HALs).

An application may pin some version of nrf-pac/whatever-pac and just needs to know the address for some peripheral singleton. That could be exposed regardless of the unstable-pac feature.

This makes me lean more towards just giving the user a unit pointer *mut (). And let the application explicitly cast as needed to the pinned version or the reexport of the pac if unstable-pac is enabled.

[0e4ef622]

Hmm, that's an interesting point. Are there any downsides other than ergonomics? Most I can think of is if someone writes a driver against one chip and someone else tries to use it with another chip that has registers at different offsets, that wouldn't become a compiler error. That doesn't seem to be an issue with nRF at least, not sure about other families.

[lulf]

I think any API from the underlying should be guarded behind the unstable-pac signalling that we reserve the right to break the API at any time.

Even so, I think leaking these internals defeats the purpose of having a HAL in the first place. Ideally most functionality should be covered within the HAL, and for the few cases it isn't, you can use the pac directly. Exposing the regs could easily promote some bad patterns. It makes it easier to footgun yourself where the HAL assumes ownership of the peripheral but some other part of your code could be manipulating the registers as well. It's always possible to do that, but the point is lets not make it too easy?

[0e4ef622]

I can understand that. The main thing this makes possible (or at least not inconvenient) is accessing the regs from a generic interrupt handler that is usable with the bind_interrupts! macro.

pub struct InterruptHandler<T: Instance> {
    _phantom: PhantomData<T>,
}

impl<T: Instance> interrupt::typelevel::Handler<T::Interrupt> for InterruptHandler<T> {
    unsafe fn on_interrupt() {
        // how to get regs?
    }
}

[i509VCB]

Exposing the regs could easily promote some bad patterns. It makes it easier to footgun yourself where the HAL assumes ownership of the peripheral but some other part of your code could be manipulating the registers as well. It's always possible to do that, but the point is lets not make it too easy?

I would agree that using the pointers to a peripheral correctly is the user's responsibility while considering ownership. If the HAL owns a peripheral and you need to do something PAC related that the HAL doesn't do (although a pull request would make sense here) or shouldn't expose (XBAR triggering is practically impossible for embassy-nxp to expose without unsafe, or purpose built drivers).

The return type as a *mut () makes sense because in user code it makes it blindingly clear that you are trying to access PAC related functionality because converting to the PAC types would be unsafe. While it's made easier, the unsafe blocks you will need to write as a user will shame you into asking "am I going to cause awful bugs".