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

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

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

1,139 Views
mengxp
Contributor II


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

Labels (2)
Tags (1)
0 Kudos
Reply
2 Replies

776 Views
Carlos_Mendoza
NXP Employee
NXP Employee

Hi Mengxp,

Thank you for reporting this bug and its fix, I have already forwarded this to the developers for it to be fixed.

Best Regards!

Carlos Mendoza

Technical Support Engineer

0 Kudos
Reply

776 Views
stdout
Contributor I

I am experiencing the same bug.

carlosmendoza‌: Any update on that topic?

0 Kudos
Reply