alloc and interrupts and general BSP questions

[x37v]

It looks like you need the alloc/nightly feature because you manage interrupts in the library code and you need a concrete struct for your statics.

I wonder if it would make sense to do that handling behind a feature flag and allow for a non alloc version with generics and exposed methods that do all the interrupt handling work but don't actually implement the interrupt callback?

I guess I also have a larger question about what a board support crate should include and not. For instance, if you initialized the board by only passing in the peripherals you needed, instead of basically owning all the peripherals, you wouldn't need to do unsafe steal like in https://github.com/antoinevg/daisy_bsp/issues/1 But I'm not 100% on what the "philosophy" of a board support crate is in general, or this one in particular.

[antoinevg]

I really like the idea of putting the alloc-dependent code behind a feature flag, thanks! +1

re: owning all the things - I actually reached the same conclusion a couple weeks back and did a big refactor recently: 301d2a3a0f2499a644a7a2974ab6929ed000eedb

If you check out the updated adc example you can see the difference:

https://github.com/antoinevg/daisy_bsp/blob/main/examples/adc_bsp.rs

[x37v]

Ahh okay! you've already done the refactor. Cool!

[x37v]

@antoinevg I'm wondering what the reasoning behind the use of steal for some peripherals and not others is?

For instance, clocks::configure you steal so you can get some things for to enable_itm

Couldn't you simply have an optional argument to configure that contains what enable_itm needs and remove the log-itm feature flag and the steals?

But, maybe there you have a strategy for this?

[antoinevg]

The enable_itm code is really just a temporary workaround to deal with the fact that older versions of openocd have an issue with stm32h7* microprocessors: stm32-rs/stm32h7xx-hal#7

I'm imagining it will probably fall away in a year or two so it's probably better not to bake the peripheral requirements into the API here.

The logging feature flags are needed because they're mutually exclusive - i.e. you need to choose one and only one of rtt/itm/semihosting because they provide the global panic handler etc.

[x37v]

maybe that was a bad example then. for instance, in audio.rs

    pub fn init(
        clocks: &hal::rcc::CoreClocks,
        rec: hal::rcc::rec::Sai1, // reset and enable control
        pins: Sai1Pins
    ) -> Result<Interface<'a>, Error> {

        // - Peripherals ------------------------------------------------------

        let rcc_pac = unsafe { pac::Peripherals::steal().RCC };
        let dma1_pac = unsafe { pac::Peripherals::steal().DMA1 };
        let dmamux1_pac = unsafe { pac::Peripherals::steal().DMAMUX1 };
        let sai1_pac = unsafe { pac::Peripherals::steal().SAI1 };

Why do you pass in hal::rcc::rec::Sai1 but 'unsafe steal' a bunch from hal::pac ?

I'm just curious in case I want to try to contribute, if there is a method to this.

Generally, I try to avoid unsafe as much as possible, I assume that the hal's peripherals API is set up to try to assure that, if used without unsafe, users don't share resources that shouldn't be shared? Maybe its too restrictive in these cases?

[antoinevg]

Hehe, audio.rs is another special case!

Background here is that I wrote this first version of the audio driver before rust-embedded/embedded-dma support was added to the stm32-rs/stm32h7xx-hal crate.

It's a direct register-level implementation using the stm32-rs/stm32h7 Peripheral Access Crate and I wanted to:

1. keep to a relatively simple API and
2. still allow for access to the DMA1 & DMAMUX1 peripheral register blocks in other parts of the code.

In general it's probably forgivable to steal register blocks in this way if it's during startup code that will only be run once.

So a nice counterpoint to this is the newer version of the audio driver that uses the embedded-dma traits: audio_hal.rs

All the peripheral theft you see happening here is due to the limitations of the embedded-hal traits themselves which often have conflicting ownership requirements.

e.g.

Here the input and output DMA streams each need their own access to the SAI1 register block:

        // dma1 stream 0
        ...
        let dma1_str0: dma::Transfer<_, _, dma::MemoryToPeripheral, _, _> = dma::Transfer::init(
            dma1_streams.0,
==>         unsafe { pac::Peripherals::steal().SAI1 },  <==
            tx_buffer,
            None,
            dma_config,
        );

        // dma1 stream 1
        ...
        let dma1_str1: dma::Transfer<_, _, dma::PeripheralToMemory, _, _> = dma::Transfer::init(
            dma1_streams.1,
==>         unsafe { pac::Peripherals::steal().SAI1 },   <==
            rx_buffer,
            None,
            dma_config,
        );

More info: stm32-rs/stm32h7xx-hal#153

And again, this would be a very different situation if any of this code were active in the main loop and/or exposed in concurrent execution contexts!

Tx muchly for the questions, it's great to be able to talk it all through with someone else!

[x37v]

@antoinevg great! thanks for the info!