Skip to content

Commit

Permalink
fix(usb/msc): Fix driver events handling without background task
Browse files Browse the repository at this point in the history
  • Loading branch information
tore-espressif committed Sep 12, 2023
1 parent 5d8b0e2 commit 81f1fd3
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 32 deletions.
1 change: 1 addition & 0 deletions usb/usb_host_msc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- Fix `msc_host_get_device_info` for devices without Serial Number string descriptor https://github.com/espressif/esp-idf/issues/12163
- Fix regression from version 1.1.0 that files could not be opened in PSRAM https://github.com/espressif/idf-extra-components/issues/202
- Fix MSC driver event handling without background task

## 1.1.0 - yanked

Expand Down
15 changes: 11 additions & 4 deletions usb/usb_host_msc/include/usb/msc_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,15 @@ __attribute__((deprecated("use API from esp_private/msc_scsi_bot.h")));
/**
* @brief Handle MSC HOST events.
*
* @param[in] timeout_ms Timeout in miliseconds
* @return esp_err_t
* If MSC Host install was made with create_background_task=false configuration,
* application needs to handle USB Host events itself.
* Do not use if MSC host install was made with create_background_task=true configuration
*
* @param[in] timeout Timeout in FreeRTOS tick
* @return
* - ESP_OK: All events handled
* - ESP_ERR_TIMEOUT: No events handled within the timeout
* - ESP_FAIL: Event handling finished, driver uninstalled. You do not have to call this function further
*/
esp_err_t msc_host_handle_events(uint32_t timeout_ms);

Expand Down Expand Up @@ -171,8 +178,8 @@ esp_err_t msc_host_print_descriptors(msc_host_device_handle_t device);
*
* @param[in] device Handle of MSC device
* @return
* - ESP_OK: The device was recovered from reset
* - ESP_FAI: Recovery unsuccessful, might indicate broken device
* - ESP_OK: The device was recovered from reset
* - ESP_FAIL: Recovery unsuccessful, might indicate broken device
*/
esp_err_t msc_host_reset_recovery(msc_host_device_handle_t device);

Expand Down
49 changes: 32 additions & 17 deletions usb/usb_host_msc/src/msc_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ typedef struct {
void *user_arg;
SemaphoreHandle_t all_events_handled;
volatile bool end_client_event_handling;
bool event_handling_started;
STAILQ_HEAD(devices, msc_host_device) devices_tailq;
} msc_driver_t;

Expand Down Expand Up @@ -321,17 +322,34 @@ static bool is_mass_storage_device(uint8_t dev_addr)
return is_msc_device;
}

static void event_handler_task(void *arg)


esp_err_t msc_host_handle_events(uint32_t timeout)
{
while (1) {
usb_host_client_handle_events(s_msc_driver->client_handle, portMAX_DELAY);
MSC_RETURN_ON_FALSE(s_msc_driver != NULL, ESP_ERR_INVALID_STATE);

if (s_msc_driver->end_client_event_handling) {
break;
}
ESP_LOGV(TAG, "USB MSC handling");
s_msc_driver->event_handling_started = true;
esp_err_t ret = usb_host_client_handle_events(s_msc_driver->client_handle, timeout);
if (s_msc_driver->end_client_event_handling) {
xSemaphoreGive(s_msc_driver->all_events_handled);
return ESP_FAIL;
}
ESP_ERROR_CHECK( usb_host_client_deregister(s_msc_driver->client_handle) );
xSemaphoreGive(s_msc_driver->all_events_handled);
return ret;
}
/**
* @brief USB Client Event handler
*
* Handle all USB client events such as USB transfers and connections/disconnections
*
* @param[in] arg Argument, not used
*/
static void event_handler_task(void *arg)
{
ESP_LOGD(TAG, "USB HID handling start");
while (msc_host_handle_events(portMAX_DELAY) == ESP_OK) {
}
ESP_LOGD(TAG, "USB HID handling stop");
vTaskDelete(NULL);
}

Expand Down Expand Up @@ -445,9 +463,13 @@ esp_err_t msc_host_uninstall(void)
s_msc_driver->end_client_event_handling = true;
MSC_EXIT_CRITICAL();

ESP_ERROR_CHECK( usb_host_client_unblock(s_msc_driver->client_handle) );
xSemaphoreTake(s_msc_driver->all_events_handled, portMAX_DELAY);
if (s_msc_driver->event_handling_started) {
ESP_ERROR_CHECK( usb_host_client_unblock(s_msc_driver->client_handle) );
// In case the event handling started, we must wait until it finishes
xSemaphoreTake(s_msc_driver->all_events_handled, portMAX_DELAY);
}
vSemaphoreDelete(s_msc_driver->all_events_handled);
ESP_ERROR_CHECK( usb_host_client_deregister(s_msc_driver->client_handle) );
free(s_msc_driver);
s_msc_driver = NULL;
return ESP_OK;
Expand Down Expand Up @@ -515,13 +537,6 @@ esp_err_t msc_host_write_sector(msc_host_device_handle_t device, size_t sector,
return scsi_cmd_write10(dev, data, sector, 1, dev->disk.block_size);
}

esp_err_t msc_host_handle_events(uint32_t timeout_ms)
{
MSC_RETURN_ON_FALSE(s_msc_driver != NULL, ESP_ERR_INVALID_STATE);

return usb_host_client_handle_events(s_msc_driver->client_handle, pdMS_TO_TICKS(timeout_ms));
}

static void copy_string_desc(wchar_t *dest, const usb_str_desc_t *src)
{
if (dest == NULL) {
Expand Down
78 changes: 67 additions & 11 deletions usb/usb_host_msc/test/test_msc.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,22 @@ static void handle_usb_events(void *args)
vTaskDelete(NULL);
}

/**
* @brief MSC driver handling task
*
* This task is only used if the MSC driver was installed with no background task
*
* @param[in] args Not used
*/
static void msc_task(void *args)
{
ESP_LOGI(TAG, "USB MSC handling start");
while (msc_host_handle_events(portMAX_DELAY) == ESP_OK) {
}
ESP_LOGI(TAG, "USB MSC handling stop");
vTaskDelete(NULL);
}

#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)
static void check_file_content(const char *file_path, const char *expected)
{
Expand Down Expand Up @@ -166,7 +182,7 @@ static void check_sudden_disconnect(void)
fclose(file);
}

static void msc_setup(void)
static void msc_test_init(void)
{
BaseType_t task_created;

Expand All @@ -190,15 +206,10 @@ static void msc_setup(void)

task_created = xTaskCreatePinnedToCore(handle_usb_events, "usb_events", 2 * 2048, NULL, 2, NULL, 0);
TEST_ASSERT(task_created);
}

const msc_host_driver_config_t msc_config = {
.create_backround_task = true,
.callback = msc_event_cb,
.stack_size = 4096,
.task_priority = 5,
};
ESP_OK_ASSERT( msc_host_install(&msc_config) );

static void msc_test_wait_and_install_device(void)
{
ESP_LOGI(TAG, "Waiting for USB stick to be connected");
msc_host_event_t app_event;
xQueueReceive(app_queue, &app_event, portMAX_DELAY);
Expand All @@ -209,16 +220,32 @@ static void msc_setup(void)
ESP_OK_ASSERT( msc_host_vfs_register(device, "/usb", &mount_config, &vfs_handle) );
}

static void msc_teardown(void)
static void msc_setup(void)
{
vTaskDelay(10); // Wait to finish any ongoing USB operations
msc_test_init();
const msc_host_driver_config_t msc_config = {
.create_backround_task = true,
.callback = msc_event_cb,
.stack_size = 4096,
.task_priority = 5,
};
ESP_OK_ASSERT( msc_host_install(&msc_config) );
msc_test_wait_and_install_device();
}

static void msc_test_uninstall_device(void)
{
ESP_OK_ASSERT( msc_host_vfs_unregister(vfs_handle) );
ESP_OK_ASSERT( msc_host_uninstall_device(device) );
}

static void msc_test_deinit(void)
{
ESP_OK_ASSERT( msc_host_uninstall() );

xSemaphoreTake(ready_to_deinit_usb, portMAX_DELAY);
vSemaphoreDelete(ready_to_deinit_usb);
vTaskDelay(10); // Wait to finish any ongoing USB operations
ESP_OK_ASSERT( usb_host_uninstall() );
//Tear down USB PHY
ESP_OK_ASSERT(usb_del_phy(phy_hdl));
Expand All @@ -228,6 +255,12 @@ static void msc_teardown(void)
vTaskDelay(10); // Wait for FreeRTOS to clean up deleted tasks
}

static void msc_teardown(void)
{
msc_test_uninstall_device();
msc_test_deinit();
}

static void write_read_sectors(void)
{
uint8_t write_data[DISK_BLOCK_SIZE];
Expand Down Expand Up @@ -453,6 +486,29 @@ TEST_CASE("device_info", "[usb_msc]")
print_device_info(&info);
}

/**
* @brief USB MSC driver with no background task
*
* Install the driver without background task
* and make sure that everything works
*/
TEST_CASE("no_background_task", "[usb_msc]")
{
msc_test_init();
const msc_host_driver_config_t msc_config = {
.create_backround_task = false,
.callback = msc_event_cb,
.stack_size = 4096,
.task_priority = 5,
};
ESP_OK_ASSERT( msc_host_install(&msc_config) );
BaseType_t task_created = xTaskCreatePinnedToCore(msc_task, "msc_events", 2 * 2048, NULL, 2, NULL, 0);
TEST_ASSERT(task_created);
msc_test_wait_and_install_device();
write_read_sectors(); // Do some dummy operations
msc_teardown();
}

/**
* @brief USB MSC Device Mock
*
Expand Down

0 comments on commit 81f1fd3

Please sign in to comment.