USB MSC and FreeRTOS interaction

[Randy Lee]

On an nRF52840 using SDK 15.3 and FreeRTOS 10

I've got both a PCA10056 dev kit and a custom board. I don't think I've got a hardware issue here but I'm running the usb_msc_pca10056 example on the dev kit and my own custom code on my custom board.  Hard to mix the two but I've got it mostly the same...  the example code is bare metal without an RTOS on it. I need to run RTOS on my code for other reasons and mostly because the USB portion is a small part of what I need to do.

What I've done is to take the example code and port it into my thread that runs the USB. In the end I want to run both CDC and MSC on SD cards but to simplify this what I've done is to run BOTH of these code sets with only msc and that only with the empty device on it.  I'm just checking for windows enumeration of the things.

On the example code on the dev kit, this works fine.  I get like one USB APP_USBD_EVT_DRV_RESET through usbd_user_ev_handler() and it enumerates immediately and stably on Win 10 showing the USB drive.

On my board (which works with CDC only just fine, BTW) and using substantially the same code for msc on it but in a FreeRTOS thread, this thing gets tons of APP_USBD_EVT_DRV_RESET (anywhere between 5 and constant) and enumerates on Win10 and then goes away and comes back and eventually comes back to a stable state.

Investigating this has something to do with the wait loop and I suspect time around that.

The example code is basically:

    while (true)
    {
        while (app_usbd_event_queue_process())
        {
            /* Nothing to do */
        }
        /* Sleep CPU only if there was no interrupt since last loop processing */
        __WFE();
    }

while my code is essentially:

    for (;;)
    {
        /* Waiting for event */
        while (app_usbd_event_queue_process())
        {
            /* Nothing to do */
        }
        ulTaskNotifyTake(pdTRUE, 1);
    }

So my question here is that this works nicely in CDC without issues.  What I'm guessing is that the timing requirements of msc are very much tighter and running this in a thread is way harder to do reliably.  Has anyone got some experience with running msc in FreeRTOS like this? Can it be done?

[Turbo J]

Randy Lee said:

  What I'm guessing is that the timing requirements of msc are very much tighter

Nope. MSC timings are not tight at all - you have several seconds to answer each request.

I tested that once by accident (MSC state machine deadlock - but that was NXP example code). If you get resets on windows check that you answer those MSC status requests in time.

[Kenneth]

I don't expect you should get usb reset due to slow handling of usb events no. You might try to run the project on the DK also if you haven't already, just to rule out possible electrical issues. It may be a be a better idea to start with an example that support both usbd + ble + freertos to start with, e.g. : \examples\peripheral\usbd_ble_uart_freertos

You might also find this thread useful: https://devzone.nordicsemi.com/f/nordic-q-a/75910/add-ble-scan-to-usbd_ble_uart_freertos-example 

Kenneth

[Randy Lee]

Oh that's a good idea to try a different example here... This thing is mainly a BLE enabled device so the USB on it is for smaller use cases. It runs CDC now well so I'm not thinking this is an electrical issue.

Yesterday I ran SystemView on it and noticed that the USB thread was getting starved some and taking like 150mS to get some time to run and adjusted priorities.  That dropped the resets dramatically.

I'll check out your links on Monday and see what I can find there.  Thanx!

[Randy Lee]

I'm having issues getting the composite to work here.  I can get one or the other devices to work reasonably but I can't get the two of them to run.  Code for the thread is here:

//
// The maximum delay inside the USB task to wait for an event.
//
//#define USB_THREAD_MAX_BLOCK_TIME portMAX_DELAY
#define USB_THREAD_MAX_BLOCK_TIME 1

//
// This thread implements the USB thread and associated stuff.  
// Not clear that we'll run with this in the end or what.  Implement both CDC and MSC nodes
//
// This section deals with the CDC portion.
//
static void cdc_acm_user_ev_handler(app_usbd_class_inst_t const * p_inst,
									app_usbd_cdc_acm_user_event_t event);

#define CDC_ACM_COMM_INTERFACE  0
#define CDC_ACM_COMM_EPIN		NRF_DRV_USBD_EPIN2

#define CDC_ACM_DATA_INTERFACE  1
#define CDC_ACM_DATA_EPIN		NRF_DRV_USBD_EPIN1
#define CDC_ACM_DATA_EPOUT		NRF_DRV_USBD_EPOUT1

static char m_cdc_data_array[BLE_NUS_MAX_DATA_LEN];

/** @brief CDC_ACM class instance. */
APP_USBD_CDC_ACM_GLOBAL_DEF(m_app_cdc_acm,
							cdc_acm_user_ev_handler,
							CDC_ACM_COMM_INTERFACE,
							CDC_ACM_DATA_INTERFACE,
							CDC_ACM_COMM_EPIN,
							CDC_ACM_DATA_EPIN,
							CDC_ACM_DATA_EPOUT,
							APP_USBD_CDC_COMM_PROTOCOL_AT_V250);


//
// This section deals with the setup for the MSC section
//

/**
 * @brief Empty block device definition
 */
NRF_BLOCK_DEV_EMPTY_DEFINE(
    m_block_dev_empty,
    NRF_BLOCK_DEV_EMPTY_CONFIG(512, 1024 * 1024),
    NFR_BLOCK_DEV_INFO_CONFIG("Nordic", "EMPTY", "1.00")
);

static void msc_user_ev_handler(app_usbd_class_inst_t const * p_inst,
                                app_usbd_msc_user_event_t     event);


/**
 * @brief Block devices list passed to @ref APP_USBD_MSC_GLOBAL_DEF
 */
#define BLOCKDEV_LIST() (                                   \
    NRF_BLOCKDEV_BASE_ADDR(m_block_dev_empty, block_dev),   \
)

/**
 * @brief Endpoint list passed to @ref APP_USBD_MSC_GLOBAL_DEF
 */
// ### Change the MSC endpoints in APP_USBD_MSC_ENDPOINT_LIST from 1 to 3 to avoid conflict with the CDC endpoints
#define ENDPOINT_LIST() APP_USBD_MSC_ENDPOINT_LIST(3, 3)

/**
 * @brief Mass storage class work buffer size (Needs be large for SD cards)
 */
#define MSC_WORKBUFFER_SIZE (2048)

/**
 * @brief Mass storage class instance
 */
APP_USBD_MSC_GLOBAL_DEF(m_app_msc,
                        0,
                        msc_user_ev_handler,
                        ENDPOINT_LIST(),
                        BLOCKDEV_LIST(),
                        MSC_WORKBUFFER_SIZE);






// USB DEFINES END

// USB CODE START
static bool m_usb_connected = false;

bool USB_IsConnected(void){
	return(m_usb_connected);
}

//
// Mark port as ready to use
//
static void setPortReady (void)
{
	// Using unused in SDK bit in line_state field for mark port as ready.
	// This thing need because of different behavior of host at serial port open.
	// Many apps at host side don't use any flags for flow control, so this library will not work with they.
	// For this case we're using hack with custom flag to catch port open event and set receive handler correctly.
	if (m_app_cdc_acm.specific.p_data) {
		m_app_cdc_acm.specific.p_data->ctx.line_state |= (1 << 4);
	}
}


//
// Checks receive handler and sets it up if needed.
//
static void checkRxHandler (void)
{
	if (!m_app_cdc_acm.specific.p_data) return;

	// Check setup of receive buffer when port is open (by custom bit, see setPortReady())
	// and mark port opened manually by DTR flag.
	if ((m_app_cdc_acm.specific.p_data->ctx.line_state & (1 << 4)) &&
		!m_app_cdc_acm.specific.p_data->ctx.rx_transfer[0].p_buf
	) {
		m_app_cdc_acm.specific.p_data->ctx.line_state |= APP_USBD_CDC_ACM_LINE_STATE_DTR;
		app_usbd_cdc_acm_read(&m_app_cdc_acm, m_cdc_data_array, 1);
	}
}


/** @brief User event handler @ref app_usbd_cdc_acm_user_ev_handler_t */
static void cdc_acm_user_ev_handler(app_usbd_class_inst_t const * p_inst,
									app_usbd_cdc_acm_user_event_t event)
{
	app_usbd_cdc_acm_t const * p_cdc_acm = app_usbd_cdc_acm_class_get(p_inst);

	switch (event)
	{
		case APP_USBD_CDC_ACM_USER_EVT_PORT_OPEN:
		{
			NRF_LOG_INFO("CDC ACM port opened");
			setPortReady();
			break;
		}

		case APP_USBD_CDC_ACM_USER_EVT_PORT_CLOSE:
			NRF_LOG_INFO("CDC ACM port closed");
			break;

		case APP_USBD_CDC_ACM_USER_EVT_TX_DONE:
			setPortReady();
			break;

		case APP_USBD_CDC_ACM_USER_EVT_RX_DONE:
		{
			size_t offset = app_usbd_cdc_acm_rx_size(&m_app_cdc_acm);
			size_t size = app_usbd_cdc_acm_bytes_stored(&m_app_cdc_acm);
			size += offset;

			while (size) {
				app_usbd_cdc_acm_read_any( &m_app_cdc_acm, &m_cdc_data_array[offset], sizeof(m_cdc_data_array) - offset );
				uint32_t blockSize = (size > sizeof(m_cdc_data_array)) ? sizeof(m_cdc_data_array) : size;
				DebugIn(m_cdc_data_array, blockSize, USB_write);
				size -= blockSize;
				offset = 0;
			}
		}
		break;

		default:
			NRF_LOG_INFO("USB: unknown event (%u)", event);
			break;
	}

}
//
// Same thing for the MSC stuff (just not doing anything there apparently)
//
static void msc_user_ev_handler(app_usbd_class_inst_t const * p_inst,
                                app_usbd_msc_user_event_t     event)
{
    UNUSED_PARAMETER(p_inst);
    UNUSED_PARAMETER(event);
}

//
// USBD library specific event handler
//
static void usbd_user_ev_handler(app_usbd_event_type_t event)
{
	switch (event)
	{
		case APP_USBD_EVT_DRV_SUSPEND:
			break;

		case APP_USBD_EVT_DRV_RESUME:
			break;

		case APP_USBD_EVT_STARTED:
			break;

		case APP_USBD_EVT_STOPPED:
//			fatfs_init();
			NRF_LOG_INFO("USB Stopped");
			app_usbd_disable();
			break;

		case APP_USBD_EVT_POWER_DETECTED:
			NRF_LOG_INFO("USB power detected");
			if (!nrf_drv_usbd_is_enabled())
			{
//                fatfs_uninit();
				app_usbd_enable();
			}
			break;

		case APP_USBD_EVT_POWER_REMOVED:
			NRF_LOG_INFO("USB power removed");
			m_usb_connected = false;
			app_usbd_stop();
			break;

		case APP_USBD_EVT_POWER_READY:
			NRF_LOG_INFO("USB ready");
			app_usbd_start();
			m_usb_connected = true;
			break;

		case APP_USBD_EVT_DRV_RESET:
			NRF_LOG_INFO("USB reset");
			break;

		default:
			break;
	}
}

void usb_new_event_isr_handler(app_usbd_internal_evt_t const * const p_event, bool queued)
{
	BaseType_t xHigherPriorityTaskWoken = pdFALSE;
	UNUSED_PARAMETER(p_event);
	UNUSED_PARAMETER(queued);
	ASSERT(m_usbd_thread != NULL);
	/* Release the semaphore */
	vTaskNotifyGiveFromISR(m_usbd_thread, &xHigherPriorityTaskWoken);
	portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
}

//
// Send data via USB.. comes from multiple threads so watch it.
//
void USB_write(void* data, uint32_t size)
{
	app_usbd_cdc_acm_write(&m_app_cdc_acm, data, size);
}


//
// Actual thread here
//
void usbd_thread(void * arg)
{
	ret_code_t ret;
	static const app_usbd_config_t usbd_config = {
		.ev_isr_handler = usb_new_event_isr_handler,
		.ev_state_proc  = usbd_user_ev_handler
	};
	UNUSED_PARAMETER(arg);

//	fatfs_init();		// this will have been done basically in the SD card task, so let's ignore it here (or we might have to pull it in here first)

	ret = app_usbd_init(&usbd_config);
	APP_ERROR_CHECK(ret);

	app_usbd_class_inst_t const * class_cdc_acm = app_usbd_cdc_acm_class_inst_get(&m_app_cdc_acm);
	ret = app_usbd_class_append(class_cdc_acm);
	APP_ERROR_CHECK(ret);

    app_usbd_class_inst_t const * class_inst_msc = app_usbd_msc_class_inst_get(&m_app_msc);
    ret = app_usbd_class_append(class_inst_msc);
    APP_ERROR_CHECK(ret);

	ret = app_usbd_power_events_enable();
	APP_ERROR_CHECK(ret);

	// Set the first event to make sure that USB queue is processed after it is started
	xTaskNotifyGive(m_usbd_thread);
	// Enter main loop.
	for (;;)
	{
		/* Waiting for event */
		while (app_usbd_event_queue_process())
		{
			/* Nothing to do */
		}
		checkRxHandler();
		ulTaskNotifyTake(pdTRUE, USB_THREAD_MAX_BLOCK_TIME);
	}
}

It's interesting... I change the MSC end point to (1,1) when I do the msc only and comment out the CDC portions and that runs.  commenting out the CDC portions, the CDC runs. But doing them both I get neither showing up on win10 machines.  Must be something simple I'm not doing there...

[Kenneth]

I can't find an example out of the box that does this right now, but I also expect it's something "simple". Someone posted some code that didn't work here, but seems the problem is unrelated to yours, so maybe you can use if for guidenance:
https://devzone.nordicsemi.com/f/nordic-q-a/62100/nrf52-usb-instability-with-usb-cdc-and-usb-msc-instantiated 

Kenneth