Packed attribute warning - what is a "packed uint32_t"?

[SiliconWizard]

And arguably it should work, given that the 32F4 supports unaligned 32 bit loads and stores.

  Well yes, if unaligned traps are not enabled, it would work. The only thing to keep in mind is that ldrd/strd do not support unaligned access, but that's generally not a problem.

All Cortex-M3/M4/M7 devices should support unaligned access. I don't think there is even an option in IP to disable that.

Not sure either.
Now for Aarch64 (which is not the OP's concern here, but just saying), unaligned access is not supported unless you configure the MMU - learned that when writing baremetal code for the RPi 4.

[wek]

I posted the question on th ST forum (where usually there are no replies)

Of your 39 questions there, 3 went with no replies.
[EDIT]Okay, 4, as this is no reply either. I just counted the zeros.

JW

[Nominal Animal]

What is actually wrong with the existing (ST) code assuming the buffers are explicitly 4-aligned?

That USB FIFO works explicitly in 32-bit units, so feeding it a byte array and a byte length (which is silently rounded up to a multiple of four), is just the wrong approach.

Just because you do 16 stores, does not mean you have to do exactly 16 loads, too.

I think it does.

Nope.  Using the functions I showed, it does 64 byte loads on architectures where unaligned 32-bit loads are not supported.

That's not all.  You can also do 15 32-bit reads, and 4 byte reads (for a total of 19 reads), and exactly 16 writes.

You do this by checking the lowest two bits of the source pointer, and pick one of four functions, one for each alignment (correct, off-by-1, off-by-2, off-by-3).  The aligned one is trivial.  For the unaligned ones, you combine two 32-bit registers into a 64-bit word, to use as a barrel shifter recovering the alignment.

Consider the case where the buffer address low bits are 11, meaning there is a single byte, followed by 15 aligned 32-bit words, followed by three bytes.  You load the first byte to the low bits of the upper word, and the first full word to the low word:
    |00000000 00000000 00000000 aaaaaaaa|bbbbbbbb cccccccc dddddddd eeeeeeee|
and then rotate left by 24 bits, getting
    |aaaaaaaa bbbbbbbb cccccccc dddddddd|eeeeeeee 00000000 00000000 00000000|
You write the upper word, now perfectly aligned.  Then, you rotate left by 8 bits:
    |bbbbbbbb cccccccc dddddddd eeeeeeee|00000000 00000000 00000000 00000000|
For the rest of the aligned 32-bit words, you repeat from the first step, reading the next aligned 32-bit word to the low part. For the three last bytes, you read them into the high bits of the low part instead:
    |bbbbbbbb cccccccc dddddddd eeeeeeee|xxxxxxxx yyyyyyyy zzzzzzzz 00000000|
and do a final rotate left by 24 bits, getting the final 32-bit word,
    |eeeeeeee xxxxxxxx yyyyyyyy zzzzzzzz 00000000|00000000 00000000 00000000 00000000|
writing the upper word.  You're now done.

For an offset of k bits, the shifts are k and 32-k bits.  This works with even non-byte-aligned inputs, you see.

You do not need hardware bit rotate support or even 64-bit type support to do this, either; you can use

Code: [Select]

struct u32pair {
    uint32_t  lo;
    uint32_t  hi;
};

static inline u32pair u32pair_shift_left(struct u32pair p, uint_fast8_t n)
{
    return (struct u32pair){ .hi = (p.hi << n) | (p.lo >> (32 - n)), .lo = p.lo << n);
}

See?

Yes, the cost is four shifts per 32-bit word output, plus some extra work for the initial and final unaligned bytes, but that wasn't the point.  The point is that just because you need to write exactly 16 32-bit words from a possibly unaligned buffer, it does not mean you have to do 16 unaligned 32-bit reads from said unaligned buffer: you can do 15 aligned 32-bit reads and 4 byte reads instead, plus some extra work per 32-bit word.

(If you don't see, just say so: I can write the functions [and test and verify they work as I described, which is the annoying bit] for you to examine at leisure.)

[peter-h]

Of your 39 questions there, 3 went with no replies.

Did you count the

- replies with little or no information content
- replies weeks later
- replies by "Piranha" telling one that they are a useless idiot, can't read, and providing info which is deliberately incomplete so "Piranha" gets multiple chances to tell the person that they are a useless idiot who can't read (many others have been on the receiving end of this; not just me)

That USB FIFO works explicitly in 32-bit units, so feeding it a byte array and a byte length (which is silently rounded up to a multiple of four), is just the wrong approach.

Do we actually know this? All the info I have seen is in ST'd code. Debugs in the tx function (done at high speed using ITM console) on "len" show this:

18 18 64 34 26 4 36 18 18 9 64 34 18 4 28 18 4 38 2 4 2 26 1 36 13 36 13 12 13 8 13 7 8 13 64 64 64 64 64 64 64 64 13 23 13 23 13 8 13 8 9 13 64 64 64 64 64 64 64 64 64 13 34 64 64 64 64 64 64 64 64 4 13 8 13 8 13 38 64 64 64 64 64 64 64 64 13 36 64 64 64 64 64 64 64 64 26 13 8 13 8 13 64 64 64 64 64 64 64 64 13 8 13 64 64 64 64 64 64 64 64 13 8 13 8 13 64 64 64 64 64 64 64 64 13 8 13 13 8 13 8 13 64 64 64 64 64 64 64 64 13 13 64 64 64 64 64 64 64 64 13 8 13 8 13 64 64 64 64 64 64 64 64 13 8 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 13 8 13 8 13 64 64 64 64 64 64 64 64 13 64 64 64 64 64 64 64 64 13 8 13 8 13 64 64 64 64 64 64 64 64 13 8 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 13 8 13 8 13 64 64 64 64 64 64 64 64 13 64 64 64 64 64 64 64 64 13 8 13 8 13 64 64 64 64 64 64 64 64 13 8 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 23 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 8 13 8 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 13 13 13 13 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 64 13 13 13 13 7 7 7 7 13 13 13 13 13 13 13 13 13 13 24 13 5 64 64 64 64 50 10 6 4 6 4 19 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 13 6 2 7 64 12 2 3 2 3 2 3 1 13 13 13 13 13 13 13 13 13 13 13 13 13

so you can see it is any number from 1 to 64. Circumstantially I think most of the 64s above are MSC (which always transfers 8x64=512 rapidly) and the rest are CDC; I have Teraterm running on a VCP port.

So clearly the 32F4 USB hardware is happy to get 4 byte multiples but it discards the extra.

wek may well know all this.

But, hey, I did something innovative: googled on USBx_DFIFO (googling on register names is what 73.5% of ST coders do), and found this

https://community.st.com/s/question/0D50X0000BSXYHCSQ5/sw4stm32-doesnt-like-the-project-generated-by-stmcubemx-v53



That is for 32F7xx. The last post was unanswered

Or the compiler will actually generate code to access unaligned data by bytes and reassemble?

but does it make sense? Is there a need for different machine code if the buffer is unaligned?

Eventually we come full circle
https://www.eevblog.com/forum/microcontrollers/32f417-usb-fs-(not-hs)-and-dma/

[Nominal Animal]

That USB FIFO works explicitly in 32-bit units, so feeding it a byte array and a byte length (which is silently rounded up to a multiple of four), is just the wrong approach.

Do we actually know this?

Even if it internally discards extra bytes, it still works in explicitly 32-bit units.  Making the buffers 32-bit avoids all the alignment problems.  And, as I showed, if one needs to access individual bytes in the buffer, that is still trivially possible using a simple cast.

Edited to add:  Obviously, the USB messages themselves are not 32-bit aligned.  Only the interface to the USB FIFO features on that MCU is 32-bit.  And because it is, it makes a lot of sense to make the buffer interface 32-bit and 32-bit aligned also.

[ataradov]

Putting the data in a buffer and specifying the size are two separate operations. It is not an actual FIFO. It is cleared after each frame is sent, so you can write in multiples of 4, the remaining 1-3 bytes will be discarded.

[Nominal Animal]

Putting the data in a buffer and specifying the size are two separate operations. It is not an actual FIFO. It is cleared after each frame is sent, so you can write in multiples of 4, the remaining 1-3 bytes will be discarded.

I suspected that would be the case.

Nevertheless, because the interface processes the messages in 32-bit units, making the C interface use 32-bit units for the message buffers makes most sense.  Abstraction, making the interface consume (unaligned) arbitrary buffers, is incorrect here if the hardware does not always support unaligned 32-bit accesses.  It is better to ensure alignment, because it is the Path of Least Surprises.

Perhaps the following would be acceptable?  Data buffer is explicitly 32-bit units, to ensure proper alignment, but the length is in bytes?  (You have much more experience with these than I do.)

Code: [Select]

HAL_StatusTypeDef USB_WritePacket(USB_OTG_GlobalTypeDef *USBx, const uint32_t *src, uint_fast8_t ch_ep_num, uint_fast16_t bytes, uint_fast8_t dma)
{
  const uintptr_t USBx_BASE = (uintptr_t)USBx;

  if (dma == 0U) {
    const uint32_t *p = src;
    const uint32_t *const q = src + (bytes + 3) / 4;

    while (p < q) {
      USBx_DFIFO((uint32_t)ch_ep_num) = *(p++);
    }
  }

  return HAL_OK;
}

uint32_t *USB_ReadPacket(USB_OTG_GlobalTypeDef *USBx, uint32_t *dst, uint_fast16_t bytes)
{
  const uintptr_t USBx_BASE = (uintptr_t)USBx;

  uint32_t *p = dst;
  uint32_t *const q = dst + (bytes + 3) / 4;

  while (p < q) {
    *(p++) = USBx_DFIFO(0U);
  }

  return dst;
}

Although, I must admit, the way the USBx_DFIFO() macro refers USBx_BASE gives me the willies.  I'd really prefer
    #define  USBx_DFIFO(base, ep)  (*(__IO uint32_t *)((unsigned char *)(base) + USB_OTG_FIFO_BASE + (uintptr_t)(ep) * USB_OTG_FIFO_SIZE))
so that the loop innards above become
      USBx_DFIFO(USBx, ch_ep_num) = *(p++);
and
      *(p++) = USBx_DFIFO(USBx, 0);
respectively.

[wek]

Of your 39 questions there, 3 went with no replies.

Did you count the

- replies with little or no information content
- replies weeks later
- replies by "Piranha" telling one that they are a useless idiot, can't read, and providing info which is deliberately incomplete so "Piranha" gets multiple chances to tell the person that they are a useless idiot who can't read (many others have been on the receiving end of this; not just me)

Yes. And I counted also the information provided by unpaid volunteers, who provide genuine information and don't have time to custom-tailor them to your very particular setup, so the burden of understanding and implementing it is upon you.

That USB FIFO works explicitly in 32-bit units, so feeding it a byte array and a byte length (which is silently rounded up to a multiple of four), is just the wrong approach.

This is true, but irrelevant. The __packed attribute is placed on the user buffer, not the USB hardware FIFO.

As I've said over on the forum where nobody ever answers, the code has already changed. I don't know if it's perfect now, I don't use Cube.

JW

[peter-h]

I've designed hardware FIFOs in the past (FPGA & FPGA -> ASIC) and they were always bytewide, if you want to end up with bytes at the output (which, with USB, you do).

So I think that 32 bit data register just gets dumped to a bytewide FIFO, in four clocks, until the byte count is exhausted and then the dumping stops.

Come to think of it, I can't see where the above byte count (1-64) is actually specified. It probably isn't, and the USB controller always transfers a multiple of 4 into a FIFO, and then extracts the required # of bytes from there (it knows how many it wants) and clears the FIFO.

I am a "simple" C programmer, and keep my code very obvious, because I am working alone and need to understand it myself. I had an occassion recently where I wanted to compare two uint8_t 2k buffers fast, and memcmp was not particularly fast (surprisingly; it is always claimed to be optimised) so I tried a loop which does it 32 bits at a time, but failed to make it work. I am sure it is dead simple.

I see wek has replied in the ST forum, affirmatively that the existing code should work regardless of the buffer being aligned, on the 32F4, but there were past issues with other 32F CPUs and this code is a fossil from there.

the code has already changed.

Where can it be found?

who provide genuine information and don't have time to custom-tailor them to your very particular setup, so the burden of understanding and implementing it is upon you.

You do, for which I thank you; not many others, and some of the other stuff is positively offensive (that guy needs to get a life).

[Nominal Animal]

That USB FIFO works explicitly in 32-bit units, so feeding it a byte array and a byte length (which is silently rounded up to a multiple of four), is just the wrong approach.

This is true, but irrelevant. The __packed attribute is placed on the user buffer, not the USB hardware FIFO.

My comment was about the C interface, so it most definitely is relevant.

Instead of simply marking the buffer unaligned, so the compiler will deal with any alignment issues, I claim that it is better to use the data type native to the interface (32-bit words) instead, since it also ensures the buffers are aligned, and reminds the programmers that this interface really prefers the buffers to be aligned.  It does not preclude using the buffers in a byte-wise manner, since casting the buffer to an unsigned char pointer does exactly that.  Path of Least Surprise and all.

[wek]

I've designed hardware FIFOs in the past (FPGA & FPGA -> ASIC) and they were always bytewide, if you want to end up with bytes at the output (which, with USB, you do).

In 32-bitters, byte-aligned bus interface is an extra burden (read: silicon area, logic complexity hence speed, etc.), especially if you incorporate things like DMA (and that in both simple and scatter-gather form, optionally). The Synopsys OTG-USB module we are talking about here is a mammoth, in all senses of the word.

The other USB module (mostly device-only) used in "lower-end" STM32 is even more weird in this respect, as it for historical reasons uses 16-bit data, in some STM32 models mapped to 32-bits (yeah!) and in others accessed strictly-16-bit.

In both cases, the burden of packing/unpacking/realigning/whatever the naturally byte-natured data is upon the programmer, but at the moment somebody starts to write a "library", it is expected by the users of "library" that it's that "library" which takes upon itself that burden.

> Come to think of it, I can't see where the above byte count (1-64) is actually specified.

This includes 0.

We've discussed this already: the Synopsys OTG has the entire transfer detached into two separate processes - "set up transfer" and "actually dump data" - to allow for the optional DMA. Number of bytes to be transferred is written in the former into the respective DIEPTSIZx/DOEPTSIZx register.

JW

[wek]

That USB FIFO works explicitly in 32-bit units, so feeding it a byte array and a byte length (which is silently rounded up to a multiple of four), is just the wrong approach.

This is true, but irrelevant. The __packed attribute is placed on the user buffer, not the USB hardware FIFO.

My comment was about the C interface, so it most definitely is relevant.

Indeed, I was just impatient, sorry.

Instead of simply marking the buffer unaligned, so the compiler will deal with any alignment issues, I claim that it is better to use the data type native to the interface (32-bit words) instead, since it also ensures the buffers are aligned, and reminds the programmers that this interface really prefers the buffers to be aligned.  It does not preclude using the buffers in a byte-wise manner, since casting the buffer to an unsigned char pointer does exactly that.  Path of Least Surprise and all.

I see your point. Let me not disagree while offering an alternative view: Path of User's Absolute Comfort.

the burden of packing/unpacking/realigning/whatever the naturally byte-natured data is upon the programmer, but at the moment somebody starts to write a "library", it is expected by the users of "library" that it's that "library" which takes upon itself that burden.

JW

[DiTBho]

All Cortex-M3/M4/M7 devices should support unaligned access

Yup, indeed! ARM is usually tolerant, other architectures are not.

It's a serious problem with machines like

• MIPS (R5k...R16K, MIPS32, MIPS64)
• PPC embedded (PPC4xx)
• SH4 embedded

their load/store don't support unaligned access.

/* uint08 */ when (IO.size isEqualTo 1 ) ----> no problem
/* uint16 */ when ((IO.size isEqualTo 2 ) and (modulo(addr,2) isNotEqualTo 0)) ----> trap(misaligned)
/* uint32 */ when ((IO.size isEqualTo 4 ) and (modulo(addr,4) isNotEqualTo 0)) ----> trap(misaligned)
/* uint64 */ when ((IO.size isEqualTo 8 ) and (modulo(addr,8) isNotEqualTo 0)) ----> trap(misaligned)

[DiTBho]

in others accessed strictly-16-bit

Dunno why, but things like C8900 (Ethernet) are 16-bit
(8bit works, but ... it's hw-bugged)

[nctnico]

All Cortex-M3/M4/M7 devices should support unaligned access

Yup, indeed! ARM is usually tolerant, other architectures are not.

Not in my experience. Even application processors may have ARM CPUs that don't allow unaligned access and I have come across systems that don't even throw a bus error but corrupt data silently.

[peter-h]

This includes 0.

As a small point, I think 0 probably doesn't exist in the target -> PC direction (what USB calls "IN" direction). May be legal though, of course, but why would the target want to transmit 0 bytes? ST don't test for it, but

Code: [Select]

    for (i = 0U; i < count32b; i++)
won't execute if count32b==0.

but at the moment somebody starts to write a "library", it is expected by the users of "library" that it's that "library" which takes upon itself that burden.

Yes; we are stuck with this, since it appears nobody actually knows how the hardware works, so "we" have to speculate.

Anyway it looks like, for the 32F4, the ST code is right with the __packed simply removed, and explicitly aligning the buffers is just an extra "safety" step.

It does make sense to feed peripherals with 32 bits, since they run much slower than the CPU (48MHz for the USB?).

[wek]

> why would the target want to transmit 0 bytes?

The reason is the same in both directions - see USB2.0 5.3.2. Pipes, treatise of "short packets" (for definition of short packet see 9.4.3).

> for (i = 0U; i < count32b; i++)
> won't execute if count32b==0.

This is in the "data" stage of transfer; in case of ZLP (zero-length packet), the OTG machine won't proceed to that. The Operational model subchapter of OTG chapter(s) in RM0090 describe that.

JW

[peter-h]

Long irrelevant now, but

And I counted also the information provided by unpaid volunteers

I've just got a reply (email notification) from "Piranha" to a Q I posted in March 2022

I realise everyone is unpaid but that is quite wrong for a $14BN company.

[Nominal Animal]

This is in the "data" stage of transfer; in case of ZLP (zero-length packet), the OTG machine won't proceed to that. The Operational model subchapter of OTG chapter(s) in RM0090 describe that.

Would a "dummy" write to DFIFO fix that?  (As in, trigger the transfer but discard all four bytes?)

If so,
    len += !len;  // Minimum 1
or
    count32b = ((uint32_t)len + 3U + !len) / 4U;  // Minimum 1
or
    q = src + (bytes + 3 + !bytes) / 4;  // Minimum 1
would be a simple fix.

[ataradov]

Would a "dummy" write to DFIFO fix that?  (As in, trigger the transfer but discard all four bytes?)

Fix what exactly? The trigger for a transfer is not a FIFO write, but the count write. You can write 0 and ZLP would be sent. No need to write FIFO in that case.

[Nominal Animal]

Would a "dummy" write to DFIFO fix that?  (As in, trigger the transfer but discard all four bytes?)

Fix what exactly? The trigger for a transfer is not a FIFO write, but the count write. You can write 0 and ZLP would be sent. No need to write FIFO in that case.

Ah, okay.  I understood wek's post above to mean the functions shown have a bug in the ZLP case.  They don't, because in the zero length case nothing needs to be written to the DFIFO anyway.

The C interface feels increasingly odd to me.  In particular, why is the count write separate from the buffer fill?  Weird.

[ataradov]

It is impossible to even enumerate the device without exchanging ZLPs, so there won't be bugs like that.

The interface is structured that way because this is a reasonably low level function, basically just an abstraction over hardware. Plus you may want to use multiple calls before you send the data. FIFO can store more data than a endpoint size and USB controller is capable of splitting that into multiple packets automatically.

[peter-h]

I did wonder why ZLPs were needed... What a weird protocol.

In the early days of USB, the standard comment was that the spec was written by a bunch of kids, didn't work, but there was so much commercial drive behind it that eventually it got sorted out.

Bluetooth is a decade further along that curve

This has been an interesting learning experience but I would hate to be someone employing coders at $100/hr having to sort out this stuff. It works for me because I value my time at zero, and I enjoy most of it. I guess larger industrial firms buy in commercial products for USB, ETH, etc, with paid support.

[DiTBho]

All Cortex-M3/M4/M7 devices should support unaligned access

Yup, indeed! ARM is usually tolerant, other architectures are not.

Not in my experience. Even application processors may have ARM CPUs that don't allow unaligned access and I have come across systems that don't even throw a bus error but corrupt data silently.

yeah, that's why I say "usually" 

My R18200 does the same: it silently corrupts things if you don't enable the bus misalignment trap.
So it's not a "tolerant" CPU, it's a "bastard" CPU.

Yup, there are also this kind of CPUs around!

[ataradov]

I did wonder why ZLPs were needed... What a weird protocol.

ZLPs are extremely useful. If you need to send 512 byte array over 64 byte endpoint, you just send it in 8 packets and finish with ZLP notifying that you are done. CDC protocol relies on this, since CDC is not just serial ports as people assume, but it also sends fully separated Ethernet frames. ZLP acts as a packet  separator.

In the early days of USB, the standard comment was that the spec was written by a bunch of kids

Get off your high horse. You have no idea about how the standard works, yet make comments like this.

I guess larger industrial firms buy in commercial products for USB, ETH, etc, with paid support.

Not really. Vendor stacks usually works fine if you take just a bit of time to figure it out and actually just take time to read the spec. Commercial firms just hire people that don't struggle with proimitive things.