• State
  • Locked Locked
  • Replies 3 replies
  • Subscribers 31 subscribers
  • Views 464 views
  • Users 0 members are here
Support feedback
Options
Options
Related

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

RM48L950ZVT HALCoGen USB driver bugs

Engy
Prodigy 50 points

Other Parts Discussed in Thread: HALCOGEN

Greetings to all.

  I have trouble with USB device driver generated by latest HALCoGen 04.05.00 for RM48L950ZVT CPU. I have found several bugs in USB device driver. I have created the patch (http://pastebin.com/CZpQN9LL) which fixes most of them, but not all. Here is detailed description for individual changes

1) usb.c - line 285 - USBDevEndpointStall - missing endpoint specification to usbd0Regs->epnum

2) usb.c - line 333 - USBDevEndpointStallClear - missing endpoint specification to usbd0Regs->epnum

3) usb.c - line 454 - USBEndpointDataAvail - mask the endpoint with USBD_EP_NUM_EP_NUM_MASK

4) usb.c - line 536 - USBEndpointDataGet - do NOT ack received data to FIFO. Acked by USBDevEndpointDataAck function

5) usb.c - line 574 - USBDevGetSetupPacket - typo

6) usb.c - line 651 - USBDevEndpointDataAck - missing endpoint specification to usbd0Regs->epnum, removed double write to usbd0Regs->epnum

7) usbdcdc.c - line 1335 - ProcessDataToHost - When sending data packet to host with size == maxpacketsize, empty packet must follow

8) usbdconfig.c - line 545 - USBDeviceConfig - using declared constant instead of "magic number"

9) usbdenum.c - line 1939, 1985 - USBDGetDescriptor - clear pEP0Data pointer when signaling STALL (no data to send later in function)

10) usbdenum.c - line 3029 - USBDeviceIntHandlerInternal - using declared constant instead of "magic number"

Next issue is in USB send function (USBEndpointDataPut) when interrupted by USB interrupt function, it destroys selected endpoint for writing in usbd0Regs->epnum. This issue is not fixed by the patch.

↓ NEED HELP ↓

But one issue I'm not able to solve. It is STALL signaling. For example when reading not existing "string descriptor", device should respond with STALL to signal to the hast that function failed, but it not respond. Stall function is called but on USB line there are only NAKs forever. See http://snag.gy/3S4EX.jpg  It can be tested with "Thesycon USB Descriptor Dumper" from here http://www.thesycon.de/eng/usb_descriptordumper.shtml .

  • 0
    TI__Mastermind 22375 points

    Engy,

    Could you attached in the post your patch for your modified files. The website you are using is blocked from TI.

    Concerning your open problem, I will contact the Halcogen/USB team.


    Thanks for your comments.

  • 0 in reply to Jean-Marc Mifsud
    Prodigy 50 points

    Here is the patch
    0385.rm48_usb_patch.txt



    diff -Burp src/usb.c usbmod/usb.c
    --- src/usb.c 2015-09-01 12:33:27.691869900 +0200
    +++ usbmod/usb.c 2015-09-02 14:51:54.694726800 +0200
    @@ -285,8 +285,10 @@ void USBDevEndpointStall(uint32 ulBase,
    (usEndpoint == USB_EP_12) || (usEndpoint == USB_EP_13) ||
    (usEndpoint == USB_EP_14) || (usEndpoint == USB_EP_15));

    + usEndpoint = usEndpoint >> 4;
    +
    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    - USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | usFlags)); /* Select EP */
    + USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | (usEndpoint & USBD_EP_NUM_EP_NUM_MASK) | usFlags)); /* Select EP */

    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    USBD_REG_SET_ONE(usbd0Regs->ctrl, USBD_CTRL_SET_HALT); /* halt the selected EP */
    @@ -296,6 +298,8 @@ void USBDevEndpointStall(uint32 ulBase,
    /*SAFETYMCUSW 45 D MR:21.1 <APPROVED> "Statically defined non-null hardware register address" */
    USBD_REG_BIT_CLR(usbd0Regs->epnum, USBD_EP_NUM_EP_SEL); /* Deselct EP */

    + usEndpoint = usEndpoint << 4;
    +
    return;
    }

    @@ -333,15 +337,19 @@ void USBDevEndpointStallClear(uint32 ulB
    (usEndpoint == USB_EP_14) || (usEndpoint == USB_EP_15));
    ASSERT((usFlags & ~(USB_EP_DEV_IN | USB_EP_DEV_OUT)) == 0);

    + usEndpoint = usEndpoint >> 4;
    +
    /* Select the specified endpoint and clear the stall */
    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    - USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | usFlags));
    + USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | (usEndpoint & USBD_EP_NUM_EP_NUM_MASK) | usFlags));
    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    USBD_REG_BIT_SET(usbd0Regs->ctrl, USBD_CTRL_CLR_HALT);
    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    /*SAFETYMCUSW 185 S <INSPECTED> "Reason - LDRA tool issue."*/
    /*SAFETYMCUSW 45 D MR:21.1 <APPROVED> "Statically defined non-null hardware register address" */
    USBD_REG_BIT_CLR(usbd0Regs->epnum, USBD_EP_NUM_EP_SEL);
    +
    + usEndpoint = usEndpoint << 4;
    }

    /** ***************************************************************************
    @@ -454,7 +462,7 @@ uint16 USBEndpointDataAvail(uint32 ulBas

    /* Setup the epnum register. Note that it's always OUT EP */
    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    - USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | usEndpoint | USBD_EP_DIR_OUT));
    + USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | (usEndpoint & USBD_EP_NUM_EP_NUM_MASK) | USBD_EP_DIR_OUT));

    /* Read the count from the RXFstat register */
    retVal = usbd0Regs->rxf_stat & USBD_RXFSTAT_RXF_COUNT;
    @@ -536,13 +544,9 @@ sint32 USBEndpointDataGet(uint32 ulBase,
    pucData[i] = *((volatile uint8 *)(&usbd0Regs->data));
    }

    - /* Restore the epnym register */
    + /* Restore the epnum register */
    usbd0Regs->epnum = usEpNum;

    - /* setup to receive data on ep */
    - /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    - USBD_REG_SET_ONE(usbd0Regs->ctrl, USBD_CTRL_SET_FIFO_EN);
    -
    usEndpoint = usEndpoint << 4;

    return(0);
    @@ -574,7 +578,7 @@ void USBDevGetSetupPacket (uint32 ulBase
    /*SAFETYMCUSW 94 S MR:11.4 <INSPECTED> "Reason - Acceptable deviation."*/
    pSetupFifo = (volatile uint8 *)(&usbd0Regs->data);

    - /* Select the setup IFO. This will clear the event flag */
    + /* Select the setup FIFO. This will clear the event flag */
    usbd0Regs->epnum = USBD_EP_NUM_SETUP_SEL;

    /* Extract the setup packet */
    @@ -651,11 +655,10 @@ void USBDevEndpointDataAck(uint32 ulBase
    /* backup the EP number to restore later */
    epNum = usbd0Regs->epnum & USBD_EP_NUM_EP_NUM_MASK;

    - /* Select the endpoint, (Always read (in) endpoint) */
    - usbd0Regs->epnum = USBD_EP_DIR_OUT | USBD_EP_NUM_EP_SEL | (usEndpoint & USBD_EP_NUM_EP_NUM_MASK);
    + usEndpoint = usEndpoint >> 4;

    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    - USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | usEndpoint | USBD_EP_DIR_OUT));
    + USBD_REG_SET_ONE(usbd0Regs->epnum, (USBD_EP_NUM_EP_SEL | (usEndpoint & USBD_EP_NUM_EP_NUM_MASK) | USBD_EP_DIR_OUT));
    /* Clear endpoint */
    /*SAFETYMCUSW 184 S <INSPECTED> "Reason - LDRA tool issue."*/
    USBD_REG_SET_ONE(usbd0Regs->ctrl, USBD_CTRL_CLR_EP);
    @@ -667,10 +670,12 @@ void USBDevEndpointDataAck(uint32 ulBase
    /*SAFETYMCUSW 185 S <INSPECTED> "Reason - LDRA tool issue."*/
    USBD_REG_BIT_CLR(usbd0Regs->epnum, USBD_EP_NUM_EP_SEL);

    - /* Restore the epnym register */
    + /* Restore the epnum register */
    /*SAFETYMCUSW 45 D MR:21.1 <APPROVED> "Statically defined non-null hardware register address" */
    usbd0Regs->epnum = epNum;

    + usEndpoint = usEndpoint << 4;
    +
    return;
    }

    diff -Burp src/usbdcdc.c usbmod/usbdcdc.c
    --- src/usbdcdc.c 2015-09-01 12:33:27.680868800 +0200
    +++ usbmod/usbdcdc.c 2015-09-02 14:32:47.055080400 +0200
    @@ -1335,8 +1335,15 @@ ProcessDataToHost(const tUSBDCDCDevice *
    */
    txSize = (uint32)psInst->usLastTxSize;
    psInst->usLastTxSize = 0u;
    - psDevice->pfnTxCallback(psDevice->pvTxCBData, USB_EVENT_TX_COMPLETE,
    - txSize, (void *)0);
    + if (txSize == DATA_OUT_EP_MAX_SIZE)
    + {
    + USBDCDCPacketWrite((void *)psDevice, 0, 0, TRUE);
    + }
    + else
    + {
    + psDevice->pfnTxCallback(psDevice->pvTxCBData, USB_EVENT_TX_COMPLETE,
    + txSize, (void *)0);
    + }

    return (TRUE);
    }
    diff -Burp src/usbdconfig.c usbmod/usbdconfig.c
    --- src/usbdconfig.c 2015-09-01 12:33:27.684869200 +0200
    +++ usbmod/usbdconfig.c 2015-09-02 14:22:13.702854800 +0200
    @@ -545,8 +545,8 @@ USBDeviceConfig(uint32 ulIndex, const tC
    USBDevCfgLock(USBD_0_BASE);

    /* Send a zero packet */
    - USBEndpointDataPut(USBD_0_BASE, 0u, 0, 0u);
    - USBEndpointDataSend(USBD_0_BASE, 0u, (uint32)USBD_EP_DIR_IN);
    + USBEndpointDataPut(USBD_0_BASE, USB_EP_0, 0, 0u);
    + USBEndpointDataSend(USBD_0_BASE, USB_EP_0, (uint32)USBD_EP_DIR_IN);

    /*
    * If we get to the end, all is well.
    diff -Burp src/usbdenum.c usbmod/usbdenum.c
    --- src/usbdenum.c 2015-09-01 12:33:27.687869500 +0200
    +++ usbmod/usbdenum.c 2015-09-02 14:24:42.441362100 +0200
    @@ -1939,6 +1939,8 @@ USBDGetDescriptor(void * pvInstance, tUS
    if(lIndex == -1)
    {
    USBDCDStallEP0(0u);
    + psUSBControl->pEP0Data = 0u;
    + psUSBControl->ulEP0DataRemain = 0u;
    break;
    }

    @@ -1985,6 +1987,8 @@ USBDGetDescriptor(void * pvInstance, tUS
    * just stall the request.
    */
    USBDCDStallEP0(0u);
    + psUSBControl->pEP0Data = 0u;
    + psUSBControl->ulEP0DataRemain = 0u;
    }
    break;
    }
    @@ -3026,7 +3030,7 @@ USBDeviceIntHandlerInternal(uint32 uInde
    USBDIntHandlerSof(USBD_0_BASE);
    }

    - USBIntEnable(USBD_0_BASE, (uint16)0xB9u);
    + USBIntEnable(USBD_0_BASE, USBD_INT_EN_ALL);
    USBIntStatusClear(uIrqSrc);
    }

  • 0 in reply to Engy
    TI__Mastermind 22375 points

    Engy,

    Thanks for your patch.

    A ticket (SDOCM00120275) was created to report all this issues on USB driver.
    The updated driver has been commited for release 04.05.02