USBH: MSD: Rework to prevent race conditions on unload

This commit is contained in:
Diego Ismirlian 2017-07-04 19:09:39 -03:00
parent d2c155b4cf
commit c900c951a3
3 changed files with 56 additions and 99 deletions

View File

@ -58,6 +58,9 @@ struct USBHMassStorageLUNDriver {
const struct USBHMassStorageDriverVMT *vmt; const struct USBHMassStorageDriverVMT *vmt;
_base_block_device_data _base_block_device_data
/* for serializing access to the LUN driver */
mutex_t mtx;
BlockDeviceInfo info; BlockDeviceInfo info;
USBHMassStorageDriver *msdp; USBHMassStorageDriver *msdp;
@ -68,11 +71,6 @@ struct USBHMassStorageDriver {
/* inherited from abstract class driver */ /* inherited from abstract class driver */
_usbh_base_classdriver_data _usbh_base_classdriver_data
/* for LUN request serialization, can be removed
* if the driver is configured to support only one LUN
* per USBHMassStorageDriver instance */
mutex_t mtx;
usbh_ep_t epin; usbh_ep_t epin;
usbh_ep_t epout; usbh_ep_t epout;
uint8_t ifnum; uint8_t ifnum;
@ -98,11 +96,7 @@ extern USBHMassStorageDriver USBHMSD[HAL_USBHMSD_MAX_INSTANCES];
#ifdef __cplusplus #ifdef __cplusplus
extern "C" { extern "C" {
#endif #endif
/* Mass Storage Driver */
void usbhmsdObjectInit(USBHMassStorageDriver *msdp);
/* Mass Storage LUN Driver (block driver) */ /* Mass Storage LUN Driver (block driver) */
void usbhmsdLUNObjectInit(USBHMassStorageLUNDriver *lunp);
void usbhmsdLUNStart(USBHMassStorageLUNDriver *lunp); void usbhmsdLUNStart(USBHMassStorageLUNDriver *lunp);
void usbhmsdLUNStop(USBHMassStorageLUNDriver *lunp); void usbhmsdLUNStop(USBHMassStorageLUNDriver *lunp);
bool usbhmsdLUNConnect(USBHMassStorageLUNDriver *lunp); bool usbhmsdLUNConnect(USBHMassStorageLUNDriver *lunp);

View File

@ -62,9 +62,7 @@
#define uerr(f, ...) do {} while(0) #define uerr(f, ...) do {} while(0)
#endif #endif
static void usbhmsdLUNObjectDeinit(USBHMassStorageLUNDriver *lunp);
/*===========================================================================*/ /*===========================================================================*/
/* USB Class driver loader for MSD */ /* USB Class driver loader for MSD */
@ -187,15 +185,7 @@ alloc_ok:
MSBLKD[i].next = msdp->luns; MSBLKD[i].next = msdp->luns;
msdp->luns = &MSBLKD[i]; msdp->luns = &MSBLKD[i];
MSBLKD[i].msdp = msdp; MSBLKD[i].msdp = msdp;
osalSysLock();
MSBLKD[i].state = BLK_ACTIVE; /* transition directly to active, instead of BLK_STOP */ MSBLKD[i].state = BLK_ACTIVE; /* transition directly to active, instead of BLK_STOP */
osalSysUnlock();
/* connect the LUN (TODO: review if it's best to leave the LUN disconnected) */
msdp->dev = dev;
usbhmsdLUNConnect(&MSBLKD[i]);
msdp->dev = NULL;
luns--; luns--;
} }
} }
@ -212,25 +202,15 @@ static void _msd_unload(usbh_baseclassdriver_t *drv) {
USBHMassStorageDriver *const msdp = (USBHMassStorageDriver *)drv; USBHMassStorageDriver *const msdp = (USBHMassStorageDriver *)drv;
USBHMassStorageLUNDriver *lunp = msdp->luns; USBHMassStorageLUNDriver *lunp = msdp->luns;
osalMutexLock(&msdp->mtx); /* disconnect all LUNs */
osalSysLock();
usbhEPCloseS(&msdp->epin);
usbhEPCloseS(&msdp->epout);
while (lunp) { while (lunp) {
lunp->state = BLK_STOP; usbhmsdLUNDisconnect(lunp);
usbhmsdLUNObjectDeinit(lunp);
lunp = lunp->next; lunp = lunp->next;
} }
osalSysUnlock();
osalMutexUnlock(&msdp->mtx);
/* now that the LUNs are idle, deinit them */ usbhEPClose(&msdp->epin);
lunp = msdp->luns; usbhEPClose(&msdp->epout);
osalSysLock();
while (lunp) {
usbhmsdLUNObjectInit(lunp);
lunp = lunp->next;
}
osalSysUnlock();
} }
@ -238,8 +218,6 @@ static void _msd_unload(usbh_baseclassdriver_t *drv) {
/* MSD Class driver operations (Bulk-Only transport) */ /* MSD Class driver operations (Bulk-Only transport) */
/*===========================================================================*/ /*===========================================================================*/
/* USB Bulk Only Transport SCSI Command block wrapper */ /* USB Bulk Only Transport SCSI Command block wrapper */
typedef PACKED_STRUCT { typedef PACKED_STRUCT {
uint32_t dCBWSignature; uint32_t dCBWSignature;
@ -704,11 +682,23 @@ static const struct USBHMassStorageDriverVMT blk_vmt = {
(bool (*)(void *, BlockDeviceInfo *))usbhmsdLUNGetInfo (bool (*)(void *, BlockDeviceInfo *))usbhmsdLUNGetInfo
}; };
void usbhmsdLUNObjectInit(USBHMassStorageLUNDriver *lunp) { static void usbhmsdLUNObjectDeinit(USBHMassStorageLUNDriver *lunp) {
osalDbgCheck(lunp != NULL);
osalMutexLock(&lunp->mtx);
lunp->msdp = NULL;
lunp->next = NULL;
memset(&lunp->info, 0, sizeof(lunp->info));
lunp->state = BLK_STOP;
osalMutexUnlock(&lunp->mtx);
}
static void usbhmsdLUNObjectInit(USBHMassStorageLUNDriver *lunp) {
osalDbgCheck(lunp != NULL); osalDbgCheck(lunp != NULL);
memset(lunp, 0, sizeof(*lunp)); memset(lunp, 0, sizeof(*lunp));
lunp->vmt = &blk_vmt; lunp->vmt = &blk_vmt;
lunp->state = BLK_STOP; lunp->state = BLK_STOP;
osalMutexObjectInit(&lunp->mtx);
/* Unnecessary because of the memset: /* Unnecessary because of the memset:
lunp->msdp = NULL; lunp->msdp = NULL;
lunp->next = NULL; lunp->next = NULL;
@ -737,24 +727,19 @@ void usbhmsdLUNStop(USBHMassStorageLUNDriver *lunp) {
} }
bool usbhmsdLUNConnect(USBHMassStorageLUNDriver *lunp) { bool usbhmsdLUNConnect(USBHMassStorageLUNDriver *lunp) {
USBHMassStorageDriver *const msdp = lunp->msdp; osalDbgCheck(lunp != NULL);
osalDbgCheck(lunp->msdp != NULL);
msd_result_t res; msd_result_t res;
osalDbgCheck(msdp != NULL); osalMutexLock(&lunp->mtx);
osalSysLock();
//osalDbgAssert((lunp->state == BLK_ACTIVE) || (lunp->state == BLK_READY),
// "invalid state");
if (lunp->state == BLK_READY) { if (lunp->state == BLK_READY) {
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return HAL_SUCCESS; return HAL_SUCCESS;
} else if (lunp->state != BLK_ACTIVE) { } else if (lunp->state != BLK_ACTIVE) {
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return HAL_FAILED; return HAL_FAILED;
} }
lunp->state = BLK_CONNECTING; lunp->state = BLK_CONNECTING;
osalSysUnlock();
osalMutexLock(&msdp->mtx);
{ {
USBH_DEFINE_BUFFER(scsi_inquiry_response_t inq); USBH_DEFINE_BUFFER(scsi_inquiry_response_t inq);
@ -821,40 +806,36 @@ bool usbhmsdLUNConnect(USBHMassStorageLUNDriver *lunp) {
uinfo("MSD Connected."); uinfo("MSD Connected.");
osalMutexUnlock(&msdp->mtx);
osalSysLock();
lunp->state = BLK_READY; lunp->state = BLK_READY;
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return HAL_SUCCESS; return HAL_SUCCESS;
/* Connection failed, state reset to BLK_ACTIVE.*/ /* Connection failed, state reset to BLK_ACTIVE.*/
failed: failed:
osalMutexUnlock(&msdp->mtx);
osalSysLock();
lunp->state = BLK_ACTIVE; lunp->state = BLK_ACTIVE;
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return HAL_FAILED; return HAL_FAILED;
} }
bool usbhmsdLUNDisconnect(USBHMassStorageLUNDriver *lunp) { bool usbhmsdLUNDisconnect(USBHMassStorageLUNDriver *lunp) {
osalDbgCheck(lunp != NULL); osalDbgCheck(lunp != NULL);
osalSysLock();
osalMutexLock(&lunp->mtx);
osalDbgAssert((lunp->state == BLK_ACTIVE) || (lunp->state == BLK_READY), osalDbgAssert((lunp->state == BLK_ACTIVE) || (lunp->state == BLK_READY),
"invalid state"); "invalid state");
if (lunp->state == BLK_ACTIVE) { if (lunp->state == BLK_ACTIVE) {
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return HAL_SUCCESS; return HAL_SUCCESS;
} }
lunp->state = BLK_DISCONNECTING; lunp->state = BLK_DISCONNECTING;
osalSysUnlock();
//TODO: complete //TODO: complete
osalSysLock();
lunp->state = BLK_ACTIVE; lunp->state = BLK_ACTIVE;
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return HAL_SUCCESS; return HAL_SUCCESS;
} }
@ -867,15 +848,13 @@ bool usbhmsdLUNRead(USBHMassStorageLUNDriver *lunp, uint32_t startblk,
msd_result_t res; msd_result_t res;
uint32_t actual_len; uint32_t actual_len;
osalSysLock(); osalMutexLock(&lunp->mtx);
if (lunp->state != BLK_READY) { if (lunp->state != BLK_READY) {
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return ret; return ret;
} }
lunp->state = BLK_READING; lunp->state = BLK_READING;
osalSysUnlock();
osalMutexLock(&lunp->msdp->mtx);
while (n) { while (n) {
if (n > 0xffff) { if (n > 0xffff) {
blocks = 0xffff; blocks = 0xffff;
@ -900,15 +879,8 @@ bool usbhmsdLUNRead(USBHMassStorageLUNDriver *lunp, uint32_t startblk,
ret = HAL_SUCCESS; ret = HAL_SUCCESS;
exit: exit:
osalMutexUnlock(&lunp->msdp->mtx);
osalSysLock();
if (lunp->state == BLK_READING) {
lunp->state = BLK_READY; lunp->state = BLK_READY;
} else { osalMutexUnlock(&lunp->mtx);
osalDbgCheck(lunp->state == BLK_STOP);
uwarn("MSD: State = BLK_STOP");
}
osalSysUnlock();
return ret; return ret;
} }
@ -921,15 +893,13 @@ bool usbhmsdLUNWrite(USBHMassStorageLUNDriver *lunp, uint32_t startblk,
msd_result_t res; msd_result_t res;
uint32_t actual_len; uint32_t actual_len;
osalSysLock(); osalMutexLock(&lunp->mtx);
if (lunp->state != BLK_READY) { if (lunp->state != BLK_READY) {
osalSysUnlock(); osalMutexUnlock(&lunp->mtx);
return ret; return ret;
} }
lunp->state = BLK_WRITING; lunp->state = BLK_WRITING;
osalSysUnlock();
osalMutexLock(&lunp->msdp->mtx);
while (n) { while (n) {
if (n > 0xffff) { if (n > 0xffff) {
blocks = 0xffff; blocks = 0xffff;
@ -954,15 +924,8 @@ bool usbhmsdLUNWrite(USBHMassStorageLUNDriver *lunp, uint32_t startblk,
ret = HAL_SUCCESS; ret = HAL_SUCCESS;
exit: exit:
osalMutexUnlock(&lunp->msdp->mtx);
osalSysLock();
if (lunp->state == BLK_WRITING) {
lunp->state = BLK_READY; lunp->state = BLK_READY;
} else { osalMutexUnlock(&lunp->mtx);
osalDbgCheck(lunp->state == BLK_STOP);
uwarn("MSD: State = BLK_STOP");
}
osalSysUnlock();
return ret; return ret;
} }
@ -982,23 +945,19 @@ bool usbhmsdLUNGetInfo(USBHMassStorageLUNDriver *lunp, BlockDeviceInfo *bdip) {
bool usbhmsdLUNIsInserted(USBHMassStorageLUNDriver *lunp) { bool usbhmsdLUNIsInserted(USBHMassStorageLUNDriver *lunp) {
osalDbgCheck(lunp != NULL); osalDbgCheck(lunp != NULL);
blkstate_t state; return (lunp->state >= BLK_ACTIVE);
osalSysLock();
state = lunp->state;
osalSysUnlock();
return (state >= BLK_ACTIVE);
} }
bool usbhmsdLUNIsProtected(USBHMassStorageLUNDriver *lunp) { bool usbhmsdLUNIsProtected(USBHMassStorageLUNDriver *lunp) {
osalDbgCheck(lunp != NULL); osalDbgCheck(lunp != NULL);
//TODO: Implement
return FALSE; return FALSE;
} }
void usbhmsdObjectInit(USBHMassStorageDriver *msdp) { static void usbhmsdObjectInit(USBHMassStorageDriver *msdp) {
osalDbgCheck(msdp != NULL); osalDbgCheck(msdp != NULL);
memset(msdp, 0, sizeof(*msdp)); memset(msdp, 0, sizeof(*msdp));
msdp->info = &usbhmsdClassDriverInfo; msdp->info = &usbhmsdClassDriverInfo;
osalMutexObjectInit(&msdp->mtx);
} }
void usbhmsdInit(void) { void usbhmsdInit(void) {

View File

@ -19,7 +19,7 @@
#include "ff.h" #include "ff.h"
#include <string.h> #include <string.h>
#define UVC_TO_MSD_PHOTOS_CAPTURE TRUE #define UVC_TO_MSD_PHOTOS_CAPTURE FALSE
#if HAL_USBH_USE_FTDI || HAL_USBH_USE_AOA #if HAL_USBH_USE_FTDI || HAL_USBH_USE_AOA
@ -340,7 +340,7 @@ static void ThreadTestMSD(void *p) {
FATFS *fsp; FATFS *fsp;
DWORD clusters; DWORD clusters;
FRESULT res; FRESULT res;
blkstate_t state;
#if !UVC_TO_MSD_PHOTOS_CAPTURE #if !UVC_TO_MSD_PHOTOS_CAPTURE
BaseSequentialStream * const chp = (BaseSequentialStream *)&USBH_DEBUG_SD; BaseSequentialStream * const chp = (BaseSequentialStream *)&USBH_DEBUG_SD;
systime_t st, et; systime_t st, et;
@ -351,11 +351,15 @@ start:
for(;;) { for(;;) {
chThdSleepMilliseconds(100); chThdSleepMilliseconds(100);
chSysLock(); if (blkGetDriverState(&MSBLKD[0]) == BLK_ACTIVE) {
state = blkGetDriverState(&MSBLKD[0]); usbDbgPuts("BLK: Active, connect....");
chSysUnlock(); usbhmsdLUNConnect(&MSBLKD[0]);
if (state != BLK_READY) }
if (blkGetDriverState(&MSBLKD[0]) != BLK_READY) {
continue; continue;
}
usbDbgPuts("BLK: Ready.");
#if !UVC_TO_MSD_PHOTOS_CAPTURE #if !UVC_TO_MSD_PHOTOS_CAPTURE
//raw read test //raw read test