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.

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

fix(PeriphDrivers): Fix UART RevB Driver not cleaning up Async Transactions #1334

Merged

Conversation

Brandon-Hurst
Copy link
Contributor

Description

Currently, the private AsyncTxRequests and AsyncRxRequests arrays are used to track transactions in progress for uart_revb.c. The Tx/Rx Async Callbacks check for a callback function before clearing the Requests:

mxc_uart_req_t *req = (mxc_uart_req_t *)AsyncTxRequests[uart_num];
if ((req != NULL) && (req->callback != NULL)) {
    AsyncTxRequests[uart_num] = NULL;
    req->callback(req, retVal);
}

This means that if the user does not supply an explicit callback function, a transaction never gets cleared, causing problems after just one transaction.

This commit resolves this by moving the cleanup of this important struct outside of the conditional check for a callback function.

mxc_uart_req_t *req = (mxc_uart_req_t *)AsyncTxRequests[uart_num];
if ((req != NULL) && (req->callback != NULL)) {
    req->callback(req, retVal);
}

// Cleanup Async Transaction
AsyncTxRequests[uart_num] = NULL;

Testing

This was tested as part of development for UART functionality in the CircuitPython BUSIO.UART module:
https://github.com/Brandon-Hurst/circuitpython/blob/ports/analog/add-busio/ports/analog/common-hal/busio/UART.c

I found that it was necessary to submit a callback function for the Async API to work correctly. This fix is to correct this, so the user doesn't have to supply a callback to have the Async Requests cleaned up.

Checklist Before Requesting Review

  • PR Title follows correct guidelines.
  • Description of changes and all other relevant information.
  • (Optional) Link any related GitHub issues using a keyword
  • (Optional) Provide info on any relevant functional testing/validation. For API changes or significant features, this is not optional.

Currently, the private AsyncTxRequests and AsyncRxRequests arrays are used to track transactions in progress for uart_revb.c.
The Tx/Rx Async Callbacks check for a callback function before clearing the Requests:

```C
mxc_uart_req_t *req = (mxc_uart_req_t *)AsyncTxRequests[uart_num];
if ((req != NULL) && (req->callback != NULL)) {
    AsyncTxRequests[uart_num] = NULL;
    req->callback(req, retVal);
}
```

This means that if the user does not supply an explicit callback function, a transaction never gets cleared, causing problems after just one transaction.

This commit resolves this by moving the cleanup of this important struct outside of the conditional check for a callback function.

```C
mxc_uart_req_t *req = (mxc_uart_req_t *)AsyncTxRequests[uart_num];
if ((req != NULL) && (req->callback != NULL)) {
    req->callback(req, retVal);
}

// Cleanup Async Transaction
AsyncTxRequests[uart_num] = NULL;
```

Signed-off-by: Brandon Hurst <[email protected]>
Copy link

github-actions bot commented Mar 6, 2025

This pull request is stale because it has been open for 30 days with no activity. Remove stale label, commit, or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale This issue or pull request is stale. label Mar 6, 2025
@Brandon-Hurst
Copy link
Contributor Author

This PR is going stale; commenting to refresh it. Please consider reviewing this change for acceptance when it is convenient.

@ttmut ttmut requested review from hfakkiz and OzgunKanal March 6, 2025 17:45
@github-actions github-actions bot removed the stale This issue or pull request is stale. label Mar 6, 2025
@sihyung-maxim sihyung-maxim merged commit 81969a3 into analogdevicesinc:main Mar 18, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants