Optimize USART driver's code size

Using tables reduces code size while also improving lisibility. That is a
win-win

drv/dma.c

@@ -95,21 +95,6 @@ void dma_reset(enum DmaPeriph dma, enum DmaChannel channel)
 	regs->CPAR = 0;
 }
 
-void dma_exit_critical(enum DmaPeriph dma, enum DmaChannel channel)
-{
-	switch (dma) {
-		case DMA_PERIPH_1:
-			nvic_enable(NVIC_IRQ_DMA1_CHANNEL1 + channel);
-			break;
-		case DMA_PERIPH_2:
-			nvic_enable(NVIC_IRQ_DMA2_CHANNEL1 + channel);
-			break;
-		default:
-			return;
-			break;
-	}
-}
-
 void dma_start(enum DmaPeriph dma, enum DmaChannel channel,
 		volatile void* mem, uint16_t size)
 {
@@ -132,21 +117,6 @@ void dma_start(enum DmaPeriph dma, enum DmaChannel channel,
 	}
 }
 
-void dma_enter_critical(enum DmaPeriph dma, enum DmaChannel channel)
-{
-	switch (dma) {
-		case DMA_PERIPH_1:
-			nvic_disable(NVIC_IRQ_DMA1_CHANNEL1 + channel);
-			break;
-		case DMA_PERIPH_2:
-			nvic_disable(NVIC_IRQ_DMA2_CHANNEL1 + channel);
-			break;
-		default:
-			return;
-			break;
-	}
-}
-
 void dma_stop(enum DmaPeriph dma, enum DmaChannel channel)
 {
 	switch (dma) {
@@ -168,6 +138,36 @@ void dma_stop(enum DmaPeriph dma, enum DmaChannel channel)
 	}
 }
 
+void dma_enter_critical(enum DmaPeriph dma, enum DmaChannel channel)
+{
+	switch (dma) {
+		case DMA_PERIPH_1:
+			nvic_disable(NVIC_IRQ_DMA1_CHANNEL1 + channel);
+			break;
+		case DMA_PERIPH_2:
+			nvic_disable(NVIC_IRQ_DMA2_CHANNEL1 + channel);
+			break;
+		default:
+			return;
+			break;
+	}
+}
+
+void dma_exit_critical(enum DmaPeriph dma, enum DmaChannel channel)
+{
+	switch (dma) {
+		case DMA_PERIPH_1:
+			nvic_enable(NVIC_IRQ_DMA1_CHANNEL1 + channel);
+			break;
+		case DMA_PERIPH_2:
+			nvic_enable(NVIC_IRQ_DMA2_CHANNEL1 + channel);
+			break;
+		default:
+			return;
+			break;
+	}
+}
+
 uint16_t dma_get_remaining(enum DmaPeriph dma, enum DmaChannel channel)
 {
 	switch (dma) {
@@ -185,6 +185,9 @@ uint16_t dma_get_remaining(enum DmaPeriph dma, enum DmaChannel channel)
 
 //--local functions-------------------------------------------------------------
 
+/**
+ * Applies the given configuration mask to the given DMA channel
+ */
 static void configure_dma(volatile struct DMA* dma, enum DmaChannel channel,
 		enum DmaConfig config_mask, volatile void* periph)
 {
@@ -195,6 +198,9 @@ static void configure_dma(volatile struct DMA* dma, enum DmaChannel channel,
 	regs->CPAR = (uint32_t)periph;
 }
 
+/**
+ * Starts the given DMA channel using the given parameters
+ */
 static void start_dma(volatile struct DMA* dma, enum DmaChannel channel,
 		volatile void* mem, uint16_t size)
 {

drv/dma.h

@@ -15,11 +15,19 @@
 
 //--type definitions------------------------------------------------------------
 
+/**
+ * Available DMA peripherals. Note that some of these peripherals may not be
+ * available on all chips
+ */
 enum DmaPeriph {
 	DMA_PERIPH_1,
 	DMA_PERIPH_2,
 };
 
+/**
+ * Available DMA channels. Note that some of these channels may not be
+ * available on all chips and all DMA peripheral
+ */
 enum DmaChannel {
 	DMA_CHANNEL_1 = 0,
 	DMA_CHANNEL_2,
@@ -30,6 +38,9 @@ enum DmaChannel {
 	DMA_CHANNEL_7, //not available for DMA 2
 };
 
+/**
+ * Configuration options for memory-to-peripheral transfers
+ */
 enum DmaConfig {
 	DMA_CONFIG_IRQ_COMPLETE = (0x1 <<  1),
 	DMA_CONFIG_IRQ_HALF     = (0x1 <<  2),
@@ -51,6 +62,9 @@ enum DmaConfig {
 	DMA_CONFIG_PRIO_VHIGH   = (0x3 << 12),
 };
 
+/**
+ * Configuration options for memory-to-memory transfers
+ */
 enum DmaConfigM2M {
 	DMA_CONFIG_M2M_IRQ_COMPLETE = (0x1 <<  1),
 	DMA_CONFIG_M2M_IRQ_HALF     = (0x1 <<  2),
@@ -69,14 +83,25 @@ enum DmaConfigM2M {
 	DMA_CONFIG_M2M_PRIO_VHIGH   = (0x3 << 12),
 };
 
+/**
+ * Available sources for a DMA IRQ. These sources can be enabled independently
+ * in the DMA configuration.
+ */
 enum DmaIRQSource {
 	DMA_IRQ_SOURCE_COMPLETE = (0x1 << 1),
 	DMA_IRQ_SOURCE_HALF     = (0x1 << 2),
 	DMA_IQR_SOURCE_ERROR    = (0x2 << 3),
 };
 
+/**
+ * Prototype of the IRQ callbacks that the applicative code can provide
+ */
 typedef void (*DmaCallback)(enum DmaIRQSource, volatile void* param);
 
+/**
+ * Generic struct used to share DAM configs between peripheral drivers and
+ * services providing DMA interfaces
+ */
 struct DmaParam {
 	void* periph;
 	enum DmaConfig config;    //DMA config, must correspond to peripheral
@@ -87,25 +112,67 @@ struct DmaParam {
 
 //--functions-------------------------------------------------------------------
 
+/**
+ * Configures the given DMA channel with the provided configuration, to perform
+ * a transfer to or from the given peripheral register. The specified callback,
+ * if any, will be called on the enabled IRQ sources, with the specified
+ * parameter, if any.
+ *
+ * This function doesn't initiate transfers, use dma_start() for that
+ */
 void dma_configure(enum DmaPeriph dma, enum DmaChannel channel,
 		enum DmaConfig config_mask, volatile void* periph,
 		DmaCallback callback, volatile void* cb_param);
 
+/**
+ * Unimplemented
+ */
 void dma_configure_mem2mem(enum DmaPeriph dma, enum DmaChannel channel,
 		enum DmaConfigM2M config_mask, const void* src, void* dest,
 		uint16_t size, DmaCallback callback, void* cb_param);
 
+/**
+ * Resets the given DMA channel, restoring the default configuration and
+ * disabling it
+ */
 void dma_reset(enum DmaPeriph dma, enum DmaChannel channel);
 
-void dma_exit_critical(enum DmaPeriph dma, enum DmaChannel channel);
+/**
-
+ * Initiate a transfer on the given DMA channel, to or from the given memory
+ * address, and of the specified size.
+ *
+ * Should only be used after the channel has been configured through
+ * dma_configure()
+ * All transfers started must be eventually stopped, or the channel reset, for
+ * proper IRQ behavior.
+ */
 void dma_start(enum DmaPeriph dma, enum DmaChannel channel,
 		volatile void* mem, uint16_t size);
 
-void dma_enter_critical(enum DmaPeriph dma, enum DmaChannel channel);
+/**
-
+ * Stops a transfer on the given DMA channel. If the transfer has already
+ * ended, properly shutdown the channel. Configuration is not lost after calling
+ * this function and dma_start() may be called again
+ */
 void dma_stop(enum DmaPeriph dma, enum DmaChannel channel);
 
+/**
+ * Enters a DMA critical section, disabling the given channel's IRQs until
+ * dma_exit_critical() is called
+ */
+void dma_enter_critical(enum DmaPeriph dma, enum DmaChannel channel);
+
+/**
+ * Exists a DMA critical section previously entered through
+ * dma_enter_critical(). Reenables the given channel's IRQs
+ */
+void dma_exit_critical(enum DmaPeriph dma, enum DmaChannel channel);
+
+/**
+ * Returns the remaining number of bytes to be transmitted while a transfer is
+ * running. When no transfer is running, returns the number of bytes to transfer
+ * next
+ */
 uint16_t dma_get_remaining(enum DmaPeriph dma, enum DmaChannel channe);
 
 

drv/usart.c

@@ -8,7 +8,6 @@
 //--includes--------------------------------------------------------------------
 
 #include "usart.h"
-#include "nvic.h"
 #include "usart_regs.h"
 #include "reg.h"
 
@@ -30,50 +29,52 @@ static void configure_baudrate(volatile struct USART* regs, uint32_t clock,
 
 //--local variables-------------------------------------------------------------
 
-static volatile struct USART* const usart1 = (struct USART*)USART1_BASE_ADDRESS;
+static volatile struct USART* const usarts[] = {
-static volatile struct USART* const usart2 = (struct USART*)USART2_BASE_ADDRESS;
+	(struct USART*)USART1_BASE_ADDRESS,
-static volatile struct USART* const usart3 = (struct USART*)USART3_BASE_ADDRESS;
+	(struct USART*)USART2_BASE_ADDRESS,
-
+	(struct USART*)USART3_BASE_ADDRESS,
-static const struct DmaParam usart1_rx_param = {
-	(void*)&usart1->DR,
-	DMA_CONFIG,
-	DMA_PERIPH_1,
-	DMA_CHANNEL_5,
 };
 
-static const struct DmaParam usart2_rx_param = {
+static const struct DmaParam usarts_rx_param[] = {
-	(void*)&usart2->DR,
+	{
-	DMA_CONFIG,
+		(void*)&usarts[USART_PERIPH_1]->DR,
-	DMA_PERIPH_1,
+		DMA_CONFIG,
-	DMA_CHANNEL_6,
+		DMA_PERIPH_1,
+		DMA_CHANNEL_5,
+	},
+	{
+		(void*)&usarts[USART_PERIPH_2]->DR,
+		DMA_CONFIG,
+		DMA_PERIPH_1,
+		DMA_CHANNEL_6,
+	},
+	{
+		(void*)&usarts[USART_PERIPH_3]->DR,
+		DMA_CONFIG,
+		DMA_PERIPH_1,
+		DMA_CHANNEL_3,
+	},
 };
 
-static const struct DmaParam usart3_rx_param = {
+static const struct DmaParam usarts_tx_param[] = {
-	(void*)&usart3->DR,
+	{
-	DMA_CONFIG,
+		(void*)&usarts[USART_PERIPH_1]->DR,
-	DMA_PERIPH_1,
+		DMA_CONFIG,
-	DMA_CHANNEL_3,
+		DMA_PERIPH_1,
-};
+		DMA_CHANNEL_4,
-
+	},
-static const struct DmaParam usart1_tx_param = {
+	{
-	(void*)&usart1->DR,
+		(void*)&usarts[USART_PERIPH_2]->DR,
-	DMA_CONFIG,
+		DMA_CONFIG,
-	DMA_PERIPH_1,
+		DMA_PERIPH_1,
-	DMA_CHANNEL_4,
+		DMA_CHANNEL_7,
-};
+	},
-
+	{
-static const struct DmaParam usart2_tx_param = {
+		(void*)&usarts[USART_PERIPH_3]->DR,
-	(void*)&usart2->DR,
+		DMA_CONFIG,
-	DMA_CONFIG,
+		DMA_PERIPH_1,
-	DMA_PERIPH_1,
+		DMA_CHANNEL_2,
-	DMA_CHANNEL_7,
+	},
-};
-
-static const struct DmaParam usart3_tx_param = {
-	(void*)&usart3->DR,
-	DMA_CONFIG,
-	DMA_PERIPH_1,
-	DMA_CHANNEL_2,
 };
 
 
@@ -88,73 +89,50 @@ void usart_configure(enum UsartPeriph periph, enum UsartConfig config,
 	switch (periph) {
 		case USART_PERIPH_1:
 			rcc_enable(RCC_AHB_NONE, RCC_APB1_NONE, RCC_APB2_USART);
-			configure_baudrate(usart1, clocks.apb2_freq, baudrate);
+			configure_baudrate(usarts[USART_PERIPH_1], clocks.apb2_freq,
-			configure_usart(usart1, config);
+					baudrate);
+			configure_usart(usarts[USART_PERIPH_1], config);
 			break;
 		case USART_PERIPH_2:
 			rcc_enable(RCC_AHB_NONE, RCC_APB1_USART2, RCC_APB2_NONE);
-			configure_baudrate(usart2, clocks.apb1_freq, baudrate);
+			configure_baudrate(usarts[USART_PERIPH_2], clocks.apb1_freq,
-			configure_usart(usart2, config);
+					baudrate);
+			configure_usart(usarts[USART_PERIPH_2], config);
 			break;
 		case USART_PERIPH_3:
 			rcc_enable(RCC_AHB_NONE, RCC_APB1_USART3, RCC_APB2_NONE);
-			configure_baudrate(usart3, clocks.apb1_freq, baudrate);
+			configure_baudrate(usarts[USART_PERIPH_3], clocks.apb1_freq,
-			configure_usart(usart3, config);
+					baudrate);
+			configure_usart(usarts[USART_PERIPH_3], config);
 			break;
 		default:
 			break;
 	}
 }
 
-uint32_t usart_write_byte(enum UsartPeriph periph, uint8_t byte)
+uint32_t usart_read_byte(enum UsartPeriph periph, uint8_t* byte)
 {
-	volatile struct USART* regs;
+	if (periph > USART_PERIPH_3) {
-
+		return 1;
-	switch (periph) {
-		case USART_PERIPH_1:
-			regs = usart1;
-			break;
-		case USART_PERIPH_2:
-			regs = usart2;
-			break;
-		case USART_PERIPH_3:
-			regs = usart3;
-			break;
-		default:
-			return 1;
-			break;
 	}
 
-	//only write data if the tx register it empty, give up otherwise
+	if (usarts[periph]->SR.RXNE) {
-	if (regs->SR.TXE) {
+		*byte = usarts[periph]->DR.DR;
-		reg_write(regs->DR, USART_DR_DR, byte);
 		return 0;
 	} else {
 		return 1;
 	}
 }
 
-uint32_t usart_read_byte(enum UsartPeriph periph, uint8_t* byte)
+uint32_t usart_write_byte(enum UsartPeriph periph, uint8_t byte)
 {
-	volatile struct USART* regs;
+	if (periph > USART_PERIPH_3) {
-
+		return 1;
-	switch (periph) {
-		case USART_PERIPH_1:
-			regs = usart1;
-			break;
-		case USART_PERIPH_2:
-			regs = usart2;
-			break;
-		case USART_PERIPH_3:
-			regs = usart3;
-			break;
-		default:
-			return 1;
-			break;
 	}
 
-	if (regs->SR.RXNE) {
+	//only write data if the tx register it empty, give up otherwise
-		*byte = regs->DR.DR;
+	if (usarts[periph]->SR.TXE) {
+		reg_write(usarts[periph]->DR, USART_DR_DR, byte);
 		return 0;
 	} else {
 		return 1;
@@ -163,50 +141,22 @@ uint32_t usart_read_byte(enum UsartPeriph periph, uint8_t* byte)
 
 const struct DmaParam* usart_configure_rx_dma(enum UsartPeriph periph)
 {
-	const struct DmaParam* param;
+	if (periph > USART_PERIPH_3) {
-
+		return nullptr;
-	switch (periph) {
-		case USART_PERIPH_1:
-			param = &usart1_rx_param;
-			reg_set(usart1->CR3, USART_CR3_DMAR);
-			break;
-		case USART_PERIPH_2:
-			param = &usart2_rx_param;
-			reg_set(usart2->CR3, USART_CR3_DMAR);
-			break;
-		case USART_PERIPH_3:
-			param = &usart3_rx_param;
-			reg_set(usart3->CR3, USART_CR3_DMAR);
-			break;
-		default:
-			return nullptr;
 	}
 
-	return param;
+	reg_set(usarts[periph]->CR3, USART_CR3_DMAR);
+	return &usarts_rx_param[periph];
 }
 
 const struct DmaParam* usart_configure_tx_dma(enum UsartPeriph periph)
 {
-	const struct DmaParam* param;
+	if (periph > USART_PERIPH_3) {
-
+		return nullptr;
-	switch (periph) {
-		case USART_PERIPH_1:
-			param = &usart1_tx_param;
-			reg_set(usart1->CR3, USART_CR3_DMAT);
-			break;
-		case USART_PERIPH_2:
-			param = &usart2_tx_param;
-			reg_set(usart2->CR3, USART_CR3_DMAT);
-			break;
-		case USART_PERIPH_3:
-			param = &usart3_tx_param;
-			reg_set(usart3->CR3, USART_CR3_DMAT);
-			break;
-		default:
-			return nullptr;
 	}
 
-	return param;
+	reg_set(usarts[periph]->CR3, USART_CR3_DMAT);
+	return &usarts_tx_param[periph];
 }
 
 
@@ -295,7 +245,7 @@ static void configure_usart(volatile struct USART* regs,
 
 /**
  * Configure the given registers with the given baudrate. Baudrate is dependant
- * on the peripheric's clock and may not be exact due to precision errors (see
+ * on the peripheral's clock and may not be exact due to precision errors (see
  * table 192 in documentation)
  */
 static void configure_baudrate(volatile struct USART* regs, uint32_t clock,

drv/usart.h

@@ -17,12 +17,19 @@
 
 //--type definitions------------------------------------------------------------
 
+/**
+ * Available USART peripherals. Note that some of these peripherals may not be
+ * available on all chips
+ */
 enum UsartPeriph {
 	USART_PERIPH_1,
 	USART_PERIPH_2,
 	USART_PERIPH_3,
 };
 
+/**
+ * Available configuration options
+ */
 enum UsartConfig {
 	USART_CONFIG_7E1,
 	USART_CONFIG_7E2,
@@ -39,14 +46,64 @@ enum UsartConfig {
 
 //--functions-------------------------------------------------------------------
 
+/**
+ * Configures the given USART peripheral using the provided condiguration
+ * options and baudrate. The baudrate may be any value supported by the
+ * peripheral, though some may not be exact due to precision errors (see
+ * table 192 in documentation). The baudrate is dependant on the peripheral's
+ * clock and changes to the later after this function has been called will cause
+ * the effective baudrate to change
+ *
+ * This function doesn't configure the required ports. This should be done using
+ * the gpio driver:
+ * - Tx port should be using GPIO_CONFIG_OUT_ALT_PUSH_PULL with
+ * the appropriate output speed based on the baurate (see GpioMode)
+ * - Rx port should be using GPIO_CONFIG_IN_FLOATING in input mode
+ * Both ports do not need to be configured if not used (e.g. if only using Tx,
+ * the Rx port can be left unconfigured)
+ */
 void usart_configure(enum UsartPeriph periph, enum UsartConfig config,
 		uint32_t baudrate);
+
+/**
+ * Resets the given USART peripheral, applying the default configuration and
+ * disabling it
+ */
 void usart_reset(enum UsartPeriph periph);
 
-uint32_t usart_write_byte(enum UsartPeriph periph, uint8_t byte);
+/**
+ * Reads a single byte to the given USART peripheral, returning 0 if
+ * successfull, 1 otherwise.
+ *
+ * The Rx port must be configured for this function to ever return successfully
+ */
 uint32_t usart_read_byte(enum UsartPeriph periph, uint8_t* byte);
 
+/**
+ * Writes a single byte to the given USART peripheral, returning 0 if
+ * successfull, 1 otherwise.
+ *
+ * The Tx port must be configured for this function to do anything, though the
+ * function would still return 0
+ */
+uint32_t usart_write_byte(enum UsartPeriph periph, uint8_t byte);
+
+/**
+ * Configures the given USART peripheral for DMA Rx operations, returning the
+ * corresponding DMA parameters to be used.
+ *
+ * The DMA must be configured separately using the DMA driver or an existing
+ * service
+ */
 const struct DmaParam* usart_configure_rx_dma(enum UsartPeriph periph);
+
+/**
+ * Configures the given USART peripheral for DMA Rx operations, returning the
+ * corresponding DMA parameters to be used.
+ *
+ * The DMA must be configured separately using the DMA driver or an existing
+ * service
+ */
 const struct DmaParam* usart_configure_tx_dma(enum UsartPeriph periph);