Thoughts on unimplemented USB_STD_SET_FEATURE & USB_STD_CLEAR_FEATURE for Device?

[GrantMTG]

I'm not sure what needs to be added to usbd_core.c, or if there is another way, but I do need to use this for USB HID. This is the area under concern and some experiments I'm doing to check the commands as issued by USB.org's test program:

uint8_t MyRemoteWakeupEnable;

/** \brief Standard control request processing for device
 * \param dev pointer to usb device
 * \param req pointer to control request
 * \return TRUE if request is handled
 */
static usbd_respond usbd_process_devrq (usbd_device *dev, usbd_ctlreq *req) 
{
switch(req->bRequest)
   {
   case USB_STD_CLEAR_FEATURE:
        /* not yet supported */
        if(req->wValue == USB_FEAT_REMOTE_WKUP)
           {
           MyRemoteWakeupEnable = 0;
           return usbd_ack;
           }
        break;

   :

   case USB_STD_GET_STATUS:
        req->data[0] = 0;
        req->data[1] = 0;
        if(MyRemoteWakeupEnable)
           req->data[0] |= 0x02;    // #define REMOTE_WAKEUP_ENABLED 0x02
        return usbd_ack;

   :

   case USB_STD_SET_FEATURE:
        /* not yet supported */
        if(req->wValue == USB_FEAT_REMOTE_WKUP)
           {
           MyRemoteWakeupEnable = 1;
           return usbd_ack;
           }
        break;

    default:
        break;
    }

return usbd_fail;
}

If anyone has worked on this in a fork, please let me know.

GB

[GrantMTG]

For anyone following this, interested in USB suspend and resume, I have implemented in my application the two callbacks required. This turned out to be pretty straightforward.

// Prototypes:
static void USBD_SuspEventCb(usbd_device *dev, uint8_t event, uint8_t ep);
static void USBD_WkupEventCb(usbd_device *dev, uint8_t event, uint8_t ep);

// Registration:
usbd_reg_event(&udev, usbd_evt_susp, USBD_SuspEventCb);
usbd_reg_event(&udev, usbd_evt_wkup, USBD_WkupEventCb);

// Bare bones/stub implementation for now:
static void USBD_WkupEventCb(usbd_device *dev, uint8_t event, uint8_t ep)
{
 if(MyRemoteWakeupEnable)
    {
    MyUsbRemoteWakeup();
    }
}

static void USBD_SuspEventCb(usbd_device *dev, uint8_t event, uint8_t ep)
{
 if(MyRemoteWakeupEnable)
    {
    __NOP();
    }
}

[dmitrystu]

There is a standard way to add or overload any control request. It's a control request callback. In the case of usbd_ack or usbd_nak return code, this request will not be handled by core and the data from dev->status.data_ptr (points to req->data[0] by default) with length in dev->status.data_count (default is req->wLength) will be passed to the control endpoint.

[GrantMTG]

OK, thanks. I will see if I can figure that out.

FWIW this is the only device "feature" that I am aware of and some implementations (SILabs) just have an element in the USB instance.

// USB Device structure
typedef struct
{
  uint8_t configurationValue;
#if SLAB_USB_REMOTE_WAKEUP_ENABLED
  uint8_t remoteWakeupEnabled;      // <-----
#endif
  uint8_t numberOfStrings;
  USBD_State_TypeDef state;
  USBD_State_TypeDef savedState;
  USB_Setup_TypeDef setup;
  union
  {
    struct
    {
      uint8_t type : 7;
      uint8_t init : 1;
    } encoding;
    uint8_t c;
  } ep0String;
  USBD_Ep_TypeDef ep0;
#if SLAB_USB_EP1IN_USED
  USBD_Ep_TypeDef ep1in;
#endif
#if SLAB_USB_EP2IN_USED
  USBD_Ep_TypeDef ep2in;
#endif
#if SLAB_USB_EP3IN_USED
  USBD_Ep_TypeDef ep3in;
#endif
#if SLAB_USB_EP1OUT_USED
  USBD_Ep_TypeDef ep1out;
#endif
#if SLAB_USB_EP2OUT_USED
  USBD_Ep_TypeDef ep2out;
#endif
#if SLAB_USB_EP3OUT_USED
  USBD_Ep_TypeDef ep3out;
#endif
#if ((SLAB_USB_EP3IN_USED) && (SLAB_USB_EP3IN_TRANSFER_TYPE == USB_EPTYPE_ISOC))
  uint16_t ep3inIsoIdx;
#endif
#if ((SLAB_USB_EP3OUT_USED) && (SLAB_USB_EP3OUT_TRANSFER_TYPE == USB_EPTYPE_ISOC))
  uint16_t ep3outIsoIdx;
#endif
#if SLAB_USB_SUPPORT_ALT_INTERFACES
  uint8_t interfaceAltSetting[SLAB_USB_NUM_INTERFACES];
#endif
  SI_VARIABLE_SEGMENT_POINTER(deviceDescriptor, USB_DeviceDescriptor_TypeDef, SI_SEG_GENERIC);
  SI_VARIABLE_SEGMENT_POINTER(configDescriptor, USB_ConfigurationDescriptor_TypeDef, SI_SEG_GENERIC);
  SI_VARIABLE_SEGMENT_POINTER(stringDescriptors, USB_StringTable_TypeDef, SI_SEG_GENERIC);
} USBD_Device_TypeDef;

[dmitrystu]

Here is some code reference for you (switching Bus Powered / Self Powered and MSOS extcapability)

static usbd_respond process_control (usbd_device *dev, usbd_ctlreq *req, usbd_rqc_callback *cb) {
  switch (req->bmRequestType) {
  case USB_REQ_DEVTOHOST | USB_REQ_STANDARD | USB_REQ_DEVICE:
    if (req->bRequest == USB_STD_GET_STATUS) {
      req->data[0] = (is_ext_power()) ? 0x01 : 0x00;
      req->data[1] = 0x00;
      return usbd_ack;
    }
    break;
/* other requests removed by NDA reason */
  // MSOS handling  (bmRequestType 0xC0)
  case USB_REQ_DEVTOHOST | USB_REQ_VENDOR | USB_REQ_DEVICE:
    if (req->bRequest == USB_MS_GET_DESCRIPTOR &&
      req->wIndex == USB_MS_EXTCOMAPT_ID) {
      dev->status.data_ptr = (void*)&ms_compat_desc;
      dev->status.data_count = sizeof(ms_compat_desc);
      return usbd_ack;
    }
    break;
  default:
    break;
  }
  return usbd_fail;
}

[dmitrystu]

BTW there are a lot of things unimplemented in this stack. But this is the cost of the lightweight.

[GrantMTG]

Thanks! I am very happy with the stack so far. Nice and focused. It's just a bit of a learning curve for me.

I did a quick and dirty test based on your suggestions and was able to complete usb.org's USB 2.0 Command Verifier successfully:

//static usbd_respond cdc_control(usbd_device *dev, usbd_ctlreq *req, usbd_rqc_callback *callback)
usbd_respond USBD_CtlReqCb(usbd_device *dev, usbd_ctlreq *req, usbd_rqc_callback *callback)
{
//------------------------------------------------------------
// Set/Clear Feature(Device,RemoteWakeup), Get Status (Device)
//------------------------------------------------------------
if(req->bmRequestType == (USB_REQ_DEVICE|USB_REQ_STANDARD|USB_REQ_HOSTTODEV))
   {
   switch(req->bRequest)
      {
      case USB_STD_CLEAR_FEATURE:
           if(req->wValue == USB_FEAT_REMOTE_WKUP)
              {
              MyRemoteWakeupEnable = 0;
              return usbd_ack;
              }
           break;

      case USB_STD_SET_FEATURE:
           if(req->wValue == USB_FEAT_REMOTE_WKUP)
              {
              MyRemoteWakeupEnable = 1;
              return usbd_ack;
              }
           break;

      default:
           break;
      }
   }
else
if(req->bmRequestType == (USB_REQ_DEVICE|USB_REQ_STANDARD|USB_REQ_DEVTOHOST))
   {
   switch(req->bRequest)
      {
      case USB_STD_GET_STATUS:
           req->data[0] = 0;
           req->data[1] = 0;
           if(MyRemoteWakeupEnable)
              req->data[0] |= 0x02;    // #define REMOTE_WAKEUP_ENABLED 0x0002 // Standard request GET_STATUS bitmask.
           return usbd_ack;

      default:
           break;
      }
   }

   :
... rest of USBD_CtlReqCb() ...

There may be mistakes and/or things I need to clean up but I think I have the basics working. And I can restore usbd_core.c to original. Thanks for the continuing help.

[dmitrystu]

You're welcome. Perhaps I need to add some flowcharts to the readme. I'll close this issue. Please feel free to reopen it.