search

electro-smith / libDaisy

Hardware Library for the Daisy Audio Platform
https://www.electro-smith.com/daisy
MIT License
312 stars 131 forks source link

Changes for issues with compiling with optimisation #484

Closed AndrewCapon closed 2 years ago

AndrewCapon commented 2 years ago

There are issues with the HAL usb files: stm32h7xx_ll_usb.h and stm32h7xx_ll_usb.cpp where there can be timing issues due to code being optimised out.

See: https://community.st.com/s/question/0D50X0000BWpRO7SQN/stm32h7-usb-driver-fails-highest-optimization-o3

The solution from that link was to set any io memory locations to _IO and make the delay count variables volatile. I have done this for these files.

stephenhensley commented 2 years ago

@AndrewCapon thanks for the report, and quick fix!!

I think it looks OK to me.

To clarify a bit, when using the default linker, and running everything from the internal flash the USB works properly even with -O3 set?

Does the failure occur with either USB port, or just one (built-in, or external).

If that's the case, it's interesting that it only becomes an issue when the firmware is executing from SRAM. Are you using the bootloader in core/, and one of the other included linker scrips, or do you have some sort of custom set up running?

I ask because we have seen one other existing issue with that bootloader that we've yet to identify/resolve where one of the external USB pins doesn't function properly after exiting the bootloader.

AndrewCapon commented 2 years ago

It is easier to hit the problem when running from SRAM with the clock at 480 but you could also hit it running from flash depending on how long the register writes take to hold in the underlying usb.

The issue in the reset is that it is checking a bit to see when the reset has occurred, without the loop initial check the initial bit read may be an old version and already at 0 so it exits the loop before it has transitioned to 1 and then back to 0.

So I think that first comparison is enough to get that initial state change it then needs to check for the correct state change!

I had had the odd issue before running from flash, but it was easy to get it in a state in sram where it would repeatedly fail and only work occasionally, some times then for multiple times!

It will be the same with both PHYs.

It might well fix the boot loader issue, they put those loops in for a reason :)

I'm just using a modified linker script at the moment and putting the vector table in flash and the code in ram.

AndrewCapon commented 2 years ago

p.s. Has anyone been working on the USB Audio, I worked on some code a few years ago for an stm32f7 that had Composite audio and midi. I may be able to dig that out...

stephenhensley commented 2 years ago

@AndrewCapon thanks for the clarification!

We'll take a quick look and see if we can repro/validate the fix, but everything looks okay to me.

USB Audio is planned, but there's been no action on it so far. Same with composite devices (which we're hoping to get working before adding too many more device classes since you already have to pick/choose for CDC vs MIDI).

If either or both of those are something you'd be interested in contributing we then that would be very welcome!

stephenhensley commented 2 years ago

Totally forgot that this was good to go.

Thanks again for the contribution!! 🚀