USB device suspend/resume issue when requesting HFCLK (update)

[astella]

To the kind attention of Nordic support team,

Is there any update about this topic:

https://devzone.nordicsemi.com/f/nordic-q-a/59738/usb-device-suspend-resume-issue-when-requesting-hfclk?

I think I'm experiencing something like that. I'd like to avoid usb queue handling in freertos environment,

but when Windows OS is resuming from sleep mode, nRF52833 usb stack stays in SUSTATE_WAKINGUP_WAITING_HFCLK state,

without any chance to get back in SUSTATE_ACTIVE. I'm testing SDK16, I didn't already find something about that in SDK17 release notes.

Could you briefly update about that if not much trouble?

Thank you very much,

Best regards

[ovrebekk]

Hi Stella

I checked the internal ticket, but there doesn't seem to be a resolution unfortunately.  

I think I'm experiencing something like that. I'd like to avoid usb queue handling in freertos environment,

Why is the USB queue handling a problem if you are running freeRTOS?

Can't you just set up a separate thread to handle the USB events?

Best regards
Torbjørn

[astella]

Hi ovrebekk thank you very much for replaying,

as we try to explain in https://devzone.nordicsemi.com/f/nordic-q-a/79119/nrf52833-app_usbd_evt_power_read , using this simple test we see that there are moment when usb stack timing reaction is really important (during enumeration, for example, and this could be easily solved, doing enumeration before starting scheduler), but we are worried that our freertos configuration can cause random timing problems (especially runtime, should selective suspend/resume interactions be requested by the host).

freertos_task{

  for(){

    // ... if everything setup feed wdt 

   

     while (app_usbd_event_queue_process())
     {
         /* Nothing to do */
         
         // for(uint32_t i = 0; i < 3000; i++); // only for test introducing random delay // also vTaskDelay(1) /
         // taskYIELD(); // or test forcinf a context switch here
         // feed wdt here if test delay in usb queue processing
     }
  }

}

//---------------------------------------------------------------------------------------------------------------------------------

This is what we set in sdk config file:

#define APP_USBD_CONFIG_EVENT_QUEUE_ENABLE 0
#define APP_USBD_CONFIG_SOF_HANDLING_MODE 2

In order to solve the remote wakeup issue when using usb interrupt handling, we used these modifications in our project.

This seems to be ok for our tests until now, but maybe you can give a better opinion about that:

Modification #1
\sdk_16-0\components\libraries\usbd\class\hid\app_usbd_hid.h


static inline bool app_usbd_hid_trans_required(app_usbd_hid_ctx_t * p_hid_ctx)
{
  if (app_usbd_hid_state_flag_test(p_hid_ctx, APP_USBD_HID_STATE_FLAG_SUSPENDED) != 0)
  {
    if(bb == 1)
      aa = 1; // meaning: find out remote wake up going on, just don't signal it very first time
    UNUSED_RETURN_VALUE(app_usbd_wakeup_req());
    return false;
  }
  return app_usbd_hid_state_flag_test(p_hid_ctx, APP_USBD_HID_STATE_FLAG_TRANS_IN_PROGRESS) == 0;
}

Modification #2
\sdk_16-0\components\libraries\usbd\app_usbd.c

In app_usbd_event_execute:

a)

case APP_USBD_EVT_HFCLK_READY:
{
  if(aa == 1){  // meaning: if remote wake up going on, get started to change execution flow of state machine, as it would                         // be using queue approach 
    sustate_set(APP_USBD_EVT_DRV_RESUME);
    break;
  }

  switch(sustate_get())
  {
    case SUSTATE_RESUMING:
    {
      sustate_set(SUSTATE_ACTIVE);
      break;
    }
   case SUSTATE_WAKINGUP_WAITING_HFCLK_WREQ:
   {
     sustate_set(SUSTATE_WAKINGUP_WAITING_WREQ);
     break;
   }
   case SUSTATE_WAKINGUP_WAITING_HFCLK:
   {
     sustate_set(SUSTATE_ACTIVE);
     break;
   }
   default:
     break; // Just ignore - it can happen in specific situation
  }
break;
}



b)

case APP_USBD_EVT_DRV_WUREQ:
{
    static const app_usbd_evt_t evt_data = {
       .type = APP_USBD_EVT_DRV_RESUME
    };
    user_event_state_proc(APP_USBD_EVT_DRV_RESUME);
    /* Processing core interface (connected only to EP0) and then all instances from the list */
    UNUSED_RETURN_VALUE(app_usbd_core_handler_call((app_usbd_internal_evt_t const *)&evt_data));
    app_usbd_all_call((app_usbd_complex_evt_t const *)&evt_data);

    if(aa == 1){  // meaning: if remote wake up going on, get started to change execution flow of state machine, as it would                         // be using queue approach
        aa = 0;

        sustate_set(SUSTATE_ACTIVE);
        break;
     }

     switch(sustate_get())
    {
        case SUSTATE_WAKINGUP_WAITING_HFCLK_WREQ:
        sustate_set(SUSTATE_WAKINGUP_WAITING_HFCLK);
        break;
        case SUSTATE_WAKINGUP_WAITING_WREQ:
        sustate_set(SUSTATE_ACTIVE);
        break;
       default:
      {
          /* This should not happen - but try to recover by setting directly active state */
          NRF_LOG_WARNING("Unexpected state on WUREQ event (%u)", sustate_get());
          sustate_set(SUSTATE_ACTIVE);
      }
 }
break;
}

Best regards

[ovrebekk]

Hi Stella

Could you please attach the modified files to the case so I can more easily have a look at the changes? 

Then I can make a diff between the original and modified files, and ask the USB developer for some input. 

Best regards
Torbjørn

[astella]

Hi ovrebekk, thank you very much for your kindness.

these are modifications we are talking about. I'm just enclosing sdk16 files as they have been modified in our test custom project. Hope this suffice to get the idea. Shouldn't be enough or something is missing from your point of view, I could try to modify an existing SDK project as well.

Thank you

app_usbd_hid.h
app_usbd.c

[ovrebekk]

Hi Stella

Thanks for sharing the files. I have asked the developer for some input, and will try to get back to you early next week. 

Best regards
Torbjørn