USB-CDC-driver hangs in USBD_Write

Hi Stefan,

I'm encountering the same problem with SET_CSR hanging up in USBD_Write on a AT91SAM7S256.

Have you managed to find a solution?

The only lead I have is a note in the datasheet, which explains that wait times should be used in a preemptive environment instead of polling the bits. I wonder if I am getting nested interrupts trying to set and clear the flags simultaneously.

Here is the note from the datasheet:

WARNING: Due to synchronization between MCK and UDPCK, the software application must wait for the end of the write operation before executing another write by polling the bits which must be set/cleared.

Note: In a preemptive environment, set or clear the flag and wait for a time of 1 UDPCK clock cycle and 1peripheral clock cycle. However,
RX_DATA_BLK0, TXPKTRDY, RX_DATA_BK1 require wait times of 3 UDPCK clock cycles and 3 peripheral clock cycles
before accessing DPR.

(from section 35.6.10 UDP Endpoint Control and Status Register in the AT91SAM7S Datasheet)

-Steven

Hi Stefan,

I'm encountering the same problem with SET_CSR hanging up in USBD_Write on a AT91SAM7S256.

Have you managed to find a solution?

The only lead I have is a note in the datasheet, which explains that wait times should be used in a preemptive environment instead of polling the bits. I wonder if I am getting nested interrupts trying to set and clear the flags simultaneously.

Here is the note from the datasheet:

WARNING: Due to synchronization between MCK and UDPCK, the software application must wait for the end of the write operation before executing another write by polling the bits which must be set/cleared.

Note: In a preemptive environment, set or clear the flag and wait for a time of 1 UDPCK clock cycle and 1peripheral clock cycle. However,
RX_DATA_BLK0, TXPKTRDY, RX_DATA_BK1 require wait times of 3 UDPCK clock cycles and 3 peripheral clock cycles
before accessing DPR.

(from section 35.6.10 UDP Endpoint Control and Status Register in the AT91SAM7S Datasheet)

-Steven

Hi,
we replaced the define

#define SET_CSR(endpoint, flags) \ 
    { \ 
        volatile unsigned int reg; \ 
        reg = AT91C_BASE_UDP->UDP_CSR[endpoint] ; \ 
        reg |= REG_NO_EFFECT_1_ALL; \ 
        reg |= (flags); \ 
        AT91C_BASE_UDP->UDP_CSR[endpoint] = reg; \ 
        while ( (AT91C_BASE_UDP->UDP_CSR[endpoint] & (flags)) != (flags)); \ 
    }

by a function with a "timeout"

int32_t USBD_setCSR(unsigned char endpoint, unsigned int flags)
{
    volatile unsigned int reg;
    volatile unsigned int cnt = 100000;
    reg = AT91C_BASE_UDP->UDP_CSR[endpoint] ;
    reg |= REG_NO_EFFECT_1_ALL;
    reg |= (flags);
    AT91C_BASE_UDP->UDP_CSR[endpoint] = reg;
    while ( cnt > 0)
    {
        if ((AT91C_BASE_UDP->UDP_CSR[endpoint] & (flags)) == flags)
        {
            break;
        }
        cnt--;
    }
    if (cnt > 0)
        return TRUE;
    else
        return FALSE;
}

Thus its not hanging any longer.

Hi Stefan,
> the USB driver hangs in the function/define SET_CSR waiting for the AT91C_UDP_TXPKTRDY flag.
> we replaced the define ... by a function with a "timeout" ... Thus its not hanging any longer.

Your comment gives me chance to think of problem on the coding on the Atmel source code.
I have doubt how this line is optimized by compiler,

while ( (AT91C_BASE_UDP->UDP_CSR[endpoint] & (flags)) != (flags));

In the Atmel source code, the macros are defined as follows - without any volatile.

AT91SAM9260.h

#define AT91_CAST(a) (a)

typedef struct _AT91S_UDP {
        AT91_REG         UDP_NUM;       // Frame Number Register
        AT91_REG         UDP_GLBSTATE;  // Global State Register
        AT91_REG         UDP_FADDR;     // Function Address Register
        AT91_REG         Reserved0[1];  //
        AT91_REG         UDP_IER;       // Interrupt Enable Register
        AT91_REG         UDP_IDR;       // Interrupt Disable Register
        AT91_REG         UDP_IMR;       // Interrupt Mask Register
        AT91_REG         UDP_ISR;       // Interrupt Status Register
        AT91_REG         UDP_ICR;       // Interrupt Clear Register
        AT91_REG         Reserved1[1];  //
        AT91_REG         UDP_RSTEP;     // Reset Endpoint Register
        AT91_REG         Reserved2[1];  //
        AT91_REG         UDP_CSR[6];    // Endpoint Control and Status Register
        AT91_REG         Reserved3[2];  //
        AT91_REG         UDP_FDR[6];    // Endpoint FIFO Data Register
        AT91_REG         Reserved4[3];  //
        AT91_REG         UDP_TXVC;      // Transceiver Control Register
} AT91S_UDP, *AT91PS_UDP;

#define AT91C_BASE_UDP       (AT91_CAST(AT91PS_UDP)     0xFFFA4000) // (UDP) Base Address

Tsuneo

Hi Stefan,

Thank you for posting your solution.

I ended up going about fixing the bug differently than your solution, because I am concerned about performance hits from using a larger function like that to replace the short macro, and I'm not sure how to handle the situation where the timeout does occur.

Noting that the datasheet states that a fixed delay should be used when code execution can be preempted, I added an interrupt lock around the USB packet sending code which includes SET_CSR. This has solved the problem for me.

Right now I'm locking interrupts around most of the packet sending code in USBD_Write. I think I made the critical section longer than it needs to be; I could probably just lock interrupts around the SET_CSR macro itself and then the performance effect would be negligible. Interestingly, I don't get lockups in other functions which use the SET_CSR macro, even though I'm not preventing preemption in those cases.

-Steven

Hi,
I just run into the problem again.
In the bootloader, the board hangs after transfering a specific amount of data.
Its curious, that it only happens with some of our boards, but not all them.
After some tests, if found, that the problem was caused by the use of a USB hub. I connected the boards directly to the PC and it runs fine.
I think it is a timing problem, which - for whatever reason - does not apply to all boards.

Can you post your code, please. I would like to try, if this solves the problem.

For startup code and interrupt handling I'm using the framework described in this series of articles:
" : "=r" (key_)); \ asm("MSR cpsr_c,#(0x1F | 0x80 | 0x40)"); \ }) #define ARM_INT_UNLOCK(key_) asm("MSR cpsr_c,%0" : : "r" (key_)) #endif

Here is how you use them:

    ARM_INT_KEY_TYPE int_lock_key;
    /* lock interrupts */
    ARM_INT_LOCK(int_lock_key);
    /* critical section goes here: */
          /*     ...      */
    /* put interrupt lock back into previous state */
    ARM_INT_UNLOCK(int_lock_key);