Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[rtl872x] hal: fix issue that uart rx dma may hang up. #2502

Merged
merged 2 commits into from
Aug 15, 2022
Merged

[rtl872x] hal: fix issue that uart rx dma may hang up. #2502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
536c9f4
[rtl872x] hal: fix issue that uart rx dma may hang up.
XuGuohui Aug 11, 2022
8a552ca
[rtl872x] resolve review comments.
XuGuohui Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 50 additions & 21 deletions hal/src/rtl872x/usart_hal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,27 +81,39 @@ class Usart {
class RxLock {
public:
RxLock(Usart* instance)
: uart_(instance) {
: uart_(instance),
locked_(0) {
uart_->rxLock(true);
locked_++;
}
~RxLock() {
uart_->rxLock(false);
locked_--;
if (locked_ == 0) {
uart_->rxLock(false);
}
}
private:
Usart* uart_;
uint32_t locked_;
};

class TxLock {
public:
TxLock(Usart* instance)
: uart_(instance) {
: uart_(instance),
locked_(0) {
uart_->txLock(true);
locked_++;
}
~TxLock() {
uart_->txLock(false);
locked_--;
if (locked_ == 0) {
uart_->txLock(false);
}
}
private:
Usart* uart_;
uint32_t locked_;
};

bool isEnabled() const {
Expand Down Expand Up @@ -323,27 +335,25 @@ class Usart {
// Must be called under RX lock!
if (receiving_ && !useInterrupt()) {
auto uartInstance = uartTable_[index_].UARTx;
size_t transferredToDmaFromUart = uartInstance->RX_BYTE_CNT & RUART_RX_READ_BYTE_CNTER;
size_t dmaAvailableInBuffer = GDMA_GetDstAddr(rxDmaInitStruct_.GDMA_Index, rxDmaInitStruct_.GDMA_ChNum) - rxDmaInitStruct_.GDMA_DstAddr;
size_t blockSize = rxDmaInitStruct_.GDMA_BlockSize;
size_t alreadyCommitted = blockSize - rxBuffer_.acquirePending();
size_t transferredToDmaFromUart = uartInstance->RX_BYTE_CNT & RUART_RX_READ_BYTE_CNTER;
SPARK_ASSERT(transferredToDmaFromUart <= blockSize);
ssize_t toConsume = std::max<ssize_t>(dmaAvailableInBuffer, transferredToDmaFromUart) - alreadyCommitted;
SPARK_ASSERT(toConsume >= 0);
if (commit && toConsume > 0) {
if (transferredToDmaFromUart > dmaAvailableInBuffer) {
uintptr_t expectedDstAddr = rxDmaInitStruct_.GDMA_DstAddr + transferredToDmaFromUart;
if ((GDMA_BASE->CH[rxDmaInitStruct_.GDMA_ChNum].CFG_LOW & BIT_CFGX_LO_FIFO_EMPTY) == 0) {
if (GDMA_GetDstAddr(rxDmaInitStruct_.GDMA_Index, rxDmaInitStruct_.GDMA_ChNum) < expectedDstAddr) {
// Suspending DMA channel forces flushing of data into destination from GDMA FIFO
GDMA_BASE->CH[rxDmaInitStruct_.GDMA_ChNum].CFG_LOW |= BIT_CFGX_LO_CH_SUSP;
__DSB();
__ISB();
while (GDMA_GetDstAddr(rxDmaInitStruct_.GDMA_Index, rxDmaInitStruct_.GDMA_ChNum) < expectedDstAddr) {
// XXX: spin around, this should be pretty fast
}
GDMA_BASE->CH[rxDmaInitStruct_.GDMA_ChNum].CFG_LOW &= ~(BIT_CFGX_LO_CH_SUSP);
__DSB();
__ISB();
if (hal_interrupt_is_isr()) {
// This method is called from DMA RX ISR
toConsume = transferredToDmaFromUart - alreadyCommitted;
flushDmaRxFiFo(transferredToDmaFromUart);
} else {
toConsume = dmaAvailableInBuffer - alreadyCommitted;
// Try not suspending DMA unless necessary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if (toConsume <= 0) {
toConsume = transferredToDmaFromUart - dmaAvailableInBuffer;
if (toConsume > 0) {
flushDmaRxFiFo(transferredToDmaFromUart);
}
}
}
Expand Down Expand Up @@ -705,6 +715,24 @@ class Usart {
return true;
}

void flushDmaRxFiFo(size_t len) {
uintptr_t expectedDstAddr = rxDmaInitStruct_.GDMA_DstAddr + len;
if ((GDMA_BASE->CH[rxDmaInitStruct_.GDMA_ChNum].CFG_LOW & BIT_CFGX_LO_FIFO_EMPTY) == 0) {
if (GDMA_GetDstAddr(rxDmaInitStruct_.GDMA_Index, rxDmaInitStruct_.GDMA_ChNum) < expectedDstAddr) {
// Suspending DMA channel forces flushing of data into destination from GDMA FIFO
GDMA_BASE->CH[rxDmaInitStruct_.GDMA_ChNum].CFG_LOW |= BIT_CFGX_LO_CH_SUSP;
__DSB();
__ISB();
while (GDMA_GetDstAddr(rxDmaInitStruct_.GDMA_Index, rxDmaInitStruct_.GDMA_ChNum) < expectedDstAddr) {
// XXX: spin around, this should be pretty fast
}
GDMA_BASE->CH[rxDmaInitStruct_.GDMA_ChNum].CFG_LOW &= ~(BIT_CFGX_LO_CH_SUSP);
__DSB();
__ISB();
}
}
}

void deinitDmaChannels() {
auto uartInstance = uartTable_[index_].UARTx;
GDMA_Cmd(txDmaInitStruct_.GDMA_Index, txDmaInitStruct_.GDMA_ChNum, DISABLE);
Expand Down Expand Up @@ -843,13 +871,14 @@ class Usart {
return 0;
}
auto rxDmaInitStruct = &uart->rxDmaInitStruct_;
UART_RXDMACmd(uartTable_[uart->index_].UARTx, DISABLE);
// Operations to DMA before it is disabled
uart->dataInFlight(true /* commit */, true /* cancel */);
// Clean Auto Reload Bit
GDMA_ChCleanAutoReload(rxDmaInitStruct->GDMA_Index, rxDmaInitStruct->GDMA_ChNum, CLEAN_RELOAD_SRC);
GDMA_Cmd(rxDmaInitStruct->GDMA_Index, rxDmaInitStruct->GDMA_ChNum, DISABLE);
// Clear Pending ISR
GDMA_ClearINT(rxDmaInitStruct->GDMA_Index, rxDmaInitStruct->GDMA_ChNum);
UART_RXDMACmd(uartTable_[uart->index_].UARTx, DISABLE);
uart->dataInFlight(true /* commit */, true /* cancel */);
uart->receiving_ = false;
uart->startReceiver();
return 0;
Expand Down
4 changes: 3 additions & 1 deletion services/inc/ringbuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ inline ssize_t RingBuffer<T>::acquireCommit(size_t size, size_t cancel) {

headPending_ -= (size + cancel);
head_ = wrap(head_ + size, curSize_);
full_ = ((head_ == tail_) && size > 0);
if (size > 0) {
full_ = (head_ == tail_);
}

return (size);
}
Expand Down