From 7e79623b394c615c12d1f9878391afde865f5dd3 Mon Sep 17 00:00:00 2001 From: Shuopeng Deng Date: Thu, 19 Dec 2019 15:35:36 -0800 Subject: [PATCH 1/2] Removed a hardcoded timeout in CyH4TransportDriver.cpp Replaced a hardcoded timeout in CyH4TransportDriver.cpp with a cypress hal function. The cypress PUTC hal API only blocks until data has been send into the HW buffer, not until all data has been out of the HW buffer. Modified an API to block untill all tx transmit is complete. This allows the removal of a hardcoded timeout in CyH4TransportDriver.cpp that waits for data int the HW buffer to be sent. --- .../CyH4TransportDriver.cpp | 65 ++++++++----------- .../COMPONENT_CYW43XXX/CyH4TransportDriver.h | 14 ++-- .../psoc6csp/hal/src/cyhal_uart.c | 2 +- 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.cpp b/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.cpp index 99d737db29..eef8935978 100644 --- a/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.cpp +++ b/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.cpp @@ -25,7 +25,7 @@ namespace cypress_ble { CyH4TransportDriver::CyH4TransportDriver(PinName tx, PinName rx, PinName cts, PinName rts, int baud, PinName bt_host_wake_name, PinName bt_device_wake_name, uint8_t host_wake_irq, uint8_t dev_wake_irq) : - uart(tx, rx, baud), cts(cts), rts(rts), + cts(cts), rts(rts), bt_host_wake_name(bt_host_wake_name), bt_device_wake_name(bt_device_wake_name), bt_host_wake(bt_host_wake_name, PIN_INPUT, PullNone, 0), @@ -33,12 +33,12 @@ CyH4TransportDriver::CyH4TransportDriver(PinName tx, PinName rx, PinName cts, Pi host_wake_irq_event(host_wake_irq), dev_wake_irq_event(dev_wake_irq) { + cyhal_uart_init(&uart, tx, rx, NULL, NULL); enabled_powersave = true; bt_host_wake_active = false; } CyH4TransportDriver::CyH4TransportDriver(PinName tx, PinName rx, PinName cts, PinName rts, int baud) : - uart(tx, rx, baud), cts(cts), rts(rts), bt_host_wake_name(NC), @@ -46,6 +46,7 @@ CyH4TransportDriver::CyH4TransportDriver(PinName tx, PinName rx, PinName cts, Pi bt_host_wake(bt_host_wake_name), bt_device_wake(bt_device_wake_name) { + cyhal_uart_init(&uart, tx, rx, NULL, NULL); enabled_powersave = false; bt_host_wake_active = false; sleep_manager_lock_deep_sleep(); @@ -93,6 +94,21 @@ void CyH4TransportDriver::bt_host_wake_fall_irq_handler(void) } } +static void on_controller_irq(void *callback_arg, cyhal_uart_event_t event) +{ + (void)(event); + cyhal_uart_t *uart_obj = (cyhal_uart_t *)callback_arg; + sleep_manager_lock_deep_sleep(); + + while (cyhal_uart_readable(uart_obj)) { + uint8_t char_received; + cyhal_uart_getc(uart_obj, &char_received, 0); + CyH4TransportDriver::on_data_received(&char_received, 1); + } + + sleep_manager_unlock_deep_sleep(); +} + void CyH4TransportDriver::initialize() { #if (defined(MBED_TICKLESS) && DEVICE_SLEEP && DEVICE_LPTICKER) @@ -101,24 +117,11 @@ void CyH4TransportDriver::initialize() sleep_manager_lock_deep_sleep(); - uart.format( - /* bits */ 8, - /* parity */ SerialBase::None, - /* stop bit */ 1 - ); - - uart.set_flow_control( - /* flow */ SerialBase::RTSCTS, - /* rts */ rts, - /* cts */ cts - ); - - uart.attach( - callback(this, &CyH4TransportDriver::on_controller_irq), - SerialBase::RxIrq - ); - - sleep_manager_unlock_deep_sleep(); + const cyhal_uart_cfg_t uart_cfg = { .data_bits = 8, .stop_bits = 1, .parity = CYHAL_UART_PARITY_NONE, .rx_buffer = NULL, .rx_buffer_size = 0 }; + cyhal_uart_configure(&uart, &uart_cfg); + cyhal_uart_set_flow_control(&uart, cts, rts); + cyhal_uart_register_callback(&uart, &on_controller_irq, &uart); + cyhal_uart_enable_event(&uart, CYHAL_UART_IRQ_RX_NOT_EMPTY, CYHAL_ISR_PRIORITY_DEFAULT, true); #if (defined(MBED_TICKLESS) && DEVICE_SLEEP && DEVICE_LPTICKER) if (bt_host_wake_name != NC) { @@ -150,28 +153,17 @@ uint16_t CyH4TransportDriver::write(uint8_t type, uint16_t len, uint8_t *pData) while (i < len + 1) { uint8_t to_write = i == 0 ? type : pData[i - 1]; - while (uart.writeable() == 0); - uart.putc(to_write); + while (cyhal_uart_writable(&uart) == 0); + cyhal_uart_putc(&uart, to_write); ++i; } + while(cyhal_uart_is_tx_active(&uart)); deassert_bt_dev_wake(); sleep_manager_unlock_deep_sleep(); return len; } -void CyH4TransportDriver::on_controller_irq() -{ - sleep_manager_lock_deep_sleep(); - - while (uart.readable()) { - uint8_t char_received = uart.getc(); - on_data_received(&char_received, 1); - } - - sleep_manager_unlock_deep_sleep(); -} - void CyH4TransportDriver::assert_bt_dev_wake() { #if (defined(MBED_TICKLESS) && DEVICE_SLEEP && DEVICE_LPTICKER) @@ -189,8 +181,6 @@ void CyH4TransportDriver::deassert_bt_dev_wake() { #if (defined(MBED_TICKLESS) && DEVICE_SLEEP && DEVICE_LPTICKER) if (enabled_powersave) { - wait_us(5000); /* remove and replace when uart tx transmit complete api is available */ - //De-assert bt_device_wake if (dev_wake_irq_event == WAKE_EVENT_ACTIVE_LOW) { bt_device_wake = WAKE_EVENT_ACTIVE_HIGH; } else { @@ -203,7 +193,8 @@ void CyH4TransportDriver::deassert_bt_dev_wake() void CyH4TransportDriver::update_uart_baud_rate(int baud) { - uart.baud(baud); + uint32_t ignore; + cyhal_uart_set_baud(&uart, (uint32_t)baud, &ignore); } bool CyH4TransportDriver::get_enabled_powersave() diff --git a/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h b/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h index 0cc27aa6e3..7b758e1b50 100644 --- a/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h +++ b/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h @@ -78,15 +78,17 @@ public: uint8_t get_dev_wake_irq_event(); private: - void on_controller_irq(); void assert_bt_dev_wake(); void deassert_bt_dev_wake(); - // Use RawSerial as opposed to Serial as we don't require the locking primitives - // provided by the Serial class (access to the UART should be exclusive to this driver) - // Furthermore, we access the peripheral in interrupt context which would clash - // with Serial's locking facilities - RawSerial uart; + // Use HAL serial because Cypress UART is buffered. + // The PUTC function does not actually blocks until data is fully transmitted, + // it only blocks until data gets into HW buffer. + // The UART APIs prevents sleep while there are data in the HW buffer. + // However UART APIs does not prevent the BT radio from going to sleep. + // Use the HAL APIs to prevent the radio from going to sleep until UART transmition is complete. + + cyhal_uart_t uart; PinName cts; PinName rts; PinName bt_host_wake_name; diff --git a/targets/TARGET_Cypress/TARGET_PSOC6/psoc6csp/hal/src/cyhal_uart.c b/targets/TARGET_Cypress/TARGET_PSOC6/psoc6csp/hal/src/cyhal_uart.c index 342d045978..fe5b2ea491 100644 --- a/targets/TARGET_Cypress/TARGET_PSOC6/psoc6csp/hal/src/cyhal_uart.c +++ b/targets/TARGET_Cypress/TARGET_PSOC6/psoc6csp/hal/src/cyhal_uart.c @@ -634,7 +634,7 @@ cy_rslt_t cyhal_uart_read_async(cyhal_uart_t *obj, void *rx, size_t length) bool cyhal_uart_is_tx_active(cyhal_uart_t *obj) { - return (0UL != (obj->context.txStatus & CY_SCB_UART_TRANSMIT_ACTIVE)); + return (0UL != (obj->context.txStatus & CY_SCB_UART_TRANSMIT_ACTIVE)) || !Cy_SCB_IsTxComplete(obj->base); } bool cyhal_uart_is_rx_active(cyhal_uart_t *obj) From 1d54f665b9cfdbfe5cb629cbd4ce4accf2e4830a Mon Sep 17 00:00:00 2001 From: Shuopeng Deng Date: Fri, 20 Dec 2019 09:16:59 -0800 Subject: [PATCH 2/2] fix code review ARs --- .../TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h | 1 + 1 file changed, 1 insertion(+) diff --git a/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h b/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h index 7b758e1b50..b5ed342c9b 100644 --- a/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h +++ b/features/FEATURE_BLE/targets/TARGET_Cypress/COMPONENT_CYW43XXX/CyH4TransportDriver.h @@ -87,6 +87,7 @@ private: // The UART APIs prevents sleep while there are data in the HW buffer. // However UART APIs does not prevent the BT radio from going to sleep. // Use the HAL APIs to prevent the radio from going to sleep until UART transmition is complete. + // Mbed layer has no API that distinguish between data in HW buffer v.s. data already transmitted. cyhal_uart_t uart; PinName cts;