From 0fdc9b5b125c60d80a368360851a50f1f03710aa Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Thu, 11 May 2017 17:57:29 +0300 Subject: [PATCH] drivers: serial: Clarify usage of TX/RX IRQ predicates. uart_irq_tx_empty() function proved to be problematic: its semantics was not documented properly, and many hardware uses terminology like "TX register empty" to signify condition of TX register being ready to accept another character (what in Zephyr is tested with uart_irq_tx_ready()). To avoid confusion, uart_irq_tx_empty() was renamed to uart_irq_tx_complete(), propagating to drivers/serial device methods. The semantics and usage model of all of uart_irq_rx_ready(), uart_irq_tx_ready(), uart_irq_tx_complete() is now described in detail. Signed-off-by: Paul Sokolovsky --- drivers/serial/uart_cc32xx.c | 4 +-- drivers/serial/uart_cmsdk_apb.c | 4 +-- drivers/serial/uart_fe310.c | 4 +-- drivers/serial/uart_mcux.c | 6 ++-- drivers/serial/uart_mcux_lpsci.c | 6 ++-- drivers/serial/uart_mcux_lpuart.c | 6 ++-- drivers/serial/uart_nrf5.c | 4 +-- drivers/serial/uart_ns16550.c | 4 +-- drivers/serial/uart_qmsi.c | 4 +-- drivers/serial/uart_stm32.c | 4 +-- include/uart.h | 49 ++++++++++++++++++++++++------- 11 files changed, 62 insertions(+), 33 deletions(-) diff --git a/drivers/serial/uart_cc32xx.c b/drivers/serial/uart_cc32xx.c index a61af59579b..829254bc627 100644 --- a/drivers/serial/uart_cc32xx.c +++ b/drivers/serial/uart_cc32xx.c @@ -202,7 +202,7 @@ static void uart_cc32xx_irq_rx_disable(struct device *dev) MAP_UARTIntDisable((unsigned long)config->base, UART_INT_RX); } -static int uart_cc32xx_irq_tx_empty(struct device *dev) +static int uart_cc32xx_irq_tx_complete(struct device *dev) { const struct uart_device_config *config = DEV_CFG(dev); @@ -296,7 +296,7 @@ static const struct uart_driver_api uart_cc32xx_driver_api = { .irq_tx_ready = uart_cc32xx_irq_tx_ready, .irq_rx_enable = uart_cc32xx_irq_rx_enable, .irq_rx_disable = uart_cc32xx_irq_rx_disable, - .irq_tx_empty = uart_cc32xx_irq_tx_empty, + .irq_tx_complete = uart_cc32xx_irq_tx_complete, .irq_rx_ready = uart_cc32xx_irq_rx_ready, .irq_err_enable = uart_cc32xx_irq_err_enable, .irq_err_disable = uart_cc32xx_irq_err_disable, diff --git a/drivers/serial/uart_cmsdk_apb.c b/drivers/serial/uart_cmsdk_apb.c index d02dfca9488..2b93aec2e1c 100644 --- a/drivers/serial/uart_cmsdk_apb.c +++ b/drivers/serial/uart_cmsdk_apb.c @@ -316,7 +316,7 @@ static void uart_cmsdk_apb_irq_rx_disable(struct device *dev) * * @return 1 if an interrupt is ready, 0 otherwise */ -static int uart_cmsdk_apb_irq_tx_empty(struct device *dev) +static int uart_cmsdk_apb_irq_tx_complete(struct device *dev) { return uart_cmsdk_apb_irq_tx_ready(dev); } @@ -435,7 +435,7 @@ static const struct uart_driver_api uart_cmsdk_apb_driver_api = { .irq_tx_ready = uart_cmsdk_apb_irq_tx_ready, .irq_rx_enable = uart_cmsdk_apb_irq_rx_enable, .irq_rx_disable = uart_cmsdk_apb_irq_rx_disable, - .irq_tx_empty = uart_cmsdk_apb_irq_tx_empty, + .irq_tx_complete = uart_cmsdk_apb_irq_tx_complete, .irq_rx_ready = uart_cmsdk_apb_irq_rx_ready, .irq_err_enable = uart_cmsdk_apb_irq_err_enable, .irq_err_disable = uart_cmsdk_apb_irq_err_disable, diff --git a/drivers/serial/uart_fe310.c b/drivers/serial/uart_fe310.c index 0adf616859d..582400edfea 100644 --- a/drivers/serial/uart_fe310.c +++ b/drivers/serial/uart_fe310.c @@ -217,7 +217,7 @@ static int uart_fe310_irq_tx_ready(struct device *dev) * * @return 1 if nothing remains to be transmitted, 0 otherwise */ -static int uart_fe310_irq_tx_empty(struct device *dev) +static int uart_fe310_irq_tx_complete(struct device *dev) { volatile struct uart_fe310_regs_t *uart = DEV_UART(dev); @@ -361,7 +361,7 @@ static const struct uart_driver_api uart_fe310_driver_api = { .irq_tx_enable = uart_fe310_irq_tx_enable, .irq_tx_disable = uart_fe310_irq_tx_disable, .irq_tx_ready = uart_fe310_irq_tx_ready, - .irq_tx_empty = uart_fe310_irq_tx_empty, + .irq_tx_complete = uart_fe310_irq_tx_complete, .irq_rx_enable = uart_fe310_irq_rx_enable, .irq_rx_disable = uart_fe310_irq_rx_disable, .irq_rx_ready = uart_fe310_irq_rx_ready, diff --git a/drivers/serial/uart_mcux.c b/drivers/serial/uart_mcux.c index cb8da3d0644..e1b72f3b6d0 100644 --- a/drivers/serial/uart_mcux.c +++ b/drivers/serial/uart_mcux.c @@ -124,7 +124,7 @@ static void uart_mcux_irq_tx_disable(struct device *dev) UART_DisableInterrupts(config->base, mask); } -static int uart_mcux_irq_tx_empty(struct device *dev) +static int uart_mcux_irq_tx_complete(struct device *dev) { const struct uart_mcux_config *config = dev->config->config_info; u32_t flags = UART_GetStatusFlags(config->base); @@ -138,7 +138,7 @@ static int uart_mcux_irq_tx_ready(struct device *dev) u32_t mask = kUART_TxDataRegEmptyInterruptEnable; return (UART_GetEnabledInterrupts(config->base) & mask) - && uart_mcux_irq_tx_empty(dev); + && uart_mcux_irq_tx_complete(dev); } static void uart_mcux_irq_rx_enable(struct device *dev) @@ -254,7 +254,7 @@ static const struct uart_driver_api uart_mcux_driver_api = { .fifo_read = uart_mcux_fifo_read, .irq_tx_enable = uart_mcux_irq_tx_enable, .irq_tx_disable = uart_mcux_irq_tx_disable, - .irq_tx_empty = uart_mcux_irq_tx_empty, + .irq_tx_complete = uart_mcux_irq_tx_complete, .irq_tx_ready = uart_mcux_irq_tx_ready, .irq_rx_enable = uart_mcux_irq_rx_enable, .irq_rx_disable = uart_mcux_irq_rx_disable, diff --git a/drivers/serial/uart_mcux_lpsci.c b/drivers/serial/uart_mcux_lpsci.c index 5d6c08d6d95..359faffd0b0 100644 --- a/drivers/serial/uart_mcux_lpsci.c +++ b/drivers/serial/uart_mcux_lpsci.c @@ -127,7 +127,7 @@ static void mcux_lpsci_irq_tx_disable(struct device *dev) LPSCI_DisableInterrupts(config->base, mask); } -static int mcux_lpsci_irq_tx_empty(struct device *dev) +static int mcux_lpsci_irq_tx_complete(struct device *dev) { const struct mcux_lpsci_config *config = dev->config->config_info; u32_t flags = LPSCI_GetStatusFlags(config->base); @@ -141,7 +141,7 @@ static int mcux_lpsci_irq_tx_ready(struct device *dev) u32_t mask = kLPSCI_TxDataRegEmptyInterruptEnable; return (LPSCI_GetEnabledInterrupts(config->base) & mask) - && mcux_lpsci_irq_tx_empty(dev); + && mcux_lpsci_irq_tx_complete(dev); } static void mcux_lpsci_irq_rx_enable(struct device *dev) @@ -258,7 +258,7 @@ static const struct uart_driver_api mcux_lpsci_driver_api = { .fifo_read = mcux_lpsci_fifo_read, .irq_tx_enable = mcux_lpsci_irq_tx_enable, .irq_tx_disable = mcux_lpsci_irq_tx_disable, - .irq_tx_empty = mcux_lpsci_irq_tx_empty, + .irq_tx_complete = mcux_lpsci_irq_tx_complete, .irq_tx_ready = mcux_lpsci_irq_tx_ready, .irq_rx_enable = mcux_lpsci_irq_rx_enable, .irq_rx_disable = mcux_lpsci_irq_rx_disable, diff --git a/drivers/serial/uart_mcux_lpuart.c b/drivers/serial/uart_mcux_lpuart.c index fa50fc27adf..02cbd0c941d 100644 --- a/drivers/serial/uart_mcux_lpuart.c +++ b/drivers/serial/uart_mcux_lpuart.c @@ -127,7 +127,7 @@ static void mcux_lpuart_irq_tx_disable(struct device *dev) LPUART_DisableInterrupts(config->base, mask); } -static int mcux_lpuart_irq_tx_empty(struct device *dev) +static int mcux_lpuart_irq_tx_complete(struct device *dev) { const struct mcux_lpuart_config *config = dev->config->config_info; u32_t flags = LPUART_GetStatusFlags(config->base); @@ -141,7 +141,7 @@ static int mcux_lpuart_irq_tx_ready(struct device *dev) u32_t mask = kLPUART_TxDataRegEmptyInterruptEnable; return (LPUART_GetEnabledInterrupts(config->base) & mask) - && mcux_lpuart_irq_tx_empty(dev); + && mcux_lpuart_irq_tx_complete(dev); } static void mcux_lpuart_irq_rx_enable(struct device *dev) @@ -258,7 +258,7 @@ static const struct uart_driver_api mcux_lpuart_driver_api = { .fifo_read = mcux_lpuart_fifo_read, .irq_tx_enable = mcux_lpuart_irq_tx_enable, .irq_tx_disable = mcux_lpuart_irq_tx_disable, - .irq_tx_empty = mcux_lpuart_irq_tx_empty, + .irq_tx_complete = mcux_lpuart_irq_tx_complete, .irq_tx_ready = mcux_lpuart_irq_tx_ready, .irq_rx_enable = mcux_lpuart_irq_rx_enable, .irq_rx_disable = mcux_lpuart_irq_rx_disable, diff --git a/drivers/serial/uart_nrf5.c b/drivers/serial/uart_nrf5.c index 03444ed8786..c4854c233c9 100644 --- a/drivers/serial/uart_nrf5.c +++ b/drivers/serial/uart_nrf5.c @@ -398,7 +398,7 @@ static void uart_nrf5_irq_rx_disable(struct device *dev) } /** Interrupt driven transfer empty function */ -static int uart_nrf5_irq_tx_empty(struct device *dev) +static int uart_nrf5_irq_tx_complete(struct device *dev) { volatile struct _uart *uart = UART_STRUCT(dev); @@ -481,7 +481,7 @@ static const struct uart_driver_api uart_nrf5_driver_api = { .irq_tx_ready = uart_nrf5_irq_tx_ready, /** IRQ transfer ready function */ .irq_rx_enable = uart_nrf5_irq_rx_enable, /** IRQ receiver enabling function */ .irq_rx_disable = uart_nrf5_irq_rx_disable, /** IRQ receiver disabling function */ - .irq_tx_empty = uart_nrf5_irq_tx_empty, /** IRQ transfer empty function */ + .irq_tx_complete = uart_nrf5_irq_tx_complete, /** IRQ transfer complete function */ .irq_rx_ready = uart_nrf5_irq_rx_ready, /** IRQ receiver ready function */ .irq_err_enable = uart_nrf5_irq_err_enable, /** IRQ error enabling function */ .irq_err_disable = uart_nrf5_irq_err_disable, /** IRQ error disabling function */ diff --git a/drivers/serial/uart_ns16550.c b/drivers/serial/uart_ns16550.c index 66af0ea2658..ae43b307b53 100644 --- a/drivers/serial/uart_ns16550.c +++ b/drivers/serial/uart_ns16550.c @@ -508,7 +508,7 @@ static int uart_ns16550_irq_tx_ready(struct device *dev) * * @return 1 if nothing remains to be transmitted, 0 otherwise */ -static int uart_ns16550_irq_tx_empty(struct device *dev) +static int uart_ns16550_irq_tx_complete(struct device *dev) { return (INBYTE(LSR(dev)) & (LSR_TEMT | LSR_THRE)) == (LSR_TEMT | LSR_THRE); } @@ -721,7 +721,7 @@ static const struct uart_driver_api uart_ns16550_driver_api = { .irq_tx_enable = uart_ns16550_irq_tx_enable, .irq_tx_disable = uart_ns16550_irq_tx_disable, .irq_tx_ready = uart_ns16550_irq_tx_ready, - .irq_tx_empty = uart_ns16550_irq_tx_empty, + .irq_tx_complete = uart_ns16550_irq_tx_complete, .irq_rx_enable = uart_ns16550_irq_rx_enable, .irq_rx_disable = uart_ns16550_irq_rx_disable, .irq_rx_ready = uart_ns16550_irq_rx_ready, diff --git a/drivers/serial/uart_qmsi.c b/drivers/serial/uart_qmsi.c index 493a7228100..2da34f249c9 100644 --- a/drivers/serial/uart_qmsi.c +++ b/drivers/serial/uart_qmsi.c @@ -285,7 +285,7 @@ static int uart_qmsi_irq_tx_ready(struct device *dev) return id == QM_UART_IIR_THR_EMPTY; } -static int uart_qmsi_irq_tx_empty(struct device *dev) +static int uart_qmsi_irq_tx_complete(struct device *dev) { qm_uart_t instance = GET_CONTROLLER_INSTANCE(dev); const u32_t mask = (QM_UART_LSR_TEMT | QM_UART_LSR_THRE); @@ -433,7 +433,7 @@ static const struct uart_driver_api api = { .irq_tx_enable = uart_qmsi_irq_tx_enable, .irq_tx_disable = uart_qmsi_irq_tx_disable, .irq_tx_ready = uart_qmsi_irq_tx_ready, - .irq_tx_empty = uart_qmsi_irq_tx_empty, + .irq_tx_complete = uart_qmsi_irq_tx_complete, .irq_rx_enable = uart_qmsi_irq_rx_enable, .irq_rx_disable = uart_qmsi_irq_rx_disable, .irq_rx_ready = uart_qmsi_irq_rx_ready, diff --git a/drivers/serial/uart_stm32.c b/drivers/serial/uart_stm32.c index d7747008d43..78bb9469160 100644 --- a/drivers/serial/uart_stm32.c +++ b/drivers/serial/uart_stm32.c @@ -150,7 +150,7 @@ static int uart_stm32_irq_tx_ready(struct device *dev) return __HAL_UART_GET_FLAG(UartHandle, UART_FLAG_TXE); } -static int uart_stm32_irq_tx_empty(struct device *dev) +static int uart_stm32_irq_tx_complete(struct device *dev) { struct uart_stm32_data *data = DEV_DATA(dev); UART_HandleTypeDef *UartHandle = &data->huart; @@ -250,7 +250,7 @@ static const struct uart_driver_api uart_stm32_driver_api = { .irq_tx_enable = uart_stm32_irq_tx_enable, .irq_tx_disable = uart_stm32_irq_tx_disable, .irq_tx_ready = uart_stm32_irq_tx_ready, - .irq_tx_empty = uart_stm32_irq_tx_empty, + .irq_tx_complete = uart_stm32_irq_tx_complete, .irq_rx_enable = uart_stm32_irq_rx_enable, .irq_rx_disable = uart_stm32_irq_rx_disable, .irq_rx_ready = uart_stm32_irq_rx_ready, diff --git a/include/uart.h b/include/uart.h index 129941f19c7..4f246d75035 100644 --- a/include/uart.h +++ b/include/uart.h @@ -138,8 +138,8 @@ struct uart_driver_api { /** Interrupt driven receiver disabling function */ void (*irq_rx_disable)(struct device *dev); - /** Interrupt driven transfer empty function */ - int (*irq_tx_empty)(struct device *dev); + /** Interrupt driven transfer complete function */ + int (*irq_tx_complete)(struct device *dev); /** Interrupt driven receiver ready function */ int (*irq_rx_ready)(struct device *dev); @@ -312,11 +312,18 @@ static inline void uart_irq_tx_disable(struct device *dev) } /** - * @brief Check if Tx IRQ has been raised. + * @brief Check if UART TX buffer can accept a new char + * + * @details Check if UART TX buffer can accept at least one character + * for transmission (i.e. uart_fifo_fill() will succeed and return + * non-zero). This function must be called in a UART interrupt + * handler, or its result is undefined. Before calling this function + * in the interrupt handler, uart_irq_update() must be called once per + * the handler invocation. * * @param dev UART device structure. * - * @retval 1 If an IRQ is ready. + * @retval 1 If at least one char can be written to UART. * @retval 0 Otherwise. */ static inline int uart_irq_tx_ready(struct device *dev) @@ -363,30 +370,52 @@ static inline void uart_irq_rx_disable(struct device *dev) } /** - * @brief Check if nothing remains to be transmitted + * @brief Check if UART TX block finished transmission + * + * @details Check if any outgoing data buffered in UART TX block was + * fully transmitted and TX block is idle. When this condition is + * true, UART device (or whole system) can be power off. Note that + * this function is *not* useful to check if UART TX can accept more + * data, use uart_irq_tx_ready() for that. This function must be called + * in a UART interrupt handler, or its result is undefined. Before + * calling this function in the interrupt handler, uart_irq_update() + * must be called once per the handler invocation. * * @param dev UART device structure. * * @retval 1 If nothing remains to be transmitted. * @retval 0 Otherwise. */ -static inline int uart_irq_tx_empty(struct device *dev) +static inline int uart_irq_tx_complete(struct device *dev) { const struct uart_driver_api *api = dev->driver_api; - if (api->irq_tx_empty) { - return api->irq_tx_empty(dev); + if (api->irq_tx_complete) { + return api->irq_tx_complete(dev); } return 0; } /** - * @brief Check if Rx IRQ has been raised. + * @deprecated This API is deprecated. + */ +static inline int __deprecated uart_irq_tx_empty(struct device *dev) +{ + return uart_irq_tx_complete(dev); +} + +/** + * @brief Check if UART RX buffer has a new char + * + * (i.e. uart_fifo_read() will succeed and return non-zero). This function + * must be called in a UART interrupt handler, or its result is undefined. + * Before calling this function in the interrupt handler, uart_irq_update() + * must be called once per the handler invocation. * * @param dev UART device structure. * - * @retval 1 If an IRQ is ready. + * @retval 1 If a new received char is ready. * @retval 0 Otherwise. */ static inline int uart_irq_rx_ready(struct device *dev)