drivers/uart: stm32: Fix pm_constraint handling

Introduce new logic to set/release pm_constraint during serial TX
transactions.

First change is to introduce an internal flag and utility functions
to control the set/release constraint balancing per uart device.
This way, whatever the mix of transactions or API calls, we
ensure a single uart device can only do 1 or 0 to the PM state
constraint. Constraint can't then be set more than once, released w/o
having been set or released more than it was set.

The last part of the change reworks the triggers for constraints
set/release operations.
In order not to disturb driver operations, if irq driven mode or PM is
enabled, don't enable TC interrupt handling by default.
Instead, map the pm_constraint setting to the way TC flag is handled
in normal mode of operations (irq driven or async).
As a consequence, in irq driven mode, pm_constraint is set/released on
tx_enable/tx_disable api calls, which gives API user full control
on transaction protection vs low power operations.
Finally, we emulate the same behavior on TX poll transaction, by
enabling TC irq at the start of a stream and disabling TC irq once
stream is completed. This is controlled with a dedicated device flag.


Signed-off-by: Erwan Gouriou <[email protected]>

Author: erwango

drivers/serial/uart_stm32.c

@@ -53,6 +53,28 @@ LOG_MODULE_REGISTER(uart_stm32);
 
 #define TIMEOUT 1000
 
+#ifdef CONFIG_PM
+static void uart_stm32_pm_constraint_set(const struct device *dev)
+{
+	struct uart_stm32_data *data = DEV_DATA(dev);
+
+	if (!data->pm_constraint_on) {
+		data->pm_constraint_on = true;
+		pm_constraint_set(PM_STATE_SUSPEND_TO_IDLE);
+	}
+}
+
+static void uart_stm32_pm_constraint_release(const struct device *dev)
+{
+	struct uart_stm32_data *data = DEV_DATA(dev);
+
+	if (data->pm_constraint_on) {
+		data->pm_constraint_on = false;
+		pm_constraint_release(PM_STATE_SUSPEND_TO_IDLE);
+	}
+}
+#endif /* CONFIG_PM */
+
 static inline void uart_stm32_set_baudrate(const struct device *dev,
 					   uint32_t baud_rate)
 {
@@ -456,8 +478,23 @@ static void uart_stm32_poll_out(const struct device *dev,
 	while (!LL_USART_IsActiveFlag_TXE(UartInstance)) {
 	}
 
-	/* do not allow system to suspend until transmission has completed */
-	pm_constraint_set(PM_STATE_SUSPEND_TO_IDLE);
+#ifdef CONFIG_PM
+	struct uart_stm32_data *data = DEV_DATA(dev);
+
+	if (!data->tx_poll_stream_on) {
+		data->tx_poll_stream_on = true;
+
+		/* Don't allow system to suspend until stream
+		 * transmission has completed
+		 */
+		uart_stm32_pm_constraint_set(dev);
+
+		/* Enable TC interrupt so we can release suspend
+		 * constraint when done
+		 */
+		LL_USART_EnableIT_TC(UartInstance);
+	}
+#endif /* CONFIG_PM */
 
 	LL_USART_TransmitData8(UartInstance, (uint8_t)c);
 }
@@ -520,11 +557,6 @@ static int uart_stm32_fifo_fill(const struct device *dev,
 	USART_TypeDef *UartInstance = UART_STRUCT(dev);
 	uint8_t num_tx = 0U;
 
-	if ((size > 0) && LL_USART_IsActiveFlag_TXE(UartInstance)) {
-		/* do not allow system to suspend until transmission has completed */
-		pm_constraint_set(PM_STATE_SUSPEND_TO_IDLE);
-	}
-
 	while ((size - num_tx > 0) &&
 	       LL_USART_IsActiveFlag_TXE(UartInstance)) {
 		/* TXE flag will be cleared with byte write to DR|RDR register */
@@ -563,13 +595,21 @@ static void uart_stm32_irq_tx_enable(const struct device *dev)
 	USART_TypeDef *UartInstance = UART_STRUCT(dev);
 
 	LL_USART_EnableIT_TC(UartInstance);
+
+#ifdef CONFIG_PM
+	uart_stm32_pm_constraint_set(dev);
+#endif
 }
 
 static void uart_stm32_irq_tx_disable(const struct device *dev)
 {
 	USART_TypeDef *UartInstance = UART_STRUCT(dev);
 
 	LL_USART_DisableIT_TC(UartInstance);
+
+#ifdef CONFIG_PM
+	uart_stm32_pm_constraint_release(dev);
+#endif
 }
 
 static int uart_stm32_irq_tx_ready(const struct device *dev)
@@ -799,16 +839,30 @@ static void uart_stm32_dma_rx_flush(const struct device *dev)
 static void uart_stm32_isr(const struct device *dev)
 {
 	struct uart_stm32_data *data = DEV_DATA(dev);
+#if defined(CONFIG_PM) || defined(CONFIG_UART_ASYNC_API)
 	USART_TypeDef *UartInstance = UART_STRUCT(dev);
+#endif
 
-#ifdef CONFIG_UART_INTERRUPT_DRIVEN
-	if (LL_USART_IsActiveFlag_TC(UartInstance)) {
-		LL_USART_ClearFlag_TC(UartInstance);
-
-		/* allow system to suspend, UART has now finished */
-		pm_constraint_release(PM_STATE_SUSPEND_TO_IDLE);
+#ifdef CONFIG_PM
+	if (LL_USART_IsEnabledIT_TC(UartInstance) &&
+		LL_USART_IsActiveFlag_TC(UartInstance)) {
+
+		if (data->tx_poll_stream_on) {
+			/* A poll stream transmition just completed,
+			 * allow system to suspend
+			 */
+			LL_USART_DisableIT_TC(UartInstance);
+			data->tx_poll_stream_on = false;
+			uart_stm32_pm_constraint_release(dev);
+		}
+		/* Stream transmition was either async or IRQ based,
+		 * constraint will be released at the same time TC IT
+		 * is disabled
+		 */
 	}
+#endif
 
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
 	if (data->user_cb) {
 		data->user_cb(dev, data->user_data);
 	}
@@ -837,8 +891,9 @@ static void uart_stm32_isr(const struct device *dev)
 		/* Generate TX_DONE event when transmission is done */
 		async_evt_tx_done(data);
 
-		/* allow system to suspend, UART has now finished */
-		pm_constraint_release(PM_STATE_SUSPEND_TO_IDLE);
+#ifdef CONFIG_PM
+		uart_stm32_pm_constraint_release(dev);
+#endif
 	}
 
 	/* Clear errors */
@@ -849,12 +904,18 @@ static void uart_stm32_isr(const struct device *dev)
 static void uart_stm32_isr(const struct device *dev)
 {
 	USART_TypeDef *UartInstance = UART_STRUCT(dev);
+	struct uart_stm32_data *data = DEV_DATA(dev);
 
 	if (LL_USART_IsActiveFlag_TC(UartInstance)) {
 		LL_USART_ClearFlag_TC(UartInstance);
+		LL_USART_DisableIT_TC(UartInstance);
+
+		__ASSERT_NO_MSG(data->tx_poll_stream_on);
+
+		data->tx_poll_stream_on = false;
 
 		/* allow system to suspend, UART has now finished */
-		pm_constraint_release(PM_STATE_SUSPEND_TO_IDLE);
+		uart_stm32_pm_constraint_release(dev);
 	}
 }
 #endif /* (CONFIG_UART_INTERRUPT_DRIVEN) || defined(CONFIG_UART_ASYNC_API) */
@@ -1079,8 +1140,10 @@ static int uart_stm32_async_tx(const struct device *dev,
 	/* Start TX timer */
 	async_timer_start(&data->dma_tx.timeout_work, data->dma_tx.timeout);
 
+#ifdef CONFIG_PM
 	/* do not allow system to suspend until transmission has completed */
-	pm_constraint_set(PM_STATE_SUSPEND_TO_IDLE);
+	uart_stm32_pm_constraint_set(dev);
+#endif
 
 	/* Enable TX DMA requests */
 	uart_stm32_dma_tx_enable(dev);
@@ -1446,14 +1509,6 @@ static int uart_stm32_init(const struct device *dev)
 	config->irq_config_func(dev);
 #endif /* defined(CONFIG_UART_INTERRUPT_DRIVEN) || defined(CONFIG_UART_ASYNC_API) */
 
-#if defined(CONFIG_UART_INTERRUPT_DRIVEN) || defined(CONFIG_PM)
-	/* Clear TC flag */
-	LL_USART_ClearFlag_TC(UartInstance);
-
-	/* Enable TC interrupt so we can release suspend constraint when done */
-	LL_USART_EnableIT_TC(UartInstance);
-#endif /* defined(CONFIG_UART_INTERRUPT_DRIVEN) || defined(CONFIG_PM) */
-
 #ifdef CONFIG_UART_ASYNC_API
 	return uart_stm32_async_init(dev);
 #else

drivers/serial/uart_stm32.h

@@ -72,6 +72,10 @@ struct uart_stm32_data {
 	uint8_t *rx_next_buffer;
 	size_t rx_next_buffer_len;
 #endif
+#ifdef CONFIG_PM
+	bool tx_poll_stream_on;
+	bool pm_constraint_on;
+#endif
 };
 
 #endif	/* ZEPHYR_DRIVERS_SERIAL_UART_STM32_H_ */