desired features

[xoviat]

This is a todo list of what features I'd like to see in this to consider using it; not that my opinion carries much weight, but it's a good place to start.

• [ ] QEI encoder read/reset/enable interrupt
• [x] DMA Serial Read/Write with nb::WouldBlock API; interrupt and one-at-a-time API is simplest to implement but not useful
• [x] DMA I2C Read/Write with nb::WouldBlock
• [x] DMA SPI Read/Write with nb::WouldBlock
• [x] DMA ADC Read/Write with nb::WouldBlock
• [x] bxcan Implementaiton
• [x] ExtiPin
• [x] GPIO
• [x] DAC
• [x] USB

There are nice to have, but not critical:

• [x] RTC
• [x] Crc32
• [ ] DMA2D

IMO, DMA implementations should be the default and should be implemented first. They are the most difficult to implement, but also the most useful.

[David-OConnor]

DAC and RTC are done. (For f3 and L4)

I plan to start with these, since they should be quick to knock out, and I can test them on current projects:

• GPIO, including EXTI. I plan to take a significant break from the existing approaches, by moving away from type state programming. It makes passing things across function boundaries, or declaring them statically tough. Thoughts? The downside is it won't catch misconfigured or wrong pins at compile time.

I think expecting the user to setup the pins correctly (eg check the user manual, and/or copy+paste examples from the examples folder) is worth avoiding this:

static SENSOR: Mutex<
    RefCell<
        Option<
            Mlx9061x<
                I2c<
                    I2C1,
                    (
                        PB6<Alternate<AF4, Output<OpenDrain>>>,
                        PB7<Alternate<AF4, Output<OpenDrain>>>,
                    ),
                >,
                ic::Mlx90614,
            >,
        >,
    >,
> = Mutex::new(RefCell::new(None));

In addition to this being verbose, it's not obvious where you find how to construct this; I've had to do it by trial+error with compile errors guiding the way.

The peripherals like I2C, SPI, DAC etc don't make use of the pins you pass as arguments directly; these args are just to check that you picked a valid pin, and set it to the right mode.

• SPI (Probably based on H7xx, but open to suggestions)
• low power (Based on L4/F3 forks I've been using)

Could you help with the DMA? I haven't used it before, so don't have a grasp on where to start.

Would also like to start implementing and eventually hardware testing the current modules with H7 and L5.

[xoviat]

GPIO, including EXTI. I plan to take a significant break from the existing approaches, by moving away from type state programming. It makes passing things across function boundaries, or declaring them statically tough. Thoughts? The downside is it won't catch misconfigured or wrong pins at compile time.

I can take a look at what your proposal is, but I am personally in favor of type-state programming.

[David-OConnor]

It's a big departure from the norm, and may not be worth it. We can change later if it seems not worth it. Here are some thoughts, as they apply to GPIO and buses like I2C.

Big:

• With TST, using the wrong pin, or setting it up wrong will give you a compile error. Without you'll get unexpected runtime behavior (like the peripheral not working)
• TST leads to unwieldy syntax when types are explicitly declared (see example above). This includes global constants, functions, and struct fields. Applies upstream, ie to devices that use a bus. We can mitigate this through documenting every peripheral (eg I2C, SPI) with example function signatures. Rust docs doesn't do a great job auto-generating these, so should document there as well. Still would be verbose.

Small:

• It feels strange to pass arguments (ie pin structs) that aren't used directly.
• TST will complicate the multi-family aspect of this crate, requiring more feature gates on the bus and GPIO modules. That said, that comes with the territory, and wouldn't be too bad, since these are similar across families. Ie we can expect PB6 and PB7 to be opendrain AF4 I2C pins on most or all devices.

Untested idea: What if we don't use TST, but initialization functions for buses check that at least one set of pins is configured correctly using register reads?

Or... You pass an eum to bus init function. Eg I2cPins::Pb6Pb7. The init fn handles pin setup. Leaning towards this.

[xoviat]

So from the chat, the best approach might be to:

• Use new<T>() -> Self
• Add new_unchecked() -> Self to all classes with new() that require pins
• Remove free() from all classes

[David-OConnor]

Agree. Going to start with new_unchecked(), since we can do this before making the GPIO module, and it's easier to implement cross-family.

[xoviat]

With respect to DMA:

1. The peripheral is set up as normal, but in addition, the peripheral DMA configuration is applied. This is what that looks like (in new_unchecked:

            DmaConfig::TxRx => unsafe {
                (*USART::ptr())
                    .cr3
                    .write(|w| w.dmar().enabled().dmat().enabled())
            },

2. In the write or read function, the DMA transfer is configured:

        assert!(buffer.len() <= u16::max_value() as usize);

        // The following configuration procedure is documented in the reference
        // manual for STM32F75xxx and STM32F74xxx, section 8.3.18.

        let nr = T::Stream::number();

        // Disable stream
        handle.dma.st[nr].cr.modify(|_, w| w.en().disabled());
        while handle.dma.st[nr].cr.read().en().is_enabled() {}

        T::Stream::clear_status_flags(&handle.dma);

        // Set peripheral port register address
        handle.dma.st[nr].par.write(|w| w.pa().bits(address));

        // Set memory address
        let memory_address = buffer.as_ptr() as u32;
        handle.dma.st[nr]
            .m0ar
            .write(|w| w.m0a().bits(memory_address));

        // Write number of data items to transfer
        //
        // We've asserted that `data.len()` fits into a `u16`, so the cast
        // should be fine.
        handle.dma.st[nr]
            .ndtr
            .write(|w| w.ndt().bits(buffer.len() as u16));

        // Configure FIFO
        handle.dma.st[nr].fcr.modify(|_, w| {
            w
                // Interrupt disabled
                .feie()
                .disabled()
                // Direct mode enabled (FIFO disabled)
                .dmdis()
                .enabled()
        });

        // Select channel
        handle.dma.st[nr].cr.write(|w| {
            let w = T::Channel::select(w);

            let w = match direction {
                Direction::MemoryToPeripheral => w.dir().memory_to_peripheral(),
                Direction::PeripheralToMemory => w.dir().peripheral_to_memory(),
            };

            w
                // Single transfer
                .mburst()
                .single()
                .pburst()
                .single()
                // Double-buffer mode disabled
                .dbm()
                .disabled()
                // Very high priority
                .pl()
                .very_high()
                // Memory data size
                .msize()
                .variant(Word::msize())
                // Peripheral data size
                .psize()
                .variant(Word::psize())
                // Memory increment mode
                .minc()
                .incremented()
                // Peripheral increment mode
                .pinc()
                .fixed()
                // Circular mode disabled
                .circ()
                .disabled()
                // DMA is the flow controller
                .pfctrl()
                .dma()
                // All interrupts disabled
                .tcie()
                .disabled()
                .htie()
                .disabled()
                .teie()
                .disabled()
                .dmeie()
                .disabled()
        });

3. Immediately after, the DMA transfer is initiated:

        atomic::fence(Ordering::SeqCst);

        handle.dma.st[T::Stream::number()]
            .cr
            .modify(|_, w| w.en().enabled());

This is the same across all stm32 devices, though there will be slight differences between them. The new devices have a more complex DMA which may need additional configuration. Also, note that although there is quite a bit of set up, it is nothing compared to the CPU time saved in communications compared to other methods.

[David-OConnor]

new_unchecked add for dac, i2c, and SPI.

[David-OConnor]

Would you be willing to PR the DMA stuff once I have SPI, I2C, SERIAL, and ADC working on a few families?

[xoviat]

Yes.

[Dirbaio]

DMA xxx with nb::WouldBlock

It is completely impossible to implement nb APIs with DMA.

Consider an API like this:

fn read(&mut self, bytes: &mut [u8]) -> nb::Result<(), Self::Error> { ..  }

On first call, this would start the DMA read into bytes, and then return WouldBlock. The problem is after return, the bytes buffer is no longer borrowed. The user could read from it which would race with DMA writes. Or even deallocate it, which would cause DMA to overwrite arbitrary memory.

[xoviat]

Not true. See how it's done in the f7 HAL.

[Dirbaio]

The f7 hal doesn't implement the embedded-hal nonblocking traits with DMA, does it?

[David-OConnor]

Hey - DMA is over my head for now, so I don't imagine solving this issue in the near future. If someone knowledgeable with DMA want to give it a go, please do! (I'll get to QEI and CAN, also mentioned in this issue, sooner)

[David-OConnor]

Going to tackle DMA and nonblocking, interrupt-based APIs, but don't plan to base it off of nb. I'm going to check the above off as I add DMA for those peripherals. nb will be used for the (non-DMA) embedded-hal implementations only.

[David-OConnor]

Could you provide more info on QEI? Can't find it in the RMs.

[David-OConnor]

Re-open if you have info on QEI, or would like to tackle DMA2D.

[kellda]

Re QEI: It seems that on STM32s, QEIs are just a special mode for timers. See e.g. “29.4.18 Encoder interface mode” in https://www.st.com/resource/en/reference_manual/rm0440-stm32g4-series-advanced-armbased-32bit-mcus-stmicroelectronics.pdf.