mengxp

USBD_API->hw->ISR bug when handle completion interrupts

Discussion created by mengxp on Jun 23, 2016
Latest reply on Sep 15, 2017 by stdout


I create a Mass Storage project

Sometimes after  WriteEP CSW in EP1_handler, Program will not get a USB_EVT_OUT

It will cause my Bulk State Machine stuck at MSC_BS_CSW

 

So I search the forum and I found a guy who post a thread about this

USBCDC sample code of LPC4330 issue. | www.LPCware.com

 

He pointed the bug occurs at clear ENDPTCOMPLETE bit

So I did a reverse engineering ..

 

void __fastcall usbd_hw_ISR(USB_CORE_CTRL *arg_pUsb)

{

  USB_CORE_CTRL *_pUsb; // r4@1

  USB_HW_DATA *p_hw_data; // r5@1

  int v3; // r0@1

  LPC_USBHS *pUsbReg; // r1@1

  int disr; // r2@1

  void (__fastcall *v6)(USB_CORE_CTRL *); // r1@2

  void (__fastcall *v7)(USB_CORE_CTRL *); // r1@6

  void (__fastcall *v8)(USB_CORE_CTRL *); // r1@11

  void (__fastcall *v9)(USB_CORE_CTRL *, int, signed int); // r3@14

  LPC_USBHS *v10; // r0@16

  int val; // r1@16

  int EpIndex; // r7@16

  unsigned int i; // r6@17

  int v14; // r0@17

  int v15; // r0@19

  void (__fastcall *v16)(USB_CORE_CTRL *, _DWORD, signed int); // r3@19

  int v17; // r0@23

  void (__fastcall *v18)(USB_CORE_CTRL *, _DWORD, signed int); // r3@23

  int v19; // r7@29

  unsigned int v20; // r6@30

  int v21; // r0@30

  int v22; // r0@32

  void (__fastcall *v23)(USB_CORE_CTRL *, _DWORD, signed int); // r3@32

  int v24; // r0@35

  void (__fastcall *v25)(USB_CORE_CTRL *, _DWORD, signed int); // r3@35

  void (__fastcall *v26)(USB_CORE_CTRL *); // r1@41

  void (__fastcall *v27)(USB_CORE_CTRL *, int); // r2@45

  int _val; // [sp+0h] [bp-28h]@16

  int v29; // [sp+0h] [bp-28h]@29

  int _disr; // [sp+4h] [bp-24h]@1

  int v31; // [sp+Ch] [bp-1Ch]@22

  USB_CORE_CTRL *pUsb; // [sp+10h] [bp-18h]@1

 

 

  pUsb = arg_pUsb;

  _pUsb = arg_pUsb;

  p_hw_data = (USB_HW_DATA *)arg_pUsb->p_hw_data;

  v3 = p_hw_data->pUsbRegBase->USBSTS;

  p_hw_data->pUsbRegBase->USBSTS = v3;

  pUsbReg = p_hw_data->pUsbRegBase;

  disr = pUsbReg->USBINTR & v3;

  _disr = disr;

  if ( disr & 0x40 )

  {

    usbd_hw_Reset(pUsb);

    UsbReset(_pUsb);

    v6 = (void (__fastcall *)(USB_CORE_CTRL *))_pUsb->USB_Reset_Event;

    if ( v6 )

      v6(pUsb);

    JUMPOUT(&loc_10401D0E);

  }

  if ( disr & 0x100 )

  {

    pUsbReg->OTGSC = 2 * ((unsigned int)pUsbReg->OTGSC >> 1);

    v7 = (void (__fastcall *)(USB_CORE_CTRL *))_pUsb->USB_Suspend_Event;

    if ( v7 )

      v7(pUsb);

  }

  if ( _disr & 4 )

  {

    if ( p_hw_data->pUsbRegBase->PORTSC1 & 0x200 )

      _pUsb->device_speed = 1;

    p_hw_data->pUsbRegBase->OTGSC |= 1u;

    v8 = (void (__fastcall *)(USB_CORE_CTRL *))_pUsb->USB_Resume_Event;

    if ( v8 )

      v8(pUsb);

  }

  if ( p_hw_data->pUsbRegBase->ENDPTSETUPSTAT )

  {

    p_hw_data->pUsbRegBase->ENDPTCOMPLETE = 0x10001;

    p_hw_data->pUsbRegBase->ENDPTNAKEN |= 0x10001u;

    v9 = (void (__fastcall *)(USB_CORE_CTRL *, int, signed int))_pUsb->ep_event_hdlr[0];

    if ( v9 )

      v9(_pUsb, _pUsb->ep_hdlr_data[0], 1);

  }

  v10 = p_hw_data->pUsbRegBase;

  val = v10->ENDPTCOMPLETE;

  EpIndex = 0;

  _val = val;

  if ( val )

  {

    v10->ENDPTNAK = val;

    i = 0;

    v14 = (int)&_pUsb->ep0_cb_data[2];

    while ( *(_BYTE *)(v14 + 25) > i )          // max_num_ep > v13

    {

      if ( (1 << i) & _val )

      {

        v15 = (int)(&_pUsb->USB_EvtSetupHandler + EpIndex);

        v16 = *(void (__fastcall **)(USB_CORE_CTRL *, _DWORD, signed int))(v15 + 88);

        if ( v16 )

          v16(_pUsb, *(_DWORD *)(v15 + 136), 2);

        p_hw_data->pUsbRegBase->ENDPTCOMPLETE = 1 << i;

      }

      v31 = 1 << (i + 16);

      if ( v31 & _val )

      {

        *(_DWORD *)(p_hw_data->ep_TD + (i << 6) + 36) &= 192u;

        v17 = (int)(&_pUsb->USB_EvtSetupHandler + EpIndex);

        v18 = *(void (__fastcall **)(USB_CORE_CTRL *, _DWORD, signed int))(v17 + 92);

        if ( v18 )

          v18(_pUsb, *(_DWORD *)(v17 + 140), 3);

        p_hw_data->pUsbRegBase->ENDPTCOMPLETE = v31;

      }

      EpIndex += 2;

      v14 = (int)&_pUsb->ep0_cb_data[2];

      ++i;

    }

  }

  if ( _disr & 0x10000 )

  {

    v19 = 0;

    v29 = p_hw_data->pUsbRegBase->ENDPTNAKEN & p_hw_data->pUsbRegBase->ENDPTNAK;

    if ( v29 )

    {

      v20 = 0;

      v21 = (int)&_pUsb->ep0_cb_data[2];

      while ( *(_BYTE *)(v21 + 25) > v20 )

      {

        if ( (1 << v20) & v29 )

        {

          v22 = (int)(&_pUsb->USB_EvtSetupHandler + v19);

          v23 = *(void (__fastcall **)(USB_CORE_CTRL *, _DWORD, signed int))(v22 + 88);

          if ( v23 )

            v23(_pUsb, *(_DWORD *)(v22 + 136), 4);

        }

        if ( (1 << (v20 + 16)) & v29 )

        {

          v24 = (int)(&_pUsb->USB_EvtSetupHandler + v19);

          v25 = *(void (__fastcall **)(USB_CORE_CTRL *, _DWORD, signed int))(v24 + 92);

          if ( v25 )

            v25(_pUsb, *(_DWORD *)(v24 + 140), 5);

        }

        v19 += 2;

        v21 = (int)&_pUsb->ep0_cb_data[2];

        ++v20;

      }

      p_hw_data->pUsbRegBase->ENDPTNAK = v29;

    }

  }

  if ( _disr & 0x80 )

  {

    v26 = (void (__fastcall *)(USB_CORE_CTRL *))_pUsb->USB_SOF_Event;

    if ( v26 )

      v26(pUsb);

  }

  if ( _disr & 2 )

  {

    v27 = (void (__fastcall *)(USB_CORE_CTRL *, int))_pUsb->USB_Error_Event;

    if ( v27 )

    {

      v27(pUsb, _disr);

      JUMPOUT(&loc_10401D0E);

    }

  }

  JUMPOUT(&loc_10401D0E);

}

 

 

see? The rom api code has the same bug.

We should clear the ENDPTCOMPLETE before calling Ep handler

Because after call in Ep Handler, the handler may call WriteEP and Write operation may complete immediately, And then the rom bug code clear it, So we will never receive a USB_EVT_OUT interrupt because It clear it at a wrong time..

 

To prove my opinion, I wrote some code very same to the above decompile code

 

 

 

typedef volatile struct

{

    volatile uint32_t next_dTD;

    volatile uint32_t total_bytes ;

    volatile uint32_t buffer0;

    volatile uint32_t buffer1;

    volatile uint32_t buffer2;

    volatile uint32_t buffer3;

    volatile uint32_t buffer4;

    volatile uint32_t reserved;

}  DTD_T;

 

 

typedef struct _USB_HW_DATA{

    uint32_t    ep_QH;

    DTD_T       *ep_TD;

    uint32_t    ep_read_len[USB_MAX_EP_NUM];

    uint32_t    pUsbRegBase;

    USB_CORE_CTRL_T *pUsb;

}USB_HW_DATA;

 

 

#define USBSTS_UI    (1<<0)

#define USBSTS_UEI    (1<<1)

#define USBSTS_PCI    (1<<2)

#define USBSTS_URI    (1<<6)

#define USBSTS_SRI    (1<<7)

#define USBSTS_SLI    (1<<8)

#define USBSTS_NAKI    (1<<16)

 

 

void UsbResetEvent(USB_CORE_CTRL_T *pUsb)

{

    int i;

 

 

    for(i = 0; i < pUsb->num_ep0_hdlrs; i++)

    {

        if(pUsb->ep0_hdlr_cb[i])

        {

            ErrorCode_t err = pUsb->ep0_hdlr_cb[i](pUsb, pUsb->ep0_cb_data[i], USB_EVT_RESET);

            if(err != ERR_USBD_UNHANDLED)

            {

                if(err)

                {

                    USBD_API->hw->SetStallEP(pUsb, 0x80);     //Stall EP0

                    pUsb->EP0Data.Count = 0;

                }

                return;

            }

        }

    }

}

 

 

void UsbReset(USB_CORE_CTRL_T *pUsb)

{

    //I confused ....

    pUsb->device_status = 1;

    pUsb->device_addr = 0;

    pUsb->config_value = 0;

 

 

    pUsb->ep_halt = 0;

    pUsb->ep_mask = 0x10001;

    pUsb->ep_stall = 0;

    UsbResetEvent(pUsb);

}

 

 

void usbd_hw_ISR(USB_CORE_CTRL_T *pUsb)

{

    USB_HW_DATA *hw_data = (USB_HW_DATA *)pUsb->hw_data;

    LPC_USBHS_T *usb_reg = (LPC_USBHS_T *)hw_data->pUsbRegBase;

    uint32_t stat, val;

 

 

    stat = usb_reg->USBSTS_D;

    usb_reg->USBSTS_D = stat;

    stat &= usb_reg->USBINTR_D;

 

 

    if(stat & USBSTS_URI)

    {

        USBD_API->hw->Reset(pUsb);

        UsbReset(pUsb);

 

 

        if(pUsb->USB_Reset_Event)

            pUsb->USB_Reset_Event(pUsb);

        return;

    }

 

 

    if(stat & USBSTS_SLI)   //suspend

    {

        usb_reg->OTGSC &= ~1;       //discharge

 

 

        if(pUsb->USB_Suspend_Event)

            pUsb->USB_Suspend_Event(pUsb);

    }

 

 

    if(stat & USBSTS_PCI)   //resume

    {

        if(usb_reg->PORTSC1_D & 0x200)  //if high-speed mode

            pUsb->device_speed = USB_HIGH_SPEED;

        usb_reg->OTGSC |= 1;        //discharge

 

 

        if(pUsb->USB_Resume_Event)

            pUsb->USB_Resume_Event(pUsb);

    }

 

 

    //handle setup status interrupts

    if(usb_reg->ENDPTSETUPSTAT)

    {

        usb_reg->ENDPTCOMPLETE = 0x10001;   // clear the endpoint complete CTRL OUT & IN when a Setup is received

        usb_reg->ENDPTNAKEN |= 0x10001;     //enable NAK interrupts

 

 

        //only EP0 will have setup packets so call EP0 handler

        if(pUsb->ep_event_hdlr[0])

            pUsb->ep_event_hdlr[0](pUsb, pUsb->ep_hdlr_data[0], USB_EVT_SETUP);

    }

 

 

    //handle completion interrupts

    val = usb_reg->ENDPTCOMPLETE;

    if(val)

    {

        int i;

 

 

        usb_reg->ENDPTNAK = val;

        usb_reg->ENDPTCOMPLETE = val;   //clear complete flag before call ep handler

        for(i = 0; i < pUsb->max_num_ep; i++)

        {

            if(val & (1 << i))

            {

                if(pUsb->ep_event_hdlr[2 * i])

                    pUsb->ep_event_hdlr[2 * i](pUsb, pUsb->ep_hdlr_data[2 * i], USB_EVT_OUT);

                //usb_reg->ENDPTCOMPLETE = 1 << i;

            }

 

 

            if(val & (1 << (i + 16)))

            {

                hw_data->ep_TD[i * 2 + 1].total_bytes &= 0xC0;

                if(pUsb->ep_event_hdlr[2 * i + 1])

                    pUsb->ep_event_hdlr[2 * i + 1](pUsb, pUsb->ep_hdlr_data[2 * i + 1], USB_EVT_IN);

                //usb_reg->ENDPTCOMPLETE = 1 << (i + 16);

            }

        }

    }

 

 

    if(stat & USBSTS_NAKI)

    {

        val = usb_reg->ENDPTNAK;

        val &= usb_reg->ENDPTNAKEN;

 

 

        //handle NAK interrupts

        if(val)

        {

            int i;

 

 

            for(i = 0; i < pUsb->max_num_ep; i++)

            {

                if(val & (1 << i))

                {

                    if(pUsb->ep_event_hdlr[2 * i])

                        pUsb->ep_event_hdlr[2 * i](pUsb, pUsb->ep_hdlr_data[2 * i], USB_EVT_OUT_NAK);

                }

 

 

                if(val & (1 << (i + 16)))

                {

                    if(pUsb->ep_event_hdlr[2 * i + 1])

                        pUsb->ep_event_hdlr[2 * i + 1](pUsb, pUsb->ep_hdlr_data[2 * i + 1], USB_EVT_IN_NAK);

                }

            }

            usb_reg->ENDPTNAK = val;        //should put this befor if(val) ?

        }

    }

 

 

    if(stat & USBSTS_SRI)

    {

        //Start os Frame Interrupt

        if(pUsb->USB_SOF_Event)

            pUsb->USB_SOF_Event(pUsb);

    }

 

 

    if(stat & USBSTS_UEI)

    {

        //Error interrupt

        if(pUsb->USB_Error_Event)

            pUsb->USB_Error_Event(pUsb, stat);

    }

}

 

 

/**

* @brief Handle interrupt from USB0

* @return Nothing

*/

void USB_IRQHandler(void)

{

  //USBD_API->hw->ISR(g_hUsb);

    usbd_hw_ISR(g_hUsb);

}

 

 

 

The bug fixed.

It has been proved

 

 

 

mengxp

Outcomes