Skip to content

Commit

Permalink
fix(cdc_acm): Fix Receive buffer append function on ESP32-P4
Browse files Browse the repository at this point in the history
The Receive buffer append function changes location of the data buffer.
While this works fine on S2 and S3, it will crash on P4 because now we must do
cache sync for the data buffer. This will crash on unaligned cache sync,
so this feature is disabled on P4 for now.
  • Loading branch information
tore-espressif committed Oct 3, 2024
1 parent ec640e9 commit 1ed2eb3
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
3 changes: 2 additions & 1 deletion host/class/cdc/usb_host_cdc_acm/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
## [Unreleased]
## 2.0.5

- Fixed CDC descriptor parsing logic, when some CDC devices could not be opened
- Added an option to open a CDC device with any VID/PID
- Fixed crash on ESP32-P4 if Receive buffer append function (introduced in v2.0.0) was used; this function does not work on P4 yet

## 2.0.4

Expand Down
10 changes: 9 additions & 1 deletion host/class/cdc/usb_host_cdc_acm/cdc_acm_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "freertos/event_groups.h"
#include "soc/soc_caps.h"
#include "esp_log.h"
#include "esp_check.h"
#include "esp_system.h"
Expand Down Expand Up @@ -172,7 +173,8 @@ static void cdc_acm_reset_in_transfer(cdc_dev_t *cdc_dev)
uint8_t **ptr = (uint8_t **)(&(transfer->data_buffer));
*ptr = cdc_dev->data.in_data_buffer_base;
transfer->num_bytes = transfer->data_buffer_size;
// This is a hotfix for IDF changes, where 'transfer->data_buffer_size' does not contain actual buffer length, but *allocated* buffer length, which can be larger
// This is a hotfix for IDF changes, where 'transfer->data_buffer_size' does not contain actual buffer length,
// but *allocated* buffer length, which can be larger if CONFIG_HEAP_POISONING_COMPREHENSIVE is enabled
transfer->num_bytes -= transfer->data_buffer_size % cdc_dev->data.in_mps;
}

Expand Down Expand Up @@ -805,6 +807,7 @@ static void in_xfer_cb(usb_transfer_t *transfer)
// In this case, the next received data must be appended to the existing buffer.
// Since the data_buffer in usb_transfer_t is a constant pointer, we must cast away to const qualifier.
if (!data_processed) {
#if !SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
// In case the received data was not processed, the next RX data must be appended to current buffer
uint8_t **ptr = (uint8_t **)(&(transfer->data_buffer));
*ptr += transfer->actual_num_bytes;
Expand All @@ -829,6 +832,11 @@ static void in_xfer_cb(usb_transfer_t *transfer)
cdc_acm_reset_in_transfer(cdc_dev);
cdc_dev->serial_state.bOverRun = false;
}
#else
// For targets that must sync internal memory through L1CACHE, we cannot change the data_buffer
// because it would lead to unaligned cache sync, which is not allowed
ESP_LOGW(TAG, "RX buffer append is not yet supported on ESP32-P4!");
#endif
} else {
cdc_acm_reset_in_transfer(cdc_dev);
}
Expand Down
2 changes: 1 addition & 1 deletion host/class/cdc/usb_host_cdc_acm/idf_component.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## IDF Component Manager Manifest File
version: "2.0.4"
version: "2.0.5"
description: USB Host CDC-ACM driver
tags:
- usb
Expand Down

0 comments on commit 1ed2eb3

Please sign in to comment.