Skip to content

Fixes issue with memory alignment in STM32 #3045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2020

Conversation

k0d
Copy link

@k0d k0d commented Jun 17, 2020

When compiling with optimizations on, an issue occurs where the claimed_pins/never_reset_pins memory location is shared with another variable. This causes some bad memory read, so the USB pins ended up being reset. Setting these to have an alignment of 4 bytes resolves this.

Tested on nucleo_f746zg

@k0d k0d requested a review from hierophect June 17, 2020 18:49
@k0d k0d requested a review from tannewt June 17, 2020 18:49
When compiling with optimizations on, an issue occurs where the claimed_pins/never_reset_pins memory location is shared with another variable. This causes some bad memory read, so the USB pins ended up being reset. Setting these to have an alignment of 4 bytes resolves this.

Tested on nucleo_f746zg
@k0d k0d force-pushed the stm32_alignment_issue branch from f61bc30 to 23da0fa Compare June 17, 2020 22:51
@tannewt
Copy link
Member

tannewt commented Jun 18, 2020

Why does it matter whether these two variables share a word? It shouldn't.

@k0d
Copy link
Author

k0d commented Jun 18, 2020

It's not that they shouldn't share a word, but that they shouldn't cross a word boundary. I don't know why it affects that variable specifically...but some way that it's used is causing issues because it's crossing the word boundary.

@dhalbert
Copy link
Collaborator

Some code might be accidentally treating them as uint32_t or similar. I looked for that briefly but didn't see that.

@k0d
Copy link
Author

k0d commented Jun 18, 2020

I'd love to get to the real 100% cause of this, and I will, but I feel that this is a suitable fix for now. I'll keep looking at it, but I need to move on to the DAPIO issue asap.

I tried switching everything to uint32_t, this didn't fix it either.

@k0d
Copy link
Author

k0d commented Jun 23, 2020

@tannewt You told me once to ping you if PRs are not looked at for a few days...so here's your ping

@tannewt
Copy link
Member

tannewt commented Jun 23, 2020

@k0d Yup! Thank you! I thought you were going to follow up with an update.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!

@tannewt tannewt merged commit f67b307 into adafruit:main Jun 23, 2020
@k0d
Copy link
Author

k0d commented Jun 23, 2020

Haha...no...that's a good enough fix for now. I'll do some deep dive into it at some point...maybe bring it up at 'in the weeds' on a Monday.

@k0d k0d deleted the stm32_alignment_issue branch June 28, 2020 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants