[DONE] USB hangs system during enumeration

[Giovanni]

You assume that each compiler has the equivalent of that packed keywork, this is not the case, the packed attribute for GCC has a different placement, from the manual:

Code: Select all

struct foo
{
  char a;
  int x[2] __attribute__ ((packed));
};

How would you wrap it in that macro?

I think you just need to set UNALIGN_TRP to zero, the unaligned accesses are meant to work on the Cortex-M3/4 and those unaligned accesses are documented and only present in drivers for Cortex-M3/4 MCUs.

Giovanni

[daemon]

Finally, I found a cause of this problem and now solution became obvious. Let's see.

Assembly listing of a problem code which results in an unaligned fault is:

Code: Select all

                  otg_do_push PROC
;;;239     */
;;;240    static uint8_t *otg_do_push(volatile uint32_t *fifop, uint8_t *buf, size_t n) {
0007de  e002              B        |L1.2022|
                  |L1.2016|
;;;241    
;;;242      while (n > 0) {
;;;243        *fifop = *((uint32_t *)buf);
0007e0  c908              LDM      r1!,{r3}
0007e2  6003              STR      r3,[r0,#0]
;;;244        buf += 4;
;;;245        n--;
0007e4  1e52              SUBS     r2,r2,#1
                  |L1.2022|
0007e6  2a00              CMP      r2,#0                 ;242
0007e8  d1fa              BNE      |L1.2016|
;;;246      }
;;;247      return buf;
0007ea  4608              MOV      r0,r1
;;;248    }
0007ec  4770              BX       lr
;;;249    
                          ENDP

Note LDM r1!,{r3}. This compiled with -O3 flag. Similar code for -O1, -O2 (RVCT).

And this is a code when compiled with -O0 or any other with -Otime which work well:

Code: Select all

                  otg_do_push PROC
;;;239     */
;;;240    static uint8_t *otg_do_push(volatile uint32_t *fifop, uint8_t *buf, size_t n) {
0000e8  4603              MOV      r3,r0
0000ea  4608              MOV      r0,r1
;;;241    
;;;242      while (n > 0) {
0000ec  2a00              CMP      r2,#0
0000ee  d005              BEQ      |L1.252|
                  |L1.240|
;;;243        *fifop = *((uint32_t *)buf);
0000f0  6801              LDR      r1,[r0,#0]
0000f2  6019              STR      r1,[r3,#0]
;;;244        buf += 4;
0000f4  1d00              ADDS     r0,r0,#4
;;;245        n--;
0000f6  1e52              SUBS     r2,r2,#1
0000f8  2a00              CMP      r2,#0                 ;242
0000fa  d1f9              BNE      |L1.240|
                  |L1.252|
;;;246      }
;;;247      return buf;
;;;248    }
0000fc  4770              BX       lr
;;;249    
                          ENDP

Note LDR instead of LDM This is a key!

Code with __packed qualifier regardless of the optimization level gives such code (snap):

Code: Select all

;;;243        *fifop = *((__packed uint32_t *)buf);
0007e0  f8513b04          LDR      r3,[r1],#4
0007e4  6003              STR      r3,[r0,#0]
;;;244        buf += 4;
;;;245        n--;
0007e6  1e52              SUBS     r2,r2,#1

There is again LDR which works on any aligned/unaligned data in Cortex-M3/M4. But why? Let's see the docs:

thats_it.png (58.15 KiB) Viewed 3925 times

So load/store multiple (which indeed a bit effective than LDR/STR when operating with many words at once) are ALWAYS generate unaligned fault regardless of an UNALIGN_TRP bit!!! So, assumption provided in the usb_lld.c is not always correct (as I thought from the start):

Code: Select all

    /* Note, this line relies on the Cortex-M3/M4 ability to perform
       unaligned word accesses and on the LSB-first memory organization.*/

GCC always(?) generate LDR/STR. It is safe, but some times produce less effective code then RVCT. We can't be sure that in the future versions of GCC (or any other compiler) it not became more effective and starts to use LDM (or any other cmd) with alignment restrictions described above. Do you agree that we can't rely on that? Instead of that it is needed to provide compiler with explicit info that this part of code it should compile as we expect from it. As a result, it is mandatory to use a "packed" qualifier on such data pointers. Example is in my previous post.

How would you wrap it in that macro?

From chtypes.h we have:

/**
* @brief Packed structure modifier (within).
* @note Empty in this port.
*/
#define PACK_STRUCT_STRUCT

/**
* @brief Packed structure modifier (before).
*/
#define PACK_STRUCT_BEGIN __packed

/**
* @brief Packed structure modifier (after).
* @note Empty in this port.
*/
#define PACK_STRUCT_END

In addition I suggest (RVCT/IAR):

Code: Select all

/**
 * @brief   Unaligned access with packed pointer.
  */
#define PACK_PTR __packed

for GCC:

Code: Select all

/**
 * @brief   Unaligned access with packed pointer.
  */
#define PACK_PTR __attribute__((packed))

then use this macro when accessing arbitrary aligned data:

Code: Select all

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

Working patch for RVCT I had provided before.
That's all.

[Giovanni]

Hi,

Now the problem is clear, thanks for finding. I will make changes to the driver after testing the other compilers. Marking the thread as TODO.

Giovanni

[Giovanni]

Hi,

Committed a fix, could you verify if it is OK on your side?

thanks,
Giovanni

[daemon]

I can confirm now that with ChibiOS-2.6.2 it is OK (problem solved) when compiled with Keil RVCT.
Thanks for your great work!