USBMSD fixes

Make the following fixes:
-deinit in destructor to prevent race conditions
-cancel the reset task before calling it since it may be in progress
-wait for tasks to complete without mutex held
-prevent double connect with _init flag
feature-hal-spec-usb-device
Russ Butler 2018-06-18 11:35:22 -05:00
parent 3b7201cc13
commit af527b490c
2 changed files with 28 additions and 1 deletions

View File

@ -63,7 +63,7 @@ enum Status {
USBMSD::USBMSD(BlockDevice *bd, USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release) USBMSD::USBMSD(BlockDevice *bd, USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release)
: USBDevice(phy, vendor_id, product_id, product_release), : USBDevice(phy, vendor_id, product_id, product_release),
_in_task(&_queue), _out_task(&_queue), _reset_task(&_queue), _control_task(&_queue), _configure_task(&_queue) _init(false), _in_task(&_queue), _out_task(&_queue), _reset_task(&_queue), _control_task(&_queue), _configure_task(&_queue)
{ {
_bd = bd; _bd = bd;
_bd->init(); _bd->init();
@ -91,16 +91,26 @@ USBMSD::~USBMSD()
{ {
disconnect(); disconnect();
_bd->deinit(); _bd->deinit();
deinit();
} }
bool USBMSD::connect() bool USBMSD::connect()
{ {
_mutex_init.lock();
_mutex.lock(); _mutex.lock();
// already initialized
if (_init) {
_mutex.unlock();
_mutex_init.unlock();
return false;
}
//disk initialization //disk initialization
if (disk_status() & NO_INIT) { if (disk_status() & NO_INIT) {
if (disk_initialize()) { if (disk_initialize()) {
_mutex.unlock(); _mutex.unlock();
_mutex_init.unlock();
return false; return false;
} }
} }
@ -118,25 +128,31 @@ bool USBMSD::connect()
_page = (uint8_t *)malloc(_block_size * sizeof(uint8_t)); _page = (uint8_t *)malloc(_block_size * sizeof(uint8_t));
if (_page == NULL) { if (_page == NULL) {
_mutex.unlock(); _mutex.unlock();
_mutex_init.unlock();
return false; return false;
} }
} }
} else { } else {
_mutex.unlock(); _mutex.unlock();
_mutex_init.unlock();
return false; return false;
} }
//connect the device //connect the device
USBDevice::connect(); USBDevice::connect();
_init = true;
_mutex.unlock(); _mutex.unlock();
_mutex_init.unlock();
return true; return true;
} }
void USBMSD::disconnect() void USBMSD::disconnect()
{ {
_mutex_init.lock();
_mutex.lock(); _mutex.lock();
USBDevice::disconnect(); USBDevice::disconnect();
_init = false;
_in_task.cancel(); _in_task.cancel();
_out_task.cancel(); _out_task.cancel();
@ -144,17 +160,23 @@ void USBMSD::disconnect()
_control_task.cancel(); _control_task.cancel();
_configure_task.cancel(); _configure_task.cancel();
_mutex.unlock();
// object mutex must be unlocked for waiting
_in_task.wait(); _in_task.wait();
_out_task.wait(); _out_task.wait();
_reset_task.wait(); _reset_task.wait();
_control_task.wait(); _control_task.wait();
_configure_task.wait(); _configure_task.wait();
_mutex.lock();
//De-allocate MSD page size: //De-allocate MSD page size:
free(_page); free(_page);
_page = NULL; _page = NULL;
_mutex.unlock(); _mutex.unlock();
_mutex_init.unlock();
} }
bool USBMSD::ready() bool USBMSD::ready()
@ -231,6 +253,7 @@ void USBMSD::callback_state_change(DeviceState new_state)
// called in ISR context // called in ISR context
if (new_state != Configured) { if (new_state != Configured) {
_reset_task.cancel();
_reset_task.call(); _reset_task.call();
} }
} }

View File

@ -189,6 +189,9 @@ private:
uint8_t Status; uint8_t Status;
} CSW; } CSW;
// If this class has been initialized
bool _init;
//state of the bulk-only state machine //state of the bulk-only state machine
Stage _stage; Stage _stage;
@ -232,6 +235,7 @@ private:
Task<void()> _configure_task; Task<void()> _configure_task;
BlockDevice *_bd; BlockDevice *_bd;
rtos::Mutex _mutex_init;
rtos::Mutex _mutex; rtos::Mutex _mutex;
// space for config descriptor // space for config descriptor