Error in stm32f4xx_ll_usb.c

[Marek Kuziel]

In function USB_DevInit (row 320) is some inconsistency:

   USBx->GOTGCTL &= ~USB_OTG_GCCFG_NOVBUSSENS;

should be:

   USBx->GOCCFG &= ~USB_OTG_GCCFG_NOVBUSSENS; ?

STM32CubeF4 version 1.24.1

[Imen.D]

Hello @Marek Kuziel​ ,

Thank you for your bug report.

We will raise this issue to our development team for fix.

Best Regards,

Imen

[Imen.D]

Hello,

This issue is confirmed and will be fixed in the next release of STM32CubeF4 firmware package.

Best Regards,

Imen

[Dave Nadler]

@Imen DAHMEN​ - Do the developers understand that if they defined+used proper bitfields for all registers, this kind of bug would not be possible?

[After Forever]

Please no, bit fields are not very portable and a lot of things are compiler/implementation specific, especially when used with unions and some other edge cases, like different endiannes.

From "Writing Portable Code" by Didier Malenfant:

Bit fields are generally a bad idea, especially if used for persistent data or hardware registers. A lot of people still find them very useful but keep in mind that they tend to be extremely machine dependent.

[Dave Nadler]

@After Forever​ - Of course they are machine-dependent, that's the whole point.

More worrisome is compiler differences, but in this case that is quite manageable with the limited compilers used.

For an example of how this should be done, look at the ARM-provided headers ie core_cm4.h

Or are you going to tell us ARM don't know what they're doing??

[Marek Kuziel]

Agree with Dave Nadler. it's a pity they don't use bit fields. the code would be much more readable. By the way, I'm playing with USB programming in Ada. Of course, I often use records with representation clauses. And this feature of Ada allowed me to easy find this kind of error.

[After Forever]

Please see this thread for some good arguments.

Added:

From the stackoverflow link:

C standard allows compiler to put bit-fields in any order. There is no reliable and portable way to determine the order.

So, if the standard gives no guaranty, then define the bit fields however you want in the code, if you want them in specific places then even if it works now it is still a hack, e.g. tomorrow's GCC or IAR version may decide to store the bit fields differently, then what?

[Piranha]

> For an example of how this should be done, look at the ARM-provided headers ie core_cm4.h

But they are doing this only for 4 registers out of dozens...