I've recently watched a few talks on implementing easy-to-use hardware abstraction layers in C++ for use on embedded devices such as ARM Cortex-based microcontrollers such as the STM32F4 that I am targeting with this project. I would like some input into whether or not the following usage structure makes sense to the average programmer and the embedded programmer due to the amount of hardware that must be normally set up manually when interacting with memory-mapped hardware peripherals in the context of an embedded application. My full project is too big but I will try to boil down the parts I want feedback on as follows:
startup.cpp: inital processor boot entry file
#include <cstddef>
#include <cstring>
/*
* External definitions from linker for stack, BSS, data, and initialization/constructor
* functions
*/
extern "C" int __stack_top__;
extern "C" char __bss_start__;
extern "C" char __bss_end__;
extern "C" char __data_start__;
extern "C" char __data_end__;
extern "C" char __data_flash__;
extern "C" void (*__preinit_array_start[])(void);
extern "C" void (*__preinit_array_end[])(void);
extern "C" void (*__init_array_start[])(void);
extern "C" void (*__init_array_end[])(void);
// Prototype definition of main function
int boot_main();
namespace Startup {
/**
* @brief Default interrupt handler for interrupt vector table
*
*/
void Default_Handler();
/**
* @brief Goes through all startup initialization, including:
* -- Copies initialization values from flash to RAM
* -- Zeros BSS area
* -- Calls all initializers and constructors
*
*/
void Initialize();
} // namespace Startup
/**
* @brief Reset handler that exectures the initialization functions then calls main
*
*/
extern "C" void Reset_Handler() {
Startup::Initialize();
boot_main();
Startup::Default_Handler();
}
void Startup::Initialize() {
size_t count;
// Copy static values from flash .data to RAM .data
count = &__data_end__ - &__data_start__;
std::memcpy(&__data_start__, &__data_flash__, count);
// Set BSS area to zero
count = &__bss_end__ - &__bss_start__;
std::memset(&__bss_start__, 0, count);
// Call all initializers & constructors
size_t i;
count = __preinit_array_end - __preinit_array_start;
for (i = 0; i < count; i++)
__preinit_array_start[i]();
count = __init_array_end - __init_array_start;
for (i = 0; i < count; i++)
__init_array_start[i]();
}
void Startup::Default_Handler() {
while (true) {
// Do nothing
}
}
// NVIC interrupt table
extern "C" void (*const vectors[])(void)
__attribute__((used, section(".vectors"))) = {(void (*)(void))(&__stack_top__),
Reset_Handler,
Startup::Default_Handler,
Startup::Default_Handler,
Startup::Default_Handler,
Startup::Default_Handler,
Startup::Default_Handler,
nullptr,
nullptr,
nullptr,
nullptr,
Startup::Default_Handler,
Startup::Default_Handler,
nullptr,
Startup::Default_Handler,
... { repeat several times } ...
Startup::Default_Handler};
main.cpp: main entry file
#include "hardware/GPIO.hpp"
#include "hardware/RCC.hpp"
#include <cstdint>
#include <cstring>
int boot_main() {
RCC &rcc = *new RCC();
rcc.EnableExternalSystemClock();
for (std::size_t i = 0; i < 2000000; i++) {
// Do nothing (busy wait)
}
GPIO &gpioc = *new (GPIO::GPIOC) GPIO();
gpioc.SetMode(GPIO::PIN13, GPIO::Mode::OUTPUT);
gpioc.SetType(GPIO::PIN13, GPIO::Type::PUSH_PULL);
gpioc.SetResistor(GPIO::PIN13, GPIO::PUPD::PULL_UP);
while (true) {
gpioc.EnableOutput(GPIO::PIN13);
for (std::size_t i = 0; i < 45000; i++) {
// Do nothing (busy wait)
}
gpioc.DisableOutput(GPIO::PIN13);
for (std::size_t i = 0; i < 45000; i++) {
// Do nothing (busy wait)
}
}
}
GPIO.cpp: GPIO class implementation
#include "GPIO.hpp"
#include "RCC.hpp"
GPIO::GPIO() {
// Do nothing
}
void *GPIO::operator new(std::size_t) {
// Enable GPIOA clock
RCC &rcc = *new RCC();
rcc.EnableGPIO(GPIO::GPIOA);
// Return GPIOA address
return reinterpret_cast<void *>(0x40020000);
}
void *GPIO::operator new(std::size_t, const enum gpio_selection n) {
RCC &rcc = *new RCC();
rcc.EnableGPIO(n);
// Return GPIO address
auto address = 0x40020000 + n * 0x400;
return reinterpret_cast<void *>(address);
}
void GPIO::SetMode(enum gpio_pin pin_num, GPIO::Mode mode) {
uint32_t set_moder = this->MODER.reg;
set_moder &= ~(0b11 << (2 * pin_num));
switch (mode) {
case GPIO::Mode::OUTPUT:
set_moder |= (0b01 << (2 * pin_num));
break;
case GPIO::Mode::ALTERNATE:
set_moder |= (0b10 << (2 * pin_num));
break;
case GPIO::Mode::ANALOG:
set_moder |= (0b11 << (2 * pin_num));
break;
default:
// Do nothing
break;
}
this->MODER.reg = set_moder;
}
... { several other method implementations } ...
GPIO.hpp: GPIO class header
#include <cstddef>
#include <cstdint>
#include <type_traits>
#pragma once
using reg32_t = volatile uint32_t;
using reg16_t = volatile uint16_t;
class GPIO {
private:
union {
reg32_t reg;
struct {
reg32_t MODER15 : 2;
reg32_t MODER14 : 2;
reg32_t MODER13 : 2;
reg32_t MODER12 : 2;
reg32_t MODER11 : 2;
reg32_t MODER10 : 2;
reg32_t MODER9 : 2;
reg32_t MODER8 : 2;
reg32_t MODER7 : 2;
reg32_t MODER6 : 2;
reg32_t MODER5 : 2;
reg32_t MODER4 : 2;
reg32_t MODER3 : 2;
reg32_t MODER2 : 2;
reg32_t MODER1 : 2;
reg32_t MODER0 : 2;
} bits;
} MODER;
union {
reg32_t reg;
struct {
reg32_t : 16;
reg32_t OT15 : 1;
reg32_t OT14 : 1;
reg32_t OT13 : 1;
reg32_t OT12 : 1;
reg32_t OT11 : 1;
reg32_t OT10 : 1;
reg32_t OT9 : 1;
reg32_t OT8 : 1;
reg32_t OT7 : 1;
reg32_t OT6 : 1;
reg32_t OT5 : 1;
reg32_t OT4 : 1;
reg32_t OT3 : 1;
reg32_t OT2 : 1;
reg32_t OT1 : 1;
reg32_t OT0 : 1;
} bits;
} OTYPER;
union {
reg32_t reg;
struct {
reg32_t OSPEEDR15 : 2;
reg32_t OSPEEDR14 : 2;
reg32_t OSPEEDR13 : 2;
reg32_t OSPEEDR12 : 2;
reg32_t OSPEEDR11 : 2;
reg32_t OSPEEDR10 : 2;
reg32_t OSPEEDR9 : 2;
reg32_t OSPEEDR8 : 2;
reg32_t OSPEEDR7 : 2;
reg32_t OSPEEDR6 : 2;
reg32_t OSPEEDR5 : 2;
reg32_t OSPEEDR4 : 2;
reg32_t OSPEEDR3 : 2;
reg32_t OSPEEDR2 : 2;
reg32_t OSPEEDR1 : 2;
reg32_t OSPEEDR0 : 2;
} bits;
} OSPEEDR;
union {
reg32_t reg;
struct {
reg32_t PUPDR15 : 2;
reg32_t PUPDR14 : 2;
reg32_t PUPDR13 : 2;
reg32_t PUPDR12 : 2;
reg32_t PUPDR11 : 2;
reg32_t PUPDR10 : 2;
reg32_t PUPDR9 : 2;
reg32_t PUPDR8 : 2;
reg32_t PUPDR7 : 2;
reg32_t PUPDR6 : 2;
reg32_t PUPDR5 : 2;
reg32_t PUPDR4 : 2;
reg32_t PUPDR3 : 2;
reg32_t PUPDR2 : 2;
reg32_t PUPDR1 : 2;
reg32_t PUPDR0 : 2;
} bits;
} PUPDR;
union {
reg32_t reg;
struct {
reg32_t : 16;
reg32_t IDR15 : 1;
reg32_t IDR14 : 1;
reg32_t IDR13 : 1;
reg32_t IDR12 : 1;
reg32_t IDR11 : 1;
reg32_t IDR10 : 1;
reg32_t IDR9 : 1;
reg32_t IDR8 : 1;
reg32_t IDR7 : 1;
reg32_t IDR6 : 1;
reg32_t IDR5 : 1;
reg32_t IDR4 : 1;
reg32_t IDR3 : 1;
reg32_t IDR2 : 1;
reg32_t IDR1 : 1;
reg32_t IDR0 : 1;
} bits;
} IDR;
union {
reg32_t reg;
struct {
reg32_t : 16;
reg32_t ODR15 : 1;
reg32_t ODR14 : 1;
reg32_t ODR13 : 1;
reg32_t ODR12 : 1;
reg32_t ODR11 : 1;
reg32_t ODR10 : 1;
reg32_t ODR9 : 1;
reg32_t ODR8 : 1;
reg32_t ODR7 : 1;
reg32_t ODR6 : 1;
reg32_t ODR5 : 1;
reg32_t ODR4 : 1;
reg32_t ODR3 : 1;
reg32_t ODR2 : 1;
reg32_t ODR1 : 1;
reg32_t ODR0 : 1;
} bits;
} ODR;
union {
reg32_t reg;
struct {
reg32_t BR15 : 1;
reg32_t BR14 : 1;
reg32_t BR13 : 1;
reg32_t BR12 : 1;
reg32_t BR11 : 1;
reg32_t BR10 : 1;
reg32_t BR9 : 1;
reg32_t BR8 : 1;
reg32_t BR7 : 1;
reg32_t BR6 : 1;
reg32_t BR5 : 1;
reg32_t BR4 : 1;
reg32_t BR3 : 1;
reg32_t BR2 : 1;
reg32_t BR1 : 1;
reg32_t BR0 : 1;
reg32_t BS15 : 1;
reg32_t BS14 : 1;
reg32_t BS13 : 1;
reg32_t BS12 : 1;
reg32_t BS11 : 1;
reg32_t BS10 : 1;
reg32_t BS9 : 1;
reg32_t BS8 : 1;
reg32_t BS7 : 1;
reg32_t BS6 : 1;
reg32_t BS5 : 1;
reg32_t BS4 : 1;
reg32_t BS3 : 1;
reg32_t BS2 : 1;
reg32_t BS1 : 1;
reg32_t BS0 : 1;
} bits;
struct {
reg16_t BR;
reg16_t BS;
} split;
} BSRR;
union {
reg32_t reg;
struct {
reg32_t : 15;
reg32_t LCKK : 1;
reg32_t LCK15 : 1;
reg32_t LCK14 : 1;
reg32_t LCK13 : 1;
reg32_t LCK12 : 1;
reg32_t LCK11 : 1;
reg32_t LCK10 : 1;
reg32_t LCK9 : 1;
reg32_t LCK8 : 1;
reg32_t LCK7 : 1;
reg32_t LCK6 : 1;
reg32_t LCK5 : 1;
reg32_t LCK4 : 1;
reg32_t LCK3 : 1;
reg32_t LCK2 : 1;
reg32_t LCK1 : 1;
reg32_t LCK0 : 1;
} bits;
} LCKR;
union {
reg32_t reg;
struct {
reg32_t AFRL7 : 4;
reg32_t AFRL6 : 4;
reg32_t AFRL5 : 4;
reg32_t AFRL4 : 4;
reg32_t AFRL3 : 4;
reg32_t AFRL2 : 4;
reg32_t AFRL1 : 4;
reg32_t AFRL0 : 4;
} bits;
} AFRL;
union {
reg32_t reg;
struct {
reg32_t AFRH15 : 4;
reg32_t AFRH14 : 4;
reg32_t AFRH13 : 4;
reg32_t AFRH12 : 4;
reg32_t AFRH11 : 4;
reg32_t AFRH10 : 4;
reg32_t AFRH9 : 4;
reg32_t AFRH8 : 4;
} bits;
} AFRH;
public:
enum gpio_selection { GPIOA = 0, GPIOB = 1, GPIOC = 2, GPIOD = 3, GPIOE = 4, GPIOH = 7 };
enum gpio_pin {
PIN0,
PIN1,
PIN2,
PIN3,
PIN4,
PIN5,
PIN6,
PIN7,
PIN8,
PIN9,
PIN10,
PIN11,
PIN12,
PIN13,
PIN14,
PIN15
};
enum class Mode { INPUT, OUTPUT, ALTERNATE, ANALOG };
enum class Type { PUSH_PULL, OPEN_DRAIN };
enum class PUPD { NONE, PULL_UP, PULL_DOWN };
GPIO();
void *operator new(std::size_t);
void *operator new(std::size_t, const enum gpio_selection n);
void SetMode(enum gpio_pin pin_num, GPIO::Mode mode);
GPIO::Mode GetMode(enum gpio_pin pin_num);
void SetType(enum gpio_pin pin_num, GPIO::Type type);
void SetResistor(enum gpio_pin pin_num, enum GPIO::PUPD pupd_type);
void EnableOutput(enum gpio_pin pin_num);
void DisableOutput(enum gpio_pin pin_num);
bool GetOutput(enum gpio_pin pin_num);
enum gpio_selection GetGPIO() {
return static_cast<enum gpio_selection>((reinterpret_cast<int>(this) & 0xFFFF) / 0x400);
}
};
static_assert(sizeof(GPIO) == 10 * sizeof(reg32_t), "GPIO contains padding bytes.");
static_assert(std::is_standard_layout<GPIO>::value, "GPIO isn't standard layout.");
RCC.cpp: RCC class implementation
#include "RCC.hpp"
#include "FLASH.hpp"
#include "GPIO.hpp"
#include "PWR.hpp"
RCC::RCC() {
// Do nothing
}
void *RCC::operator new(std::size_t) {
return reinterpret_cast<void *>(0x40023800);
}
void RCC::EnableExternalSystemClock() {
// Enable HSE
this->CR.bits.HSEON = 1;
while (this->CR.bits.HSERDY == 0) {
// Do nothing
}
// Enable power controller
this->APB1ENR.bits.PWREN = 1;
// Configure voltage regulator scaling
PWR &power = *new PWR();
power.SetRegulatorScale(PWR::Scale::DIV2);
// Enable flash prefetch & caches
FLASH &flash = *new FLASH();
flash.EnablePrefetchCache();
// Set PLL scalers
constexpr auto pll_clocks = RCC::CalculatePLLClocks(HSE_CRYSTAL_FREQ_HZ, TARGET_SYSTEM_CLOCK);
this->PLLCFGR.bits.PLLM = std::get<0>(pll_clocks);
this->PLLCFGR.bits.PLLN = std::get<1>(pll_clocks);
this->PLLCFGR.bits.PLLP = std::get<2>(pll_clocks);
// Calculate actual system clock
constexpr auto sys_clock =
RCC::CalculateSysClock(HSE_CRYSTAL_FREQ_HZ, std::get<0>(pll_clocks),
std::get<1>(pll_clocks), std::get<2>(pll_clocks));
// Set APB scalers
constexpr auto lsapb2_prescaler = RCC::CalculateLSAPB2Prescaler(sys_clock);
this->CFGR.bits.HPRE = 0b000;
this->CFGR.bits.PPRE1 = lsapb2_prescaler;
this->CFGR.bits.PPRE2 = 0b000;
// Set PLL source
this->PLLCFGR.bits.PLLSRC = 1;
// Enable PLL
this->CR.bits.PLLON = 1;
while (this->CR.bits.PLLRDY == 0) {
// Do nothing
}
// Set clock source
this->CFGR.bits.SW = 0b01;
while (this->CFGR.bits.SWS != 0b01) {
// Do nothing
}
}
RCC.hpp: RCC class header
#include "GPIO.hpp"
#include <cmath>
#include <cstddef>
#include <cstdint>
#include <tuple>
#include <type_traits>
#pragma once
#define HSE_CRYSTAL_FREQ_HZ 25000000
#define HSI_CRYSTAL_FREQ_HZ 16000000
#define TARGET_SYSTEM_CLOCK 84000000
using reg32_t = volatile uint32_t;
class RCC {
private:
union {
reg32_t reg;
struct {
reg32_t : 4;
reg32_t PLLI2SRDY : 1;
reg32_t PLLI2SON : 1;
reg32_t PLLRDY : 1;
reg32_t PLLON : 1;
reg32_t : 4;
reg32_t CSSON : 1;
reg32_t HSEBYP : 1;
reg32_t HSERDY : 1;
reg32_t HSEON : 1;
reg32_t HSICAL : 8;
reg32_t HSITRIM : 5;
reg32_t : 1;
reg32_t HSIRDY : 1;
reg32_t HSION : 1;
} bits;
} CR;
union {
reg32_t reg;
struct {
reg32_t : 4;
reg32_t PLLQ : 4;
reg32_t : 1;
reg32_t PLLSRC : 1;
reg32_t : 4;
reg32_t PLLP : 2;
reg32_t : 1;
reg32_t PLLN : 9;
reg32_t PLLM : 6;
} bits;
} PLLCFGR;
union {
reg32_t reg;
struct {
reg32_t MCO2 : 2;
reg32_t MCO2PRE : 3;
reg32_t MCO1PRE : 3;
reg32_t I2SSRC : 1;
reg32_t MCO1 : 2;
reg32_t RTCPRE : 5;
reg32_t PPRE2 : 3;
reg32_t PPRE1 : 3;
reg32_t : 2;
reg32_t HPRE : 4;
reg32_t SWS : 2;
reg32_t SW : 2;
} bits;
} CFGR;
reg32_t CIR;
reg32_t AHB1RSTR;
reg32_t AHB2RSTR;
reg32_t : 32;
reg32_t : 32;
reg32_t APB1RSTR;
reg32_t APB2RSTR;
reg32_t : 32;
reg32_t : 32;
union {
reg32_t reg;
struct {
reg32_t : 9;
reg32_t DMA2EN : 1;
reg32_t DMA1EN : 1;
reg32_t : 8;
reg32_t CRCEN : 1;
reg32_t : 4;
reg32_t GPIOHEN : 1;
reg32_t : 2;
reg32_t GPIOEEN : 1;
reg32_t GPIODEN : 1;
reg32_t GPIOCEN : 1;
reg32_t GPIOBEN : 1;
reg32_t GPIOAEN : 1;
} bits;
} AHB1ENR;
reg32_t AHB2ENR;
reg32_t : 32;
reg32_t : 32;
union {
reg32_t reg;
struct {
reg32_t : 3;
reg32_t PWREN : 1;
reg32_t : 4;
reg32_t I2C3EN : 1;
reg32_t I2C2EN : 1;
reg32_t I2C1EN : 1;
reg32_t : 3;
reg32_t USART2EN : 1;
reg32_t : 1;
reg32_t SPI3EN : 1;
reg32_t SPI2EN : 1;
reg32_t : 2;
reg32_t WWDGEN : 1;
reg32_t : 7;
reg32_t TIM5EN : 1;
reg32_t TIM4EN : 1;
reg32_t TIM3EN : 1;
reg32_t TIM2EN : 1;
} bits;
} APB1ENR;
reg32_t APB2ENR;
reg32_t : 32;
reg32_t : 32;
reg32_t AHB1LPENR;
reg32_t AHB2LPENR;
reg32_t : 32;
reg32_t : 32;
reg32_t APB1LPENR;
reg32_t APB2LPENR;
reg32_t : 32;
reg32_t : 32;
reg32_t BDCR;
reg32_t CSR;
reg32_t : 32;
reg32_t : 32;
reg32_t SSCGR;
reg32_t PLLI2SCFGR;
reg32_t : 32;
reg32_t DCKCFGR;
public:
RCC();
void *operator new(std::size_t);
void EnableExternalSystemClock();
inline void EnableGPIO(const enum GPIO::gpio_selection selection) {
this->AHB1ENR.reg |= (1 << selection);
// switch (selection) {
// case GPIO::GPIOA:
// this->AHB1ENR.bits.GPIOAEN = 1;
// break;
// case GPIO::GPIOB:
// this->AHB1ENR.bits.GPIOBEN = 1;
// break;
// case GPIO::GPIOC:
// this->AHB1ENR.bits.GPIOCEN = 1;
// break;
// case GPIO::GPIOD:
// this->AHB1ENR.bits.GPIODEN = 1;
// break;
// case GPIO::GPIOE:
// this->AHB1ENR.bits.GPIOEEN = 1;
// break;
// case GPIO::GPIOH:
// this->AHB1ENR.bits.GPIOHEN = 1;
// break;
// default:
// break; // Do nothing
// }
}
inline void DisableGPIO(const enum GPIO::gpio_selection selection) {
this->AHB1ENR.reg &= ~(1 << selection);
}
/**
* @brief Calculate system clock frequency through PLL register values
*
* @param hs_crystal_freq HS clock signal frequency in Hz
* @param pll_m M divisor
* @param pll_n N mulitplier
* @param pll_p P divisor in register form
* @return constexpr int System clock frequency
*/
static constexpr int CalculateSysClock(uint32_t hs_crystal_freq, uint32_t pll_m, uint32_t pll_n,
uint32_t pll_p) {
return ((hs_crystal_freq / pll_m) * pll_n) / ((pll_p * 2) + 2);
}
/**
* @brief Returns best match for PLL M, N, and P register values for main MCU PLL
*
* @param hs_crystal_freq Input HS clock signal frequency in Hz
* @param target Target system clock frequency
* @return constexpr std::tuple<uint32_t, uint32_t, uint32_t> M, N, and P values in tuple
*/
static constexpr std::tuple<uint32_t, uint32_t, uint32_t>
CalculatePLLClocks(uint32_t hs_crystal_freq, uint32_t target) {
uint32_t pll_m = 1, optimum_pll_n = 0, optimum_pll_p = 0;
int64_t min_diff = INT64_MAX;
// Calculate PLL M value to satisfy frequency constraints
while (hs_crystal_freq / pll_m > 2100000) {
pll_m++;
}
// Iterate through all values of N and P to find closest match to target
for (uint32_t pll_n = 430; pll_n > 50; pll_n--) {
for (uint32_t pll_p = 2; pll_p <= 8; pll_p += 2) {
// Calculate system clock with crystal, M, N, and P
uint32_t clock =
RCC::CalculateSysClock(hs_crystal_freq, pll_m, pll_n, (pll_p - 2) / 2);
if ((hs_crystal_freq / pll_m) * pll_n < 432000000 && clock < 84000000) {
// Calcualte absolute difference between target and calculated clock
int64_t diff = (static_cast<int64_t>(target) - static_cast<int64_t>(clock));
diff *= ((diff < 0) ? -1 : 1);
// If it's closer than previously saved result, overwrite it
if (diff <= min_diff) {
min_diff = diff;
optimum_pll_n = pll_n;
optimum_pll_p = pll_p;
}
}
}
}
// Return closest answer in register form
return {pll_m, optimum_pll_n, (optimum_pll_p - 2) / 2};
}
/**
* @brief Calculate system clock frequency through target clock frequency
*
* @param hs_crystal_freq HS clock signal frequency in Hz
* @return constexpr int System clock frequency
*/
static constexpr int GetSystemClock(uint32_t hs_crystal_freq) {
auto pll_clocks = RCC::CalculatePLLClocks(hs_crystal_freq, TARGET_SYSTEM_CLOCK);
return RCC::CalculateSysClock(hs_crystal_freq, std::get<0>(pll_clocks),
std::get<1>(pll_clocks), std::get<2>(pll_clocks));
}
/**
* @brief Calcualte APB2 clock prescaler
*
* @param sys_clock Current system clock
* @return constexpr int APB2 prescaler value
*/
static constexpr int CalculateLSAPB2Prescaler(uint32_t sys_clock) {
int divisor = 1;
while (sys_clock / divisor > 42000000) {
divisor *= 2;
}
switch (divisor) {
case 1:
return 0b000;
case 2:
return 0b100;
case 4:
return 0b101;
case 8:
return 0b110;
case 16:
return 0b111;
}
}
};
static_assert(sizeof(RCC) == 36 * sizeof(reg32_t), "RCC contains padding bytes.");
static_assert(std::is_standard_layout<RCC>::value, "RCC isn't standard layout.");
What I like about my current implementation is that as long as I turn on -flto
and -O2
then it pretty much inlines everything into ResetHandler which is very nice. I also use constexpr
for automatic calculation of system registers at compile-time. My primary question with this implementation is: does it make sense to automatically enable the RCC registers for the appropriate GPIO peripheral when "allocating"/creating an instance of the class? Or should I leave it to the application programmer to deal with such a hardware task themselves? The class will be useless without the RCC registers enabled.
1 Answer 1
I like the reg32_t
type alias and the good use of binary literals. The static_assert()
s are also a great way to prevent accidental mistakes, especially with bit fields where it's hard to spot if you added one bit too many.
Use of placement new
The point of new
is to create a new object. Placement new
creates a new object at a given location. However, the hardware registers are already there, you don't have to create them. So I would rather not use placement new
, but just write a static
member function that returns a pointer to the right memory location:
class RCC {
...
public:
RCC() = delete;
...
static RCC *get() {
return reinterpret_cast<RCC *>(0x40023800);
// return std::launder(reinterpret_cast<RCC *>(0x40023800)); // C++17
}
};
Since C++17, you should use std::launder()
to tell the compiler and static analysis tools that there really is a live object at that address, so it won't treat any access to it as undefined behavior.
While overloading operator new()
and calling it multiple times is fine in some cases, consider what would happen if you added constructors and destructors to your classes (for example, to ensure that the registers are in a well-defined state at startup, or to ensure all pins will be high-impedance when you stop using a hardware peripheral).
Another reason against your use of placement new
is that code like FOO &f = *new FOO()
looks like an instant memory leak, and might even cause some static analyzers to complain about it.
Since you are using __attribute__((section))
anyway, consider using sections to declare global instances of your classes at the right address, see this StackOverflow post.
Unnecessary use of this->
In C++ you almost never have to use this->
. I would remove it wherever it is unneeded.
Don't busy wait
Busy waiting uses CPU cycles unnecessarily, using more power which drains batteries faster and generates heat. On a STM32F4, you might want to go to sleep mode in the loop in main()
, which halts the CPU until an interrupt arrives. To toggle pin 13, you can set up a timer interrupt.
In some cases it is fine to busy wait if you know it's just for a few cycles, like when waiting for PLLRDY
to become set.
Inconsistent enums
Some enum
types are all lower-case, some start with a capital. I would use a consistent way to format type names, regardless of whether it is a struct
, union
, enum
or enum class
, and make sure it is different from how you format variables. Also, sometimes you add enum
in front of an existing enum name, but that is not necessary in C++. For example, you can write:
void SetMode(gpio_pin pin_num, GPIO::Mode mode);
Casting pointers to integers
While casting a pointer to an int
might work for your use case, it is more correct to cast then to std::uintptr_t
. This type is guaranteed to be large enough to fit a whole pointer.
Prefer using static constexpr
for constants
In C++ it is hardly ever necessary to use macros anymore. To declare a constant, prefer static constexpr
, like so:
static constexpr std::uint32_t HSE_CRYSTAL_FREQ_HZ = 25000000;
Avoid std::tuple
in non-generic code
std::tuple
can be very useful when writing generic code, especially in variadic templates. But they are otherwise a bit cumbersome to work with, in particular you need to use get<>()
to access tuple elements, and then you have to remember the order in which elements are stored. Instead, you can just create a struct
:
struct PLLClocks {
uint32_t pll_m;
uint32_t pll_n;
uint32_t pll_p;
};
And then let CalculatePLLClocks()
return a PLLClocks
.
Default switch cases
In SetMode()
, I would change default
to case GPIO::Mode::INPUT
. This makes it clear that you handle the case of input pins, and by not having a default
case, the compiler will be able to warn if you ever have an enum value that you didn't handle inside the switch
-statement.
-
1\$\begingroup\$ Great advice, thank you. The busy waits are there for testing reasons. Once I get the systick timer working correctly and I have an atomic static global counter then I will replace the loop with a proper time-based delay. I may go further and introduce a context/task switcher as well. To your first point, my usage of
new
is based off the talks that I have watched and the main reason they state for using it is that the constructor will be called after the call tonew
is complete. How will a static method accomplish this task? \$\endgroup\$dylanweber– dylanweber2022年08月03日 14:40:08 +00:00Commented Aug 3, 2022 at 14:40 -
\$\begingroup\$ Your current constructors are not doing anything, so they don't need to be called. If you use
new
to get a pointer to a peripheral, and more than one part of the code needs to access the same peripheral, then yournew
method would risk having the constructor called multiple times. In any case, theget()
method in my example could be modified to only call some function the first timeget()
is called, and that could serve as the constructor. Using linker sections might be the cleanest way to solve this though. \$\endgroup\$G. Sliepen– G. Sliepen2022年08月03日 14:48:57 +00:00Commented Aug 3, 2022 at 14:48