Missing required delay after enabling peripheral clock

[adamgreig]

Currently the following example code:

let gpioa = device.GPIOA.split();
gpioa.pa11.into_alternate_af10();

when built in release mode, will unintentionally corrupt the AF setting for the PA8..15, including crucially the SWD/SWCLK pins PA13/PA14, both on 0.8.3 release and latest master (which swaps to using bitbanding).

The access to the peripheral registers occurs immediately after the clock is enabled, and at least on some F4 devices (e.g. F401, F410), there is an errata 2.1.13 that a delay is required between these events.

Without the delay, the generated code reads from AFRH immediately after writing to AHB1ENR, which reads uninitialised/garbage data, and then writes that garbage back to the AF settings of all pins except 11, which disables debug access, making it harder to debug and recover the problem.

I think the best solution (as per the errata) is to insert a call to cortex_m::asm::dsb() after every clock enable. This applies to all peripheral clocks, but is especially tricky to debug when setting GPIOA AFs, since it breaks debugging.

I suspect this might apply to other F4s as well, though I haven't checked their errata specifically.

Shout out to @kalkyl who reported this bug.

[therealprof]

@adamgreig Can you confirm the issue is sufficiently addressed by #279?

[adamgreig]

I think the same DSB must be added to all peripheral drivers that enable clocks in RCC: SDIO, SYSCFG, I2C, SPI, CAN, ADC, RTC, DAC, DMA.

$ rg "bb::set\(\&rcc\.a.b.?enr"
src/sdio.rs
178:            bb::set(&rcc.apb2enr, 11);

src/syscfg.rs
18:            bb::set(&rcc.apb2enr, 14);

src/i2c.rs
690:            bb::set(&rcc.apb1enr, EN_BIT);

src/spi.rs
752:            bb::set(&rcc.apb2enr, EN_BIT);
789:            bb::set(&rcc.apb1enr, EN_BIT);
825:            bb::set(&rcc.apb1enr, EN_BIT);
857:            bb::set(&rcc.apb2enr, EN_BIT);
888:            bb::set(&rcc.apb2enr, EN_BIT);
914:            bb::set(&rcc.apb2enr, EN_BIT);

src/can.rs
141:                        crate::bb::set(&rcc.apb1enr, $peren);

src/gpio.rs
275:                        bb::set(&rcc.ahb1enr, $rcc_bit);

src/adc.rs
707:                        bb::set(&rcc.apb2enr, $en_bit);

src/rtc.rs
464:        bb::set(&rcc.apb1enr, 28);

src/dac.rs
57:        bb::set(&rcc.apb1enr, EN_BIT);

src/dma/traits.rs
264:            bb::set(&rcc.ahb1enr, 21);
277:            bb::set(&rcc.ahb1enr, 22);

[therealprof]

Yeah, maybe. Better safe than sorry. Will update.