[DONE] USB hangs system during enumeration

[daemon]

I have a strange behavior while working with my project which uses USB (OTG1) driver.
Under certain optimization levels all works OK, but with other ones - it hangs CPU during USB enumeration.
To figure out I decided to check this on demo from the Chibios release package. Behavior was the same.

Environment:

STM32F4Discovery

Keil uVision MDK version 5.0 (also tested with 4.54 & 4.57 - results are the same)

Chibios 2.6.1 release

USB CDC example from ChibiOS_2.6.1/demos/ARMCM4-STM32F407-DISCOVERY-MEMS adapted for Keil

Ordinary Linux host with USB 2.0, running WinXP on VirtualBox (the same behavior on other Linux & Windoze machines)

Wireshark to capture USB traffic

Various compiler optimization levels (see below)

Result: If optimization level is -O0 or any other but with -Otime everything works, but without -Otime system hangs as soon as USB host enumerate device. Look at the attached traffic and a wireshark screenshot.

error upon "GET DESCRIPTOR REQUEST string"

usb_debug.png (89.79 KiB) Viewed 8967 times

Various cases:
-O0 - works good
-O1 - hangs
-O2 - hangs
-O3 - hangs
-O0 -Otime - works good
-O1 -Otime - works good
-O2 -Otime - works good
-O3 -Otime - works good

compiler string example (working):

-c --cpu Cortex-M4 -g -O1 -Otime --apcs=interwork -I. -I.\conf -I.\os\kernel\include -I.\os\ports\common\ARMCMx
-I.\os\ports\common\ARMCMx\CMSIS\include -I.\os\ports\RVCT\ARMCMx -I.\os\ports\RVCT\ARMCMx\STM32F4xx
-I.\os\hal\include -I.\os\hal\platforms\STM32 -I.\os\hal\platforms\STM32\GPIOv2 -I.\os\hal\platforms\STM32\OTGv1
-I.\os\hal\platforms\STM32F4xx -I.\os\various -I.\os\hal\platforms\STM32\USARTv2 -I.\os\hal\platforms\STM32\TIMv1
-I.\os\hal\platforms\STM32\SPIv2 -I.\os\various -I.\os\various\devices_lib\accel
-I C:\Keil\ARM\RV31\INC
-I C:\Keil\ARM\PACK\ARM\CMSIS\3.20.3\CMSIS\Include
-I C:\Keil\ARM\PACK\Keil\STM32F4xx_DFP\1.0.5\Device\Include
-DSTM32F40XX -D__heap_base__="Image$$RW_IRAM1$$ZI$$Limit" -D__heap_end__="Image$$RW_IRAM2$$Base" -o ".\out\*.o"
--omf_browse ".\out\*.crf" --depend ".\out\*.d"

compiler string example (not working - hang during enumeration):

-c --cpu Cortex-M4 -g -O1 --apcs=interwork -I. -I.\conf -I.\os\kernel\include -I.\os\ports\common\ARMCMx
-I.\os\ports\common\ARMCMx\CMSIS\include -I.\os\ports\RVCT\ARMCMx -I.\os\ports\RVCT\ARMCMx\STM32F4xx
-I.\os\hal\include -I.\os\hal\platforms\STM32 -I.\os\hal\platforms\STM32\GPIOv2 -I.\os\hal\platforms\STM32\OTGv1
-I.\os\hal\platforms\STM32F4xx -I.\os\various -I.\os\hal\platforms\STM32\USARTv2 -I.\os\hal\platforms\STM32\TIMv1
-I.\os\hal\platforms\STM32\SPIv2 -I.\os\various -I.\os\various\devices_lib\accel
-I C:\Keil\ARM\RV31\INC
-I C:\Keil\ARM\PACK\ARM\CMSIS\3.20.3\CMSIS\Include
-I C:\Keil\ARM\PACK\Keil\STM32F4xx_DFP\1.0.5\Device\Include
-DSTM32F40XX -D__heap_base__="Image$$RW_IRAM1$$ZI$$Limit" -D__heap_end__="Image$$RW_IRAM2$$Base" -o ".\out\*.o"
--omf_browse ".\out\*.crf" --depend ".\out\*.d"

I'm attaching complete compileable Keil project to demonstrate it.

I'm assuming some sort of a bug in usb_lld driver in which I'm hunting now. As a result of investigation I would post a patch and write an issue in bug tracker.

Would appreciate any hints where to dig. It would also be necessary to check the bug with GCC toolchain (anyone who works with it, please test it!)

[Giovanni]

Hi,

Please enable the debug options in chconf.h and look for stack overflows. Note that the USB driver has an internal thread too.

I also see a 131 bytes-long descriptor, the current driver does not support descriptors longer than 127 bytes because HW limitations on EP0 (there is a bug open about this already, it is an HW limitation but a workaround should be possible).

Giovanni

[daemon]

In debugger I saw unhandled exception upon system hang. From the call-stack I saw last call to otg_do_push():

Code: Select all

static uint8_t *otg_do_push(volatile uint32_t *fifop, uint8_t *buf, size_t n) {

  while (n > 0) {
    /* Note, this line relies on the Cortex-M3/M4 ability to perform
       unaligned word accesses and on the LSB-first memory organization.*/
    *fifop = *((uint32_t *)buf);
    buf += 4;
    n--;
  }
  return buf;
}

Maybe the note inside the above code is not always correct, so I rewrite this code as:

Code: Select all

static uint8_t *otg_do_push(volatile uint32_t *fifop, uint8_t *buf, size_t n) {

  while (n > 0) {
    *fifop = (uint32_t)(buf[0] | (buf[1] << 8) | (buf[2] << 16) | (buf[3] << 24));
    buf += 4;
    n--;
  }
  return buf;
}

which now works OK with any optimization level. So I conclude that unaligned word access sometimes is not a right assumption. On the other hand, we are not want to loose such good optimization, so maybe there are some sort of #pragma to tell compiler how to compile this code?

Also, are there any other places inside Chibios STM32 HAL with such code? It would also need corrections like this (or better).

[Giovanni]

It is the only place if I remember well. Unaligned accesses are something that is enabled by default on Cortex-Ms (see UNALIGN_TRP setting), is it possible that Keil disables that feature by default?

Giovanni

[daemon]

Also found and corrected unaligned accesses in otg_do_pop(), otg_fifo_read_to_buffer() in usb_lld.c. More to come?

[daemon]

Giovanni, I will check MDK docs to figure out regarding unaligned access. Maybe under different optimization level it is turned off, as you suggest.

[Giovanni]

It is not matter of optimization, it is the core that does that, the feature is normally enabled by default (UNALIGN_TRP bit).

Probably the optimization level just triggers the unaligned case.

Giovanni

[daemon]

As I read from docs, UNALIGN_TRP bit is a unaligned access exception trap bit. Yes, it is disabled (0) by default in CCR register. But in one chapter of ARM architecture doc about binary compatibility of ARMv7-M they suggest to enable this trap bit, so it would generate exception upon unaligned access like in current usb_lld.c . I can't figure out where UNALIGN_TRP bit is set or reset. Maybe it is needed to be explicitly set to '0' in the STM32 startup code?..

Any way the docs from ARM give better solution of using __packed qualifier for pointers which are casting to possible unaligned data. So, do not rely on "unalign must be turned on", and explicitly use __packed qualifier for such cases. What you think, Giovanni?

align_1.png (45.17 KiB) Viewed 8939 times

As an example:
was

Code: Select all

*fifop = *((uint32_t *)buf);

becomes

Code: Select all

*fifop = *((PACK_PTR uint32_t *)buf);

where PACK_PTR defined as

Code: Select all

#define PACK_PTR __packed

in chtypes.h in analogue to PACK_STRUCT_xxx

I attach patch for this. This is only for RVCT port for ARMCMx and also must be checked and patched for other ports. For GCC it probably would be __attribute__((packed))

With above modifications usb_lld works well. More effective solution would be a correct and explicit enable of hardware unligned access in Cortex-M3/M4.

[Giovanni]

The problem is that other compilers do not have necessarily that functionality so I can't rely on that.

Giovanni

[daemon]

The problem is that other compilers do not have necessarily that functionality so I can't rely on that.

Sure, thats why there is various ports for different compilers (RVCT, IAR, GCC) inside Chibios and different chtypes.h files. Isn't it? So it's possible to use one macros which is, however, compiler specific.